Skip to content

Commit e96c877

Browse files
Fix file source infinite re-read in non-tail mode with codec (#6934) (#6937)
* Stop file source from rescheduling readers in non-tail mode (#6934) FileReaderPool.onReaderComplete inferred the "should I reschedule?" decision from the completed reader's RotationType, which has no terminal value. In non-tail mode any path that did not result in DELETED or CREATE_RENAME (notably NO_ROTATION and the codec one-shot path that never updates lastRotationType) was rescheduled every 500 ms, producing duplicate events. Make tail mode the single source of truth for rescheduling: when the reader completes in non-tail mode, mark the checkpoint completed, promote pending files, and exit. This restores the documented "non-tail = read once, stop" contract for the modern path and matches the behavior of the legacy ClassicFileStrategy. Resolves #6934 Signed-off-by: Srikanth Padakanti <srikanth_padakanti@apple.com> * Persist read offset after non-tail codec one-shot read (#6934) readFileWithCodecOneShot returned without updating the checkpoint entry, so a successful one-shot read advanced no offset. After the pool-side fix (no reschedule in non-tail mode), an in-process loop no longer occurs, but a restart would still re-read the file from offset 0 and produce duplicate events. After parseWithCodec returns true, advance readOffset, the checkpoint's read offset, and the committed offset to the file size. On parse failure the readErrors counter is incremented and the offsets stay at zero, matching the pre-fix semantics for the error path. Resolves #6934 Signed-off-by: Srikanth Padakanti <srikanth_padakanti@apple.com> --------- Signed-off-by: Srikanth Padakanti <srikanth_padakanti@apple.com>
1 parent eaca024 commit e96c877

4 files changed

Lines changed: 135 additions & 1 deletion

File tree

data-prepper-plugins/file-source/src/main/java/org/opensearch/dataprepper/plugins/source/file/FileReader.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,12 @@ private void readFileWithCodecOneShot() {
197197
try (final InputStream rawStream = Files.newInputStream(path);
198198
final InputStream decompressedStream = decompressionEngine.createInputStream(rawStream)) {
199199
metrics.getFilesOpened().increment();
200-
parseWithCodec(decompressedStream);
200+
if (parseWithCodec(decompressedStream)) {
201+
final long fileSize = fileOps.size(path);
202+
readOffset.set(fileSize);
203+
checkpointEntry.setReadOffset(fileSize);
204+
checkpointEntry.setCommittedOffset(fileSize);
205+
}
201206
} catch (final IOException e) {
202207
LOG.error("Error reading file with codec: {}", path, e);
203208
metrics.getReadErrors().increment();

data-prepper-plugins/file-source/src/main/java/org/opensearch/dataprepper/plugins/source/file/FileReaderPool.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,12 @@ private synchronized void onReaderComplete(final FileIdentity fileIdentity, fina
119119
}
120120
metrics.getActiveFileCount().decrementAndGet();
121121

122+
if (!readerContext.isTailMode()) {
123+
checkpointRegistry.markCompleted(fileIdentity.toString());
124+
promotePendingFiles();
125+
return;
126+
}
127+
122128
if (completedReader.getLastRotationType() == RotationType.CREATE_RENAME) {
123129
LOG.info("Re-adding path {} after create/rename rotation", path);
124130
final FileIdentity newIdentity = FileIdentity.from(path, readerContext.getFileOps(),

data-prepper-plugins/file-source/src/test/java/org/opensearch/dataprepper/plugins/source/file/FileReaderPoolTest.java

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@
4949
import static org.mockito.ArgumentMatchers.anyString;
5050
import static org.mockito.Mockito.lenient;
5151
import static org.mockito.Mockito.mock;
52+
import static org.mockito.Mockito.timeout;
53+
import static org.mockito.Mockito.verify;
5254
import static org.mockito.Mockito.when;
5355

5456
@ExtendWith(MockitoExtension.class)
@@ -620,4 +622,69 @@ void onReaderComplete_with_deleted_rotation_marks_completed_and_processes_pendin
620622

621623
limitedPool.shutdown();
622624
}
625+
626+
private FileReaderContext createNonTailReaderContext() {
627+
return new FileReaderContext(
628+
buffer, eventFactory, fileOps, metrics, rotationDetector,
629+
acknowledgementSetManager, false, StandardCharsets.UTF_8,
630+
4096, 1048576, 5000, Duration.ofSeconds(5),
631+
Duration.ofSeconds(30), StartPosition.BEGINNING, false,
632+
Duration.ofSeconds(30), 1000,
633+
Duration.ofSeconds(5), 3, null, false, in -> in);
634+
}
635+
636+
@Test
637+
void onReaderComplete_in_non_tail_mode_does_not_reschedule() throws Exception {
638+
Path testFile = tempDir.resolve("non-tail.log");
639+
Files.writeString(testFile, "line1\nline2\nline3\n");
640+
641+
Counter filesOpened = mock(Counter.class);
642+
Counter filesClosed = mock(Counter.class);
643+
Counter bytesRead = mock(Counter.class);
644+
Counter linesRead = mock(Counter.class);
645+
Counter eventsEmitted = mock(Counter.class);
646+
Timer backpressureTimer = mock(Timer.class);
647+
lenient().when(metrics.getFilesOpened()).thenReturn(filesOpened);
648+
lenient().when(metrics.getFilesClosed()).thenReturn(filesClosed);
649+
lenient().when(metrics.getBytesRead()).thenReturn(bytesRead);
650+
lenient().when(metrics.getLinesRead()).thenReturn(linesRead);
651+
lenient().when(metrics.getEventsEmitted()).thenReturn(eventsEmitted);
652+
lenient().when(metrics.getBackpressureTimer()).thenReturn(backpressureTimer);
653+
lenient().when(metrics.getFileLagBytes()).thenReturn(new AtomicLong(0));
654+
when(metrics.getActiveFileCount()).thenReturn(new AtomicLong(0));
655+
656+
BasicFileAttributes attrs = mock(BasicFileAttributes.class);
657+
when(attrs.fileKey()).thenReturn("inode-non-tail");
658+
when(attrs.creationTime()).thenReturn(FileTime.from(Instant.EPOCH));
659+
when(fileOps.readAttributes(testFile)).thenReturn(attrs);
660+
when(fileOps.size(testFile)).thenReturn(Files.size(testFile));
661+
FileChannel realChannel = FileChannel.open(testFile, StandardOpenOption.READ);
662+
when(fileOps.openReadChannel(testFile)).thenReturn(realChannel);
663+
664+
when(rotationDetector.checkRotation(any(), any(), any(long.class)))
665+
.thenReturn(RotationResult.NO_ROTATION);
666+
667+
EventBuilder mockBuilder = mock(EventBuilder.class);
668+
Event mockEvent = mock(Event.class);
669+
lenient().when(eventFactory.eventBuilder(EventBuilder.class)).thenReturn(mockBuilder);
670+
lenient().when(mockBuilder.withEventType(any())).thenReturn(mockBuilder);
671+
lenient().when(mockBuilder.withData(any(Map.class))).thenReturn(mockBuilder);
672+
lenient().when(mockBuilder.build()).thenReturn(mockEvent);
673+
674+
FileReaderPool pool = new FileReaderPool(
675+
checkpointRegistry, metrics, 10, 2,
676+
Duration.ofMinutes(30), createNonTailReaderContext());
677+
when(checkpointRegistry.getOrCreate(anyString())).thenReturn(new CheckpointEntry());
678+
679+
FileIdentity identity = FileIdentity.from(testFile, fileOps, 1024);
680+
pool.addFile(identity, testFile);
681+
682+
verify(checkpointRegistry, timeout(5000)).markCompleted(anyString());
683+
684+
await().pollDelay(1500, TimeUnit.MILLISECONDS)
685+
.atMost(3, TimeUnit.SECONDS)
686+
.until(() -> pool.getActiveReaderCount() == 0);
687+
688+
pool.shutdown();
689+
}
623690
}

data-prepper-plugins/file-source/src/test/java/org/opensearch/dataprepper/plugins/source/file/FileReaderTest.java

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -911,6 +911,62 @@ void run_one_shot_codec_with_acknowledgements_creates_ack_set() throws Exception
911911
verify(ackSet, atLeastOnce()).add(any(Event.class));
912912
}
913913

914+
@Test
915+
void run_one_shot_codec_persists_read_offset_after_successful_parse() throws Exception {
916+
Path testFile = tempDir.resolve("one-shot-offset.log");
917+
final String contents = "line1\nline2\nline3\n";
918+
Files.writeString(testFile, contents);
919+
final long fileSize = Files.size(testFile);
920+
921+
Counter filesOpened = mock(Counter.class);
922+
Counter filesClosed = mock(Counter.class);
923+
Counter linesRead = mock(Counter.class);
924+
Counter eventsEmitted = mock(Counter.class);
925+
Timer backpressureTimer = mock(Timer.class);
926+
when(metrics.getFilesOpened()).thenReturn(filesOpened);
927+
when(metrics.getFilesClosed()).thenReturn(filesClosed);
928+
lenient().when(metrics.getLinesRead()).thenReturn(linesRead);
929+
lenient().when(metrics.getEventsEmitted()).thenReturn(eventsEmitted);
930+
lenient().when(metrics.getBackpressureTimer()).thenReturn(backpressureTimer);
931+
932+
when(fileOps.size(testFile)).thenReturn(fileSize);
933+
934+
InputCodec mockCodec = mock(InputCodec.class);
935+
doAnswer(inv -> null).when(mockCodec).parse(any(), any());
936+
937+
FileReaderContext context = createOneShotContextWithCodecAndAcknowledgements(mockCodec);
938+
final FileReader reader = createReaderWithContext(testFile, context);
939+
reader.run();
940+
941+
assertThat(checkpointEntry.getReadOffset(), equalTo(fileSize));
942+
assertThat(checkpointEntry.getCommittedOffset(), equalTo(fileSize));
943+
assertThat(reader.getReadOffset(), equalTo(fileSize));
944+
}
945+
946+
@Test
947+
void run_one_shot_codec_does_not_persist_offset_when_parse_throws() throws Exception {
948+
Path testFile = tempDir.resolve("one-shot-offset-fail.log");
949+
Files.writeString(testFile, "line1\n");
950+
951+
Counter filesOpened = mock(Counter.class);
952+
Counter filesClosed = mock(Counter.class);
953+
Counter readErrors = mock(Counter.class);
954+
when(metrics.getFilesOpened()).thenReturn(filesOpened);
955+
when(metrics.getFilesClosed()).thenReturn(filesClosed);
956+
when(metrics.getReadErrors()).thenReturn(readErrors);
957+
958+
InputCodec mockCodec = mock(InputCodec.class);
959+
doThrow(new IOException("parse failed")).when(mockCodec).parse(any(), any());
960+
961+
FileReaderContext context = createOneShotContextWithCodecAndAcknowledgements(mockCodec);
962+
final FileReader reader = createReaderWithContext(testFile, context);
963+
reader.run();
964+
965+
assertThat(checkpointEntry.getReadOffset(), equalTo(0L));
966+
assertThat(checkpointEntry.getCommittedOffset(), equalTo(0L));
967+
verify(readErrors).increment();
968+
}
969+
914970
@Test
915971
void run_with_acknowledgements_batch_timeout_triggers_complete() throws Exception {
916972
Path testFile = tempDir.resolve("ack-timeout.log");

0 commit comments

Comments
 (0)