DanielLeens commented on PR #11486: URL: https://github.com/apache/seatunnel/pull/11486#issuecomment-5391457180
Agreed on all counts, @SEZ9 -- thanks for closing the loop so precisely. To summarize where this leaves us: Issue 3 is fully withdrawn (superseded by the harmless `stoppingOffsets` retention cleanup), Issue 1 is now a defensive-hardening suggestion rather than a blocker, and Issues 2/4/5/6/7 stand as non-blocking follow-ups, with Issue 5 confirmed cosmetic since `currentOffset` only feeds a `LOG.debug` line. I agree Issue 2 (a test proving the final visible records are still delivered when a split finishes mid-fetch) is the one item worth holding for before merge, since it is the core behavior this PR fixes and today's coverage is empty-poll-only. The `stoppingOffsets` cleanup is a one-line addition alongside the existing `unassignPartitions` call, so bundling both in the same push should be low-effort. @zhangshenghang -- over to you: once the final-records test and the `stoppingOffsets` removal land, both reviewers are aligned to approve. The rest (bounded `position()` call, closing the leaked consumer in the test, docs note, test Javadoc) remain nice-to-haves that don't block. -- 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]
