wombatu-kun commented on code in PR #19202:
URL: https://github.com/apache/hudi/pull/19202#discussion_r3533524590
##########
hudi-flink-datasource/hudi-flink/src/main/java/org/apache/hudi/table/format/cdc/CdcIterators.java:
##########
@@ -135,14 +153,25 @@ public boolean hasNext() {
}
@Override
- public RowData next() {
+ public synchronized RowData next() {
Review Comment:
The concurrency here comes from Flink's `SourceReaderBase` split-fetcher
model, not from Hudi choosing to share the iterator across threads. `fetch()`
hands the `CdcFileSplitsIterator` to `BatchRecords` undrained (lazy read), so
the task thread drains it via `BatchRecords.nextRecordFromSplit()` while, on a
forced teardown, the split-fetcher thread runs
`HoodieSourceSplitReader.close()` -> `HoodieCdcSplitReaderFunction.close()` on
the same live instance.
Keeping and closing `currentIterator` in the reader function is a deliberate
cleanup safety net: when a batch is enqueued but the task thread never
drains/recycles it (what a forced cancel causes), that `close()` is the only
thing that releases the `CdcImageManager` and the file-group readers, since
`SourceReaderBase` does not recycle batches still queued in its element queue.
A pure caller-side fix that lets only the task thread touch the iterator is
feasible, but it trades that guaranteed cleanup for a possible handle leak on
forced cancel unless the teardown cleanup is also moved onto the task-thread
drain path. Prefer that route, or keep the iterator-side guard given the
tradeoff?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
