[LogCollector] Get logs fix round 2 [1.5.x] (mlrun#4549)

jond01 · Nov 9, 2023 · 3603adc · 3603adc
1 parent 75420b9
commit 3603adc
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 12 deletions.
diff --git a/go/pkg/services/logcollector/logcollector_test.go b/go/pkg/services/logcollector/logcollector_test.go
@@ -350,8 +350,8 @@ func (suite *LogCollectorTestSuite) TestGetLogsWithSize() {
 	logFilePath := suite.logCollectorServer.resolveRunLogFilePath(suite.projectName, runUID)
 
 	// write log file
-	// write 1k log lines
-	for i := 0; i < 1000; i++ {
+	// write 10 times buffer size log lines
+	for i := 0; i < 10*suite.logCollectorServer.getLogsBufferSizeBytes; i++ {
 		logText := fmt.Sprintf("Some fake pod logs %d\n", i)
 		err := common.WriteToFile(logFilePath, []byte(logText), true)
 		suite.Require().NoError(err, "Failed to write to file")
@@ -366,12 +366,14 @@ func (suite *LogCollectorTestSuite) TestGetLogsWithSize() {
 		offset           int
 		readSize         int
 		expectedReadSize int
+		verifyContents   bool
 	}{
 		{
 			name:             "Read it all",
 			offset:           0,
 			readSize:         -1,
 			expectedReadSize: fileContentsLength,
+			verifyContents:   true,
 		},
 		{
 			name:             "Read nothing",
@@ -384,12 +386,28 @@ func (suite *LogCollectorTestSuite) TestGetLogsWithSize() {
 			offset:           0,
 			readSize:         100,
 			expectedReadSize: 100,
+			verifyContents:   true,
 		},
 		{
-			name:             "Read 100 bytes with offset",
-			offset:           10,
-			readSize:         100,
-			expectedReadSize: 100,
+			name:             "Read buffer size bytes with offset",
+			offset:           fileContentsLength / 2,
+			readSize:         suite.logCollectorServer.getLogsBufferSizeBytes,
+			expectedReadSize: suite.logCollectorServer.getLogsBufferSizeBytes,
+			verifyContents:   true,
+		},
+		{
+			name:             "Read buffer size * 2 bytes with offset",
+			offset:           fileContentsLength / 2,
+			readSize:         suite.logCollectorServer.getLogsBufferSizeBytes * 2,
+			expectedReadSize: suite.logCollectorServer.getLogsBufferSizeBytes * 2,
+			verifyContents:   true,
+		},
+		{
+			name:             "Read buffer size / 2 bytes with offset",
+			offset:           fileContentsLength / 2,
+			readSize:         suite.logCollectorServer.getLogsBufferSizeBytes / 2,
+			expectedReadSize: suite.logCollectorServer.getLogsBufferSizeBytes / 2,
+			verifyContents:   true,
 		},
 		{
 
@@ -399,15 +417,17 @@ func (suite *LogCollectorTestSuite) TestGetLogsWithSize() {
 			offset:           fileContentsLength - int(1.5*float64(suite.logCollectorServer.getLogsBufferSizeBytes)),
 			readSize:         int(1.3 * float64(suite.logCollectorServer.getLogsBufferSizeBytes)),
 			expectedReadSize: int(1.3 * float64(suite.logCollectorServer.getLogsBufferSizeBytes)),
+			verifyContents:   true,
 		},
 		{
 			name:             "Overflowing read size return what is left",
 			offset:           fileContentsLength - 1,
 			readSize:         100,
 			expectedReadSize: 1,
+			verifyContents:   true,
 		},
 		{
-			name:             "Overflowing offset",
+			name:             "Overflowing offset - returns nothing",
 			offset:           fileContentsLength,
 			readSize:         -1,
 			expectedReadSize: 0,
@@ -425,6 +445,11 @@ func (suite *LogCollectorTestSuite) TestGetLogsWithSize() {
 
 			// verify logs
 			suite.Require().Equal(testCase.expectedReadSize, len(nopStream.Logs))
+			if testCase.verifyContents {
+				suite.Require().Equal(string(
+					fileContents[testCase.offset:testCase.offset+testCase.expectedReadSize]),
+					string(nopStream.Logs))
+			}
 		})
 	}
 

diff --git a/go/pkg/services/logcollector/server.go b/go/pkg/services/logcollector/server.go
@@ -1086,11 +1086,6 @@ func (s *Server) getChunkSize(
 		return 0
 	}
 
-	// if we didn't read anything yet, assume read the initial offset
-	if totalLogsSize == 0 {
-		totalLogsSize += initialOffset
-	}
-
 	// if the size we need to read is bigger than the buffer, use the buffer size
 	leftToRead := endIndex - totalLogsSize - initialOffset