SEZ9 commented on PR #12316: URL: https://github.com/apache/seatunnel/pull/12316#issuecomment-6308433149
Thanks @davidzollo for the detailed verification and @nzw921rx for the LGTM. Before merging I'd like to close out the open points from the earlier review pass, since I don't see follow-up on them yet: 1. **Timer flush path in `SourceFlowLifeCycle`** — the handoff currently wraps only `triggerBarrier`. The timer-driven `FlushSignal` still contends on the same unfair monitor without calling `injectorArriving`/`injectorFinished`, and the reader-side wait in `collect()` only yields to registered injectors, so a pending timer flush keeps its pre-existing unbounded starvation exposure and can pin the shared `timerFlushWorker` thread. Could you either route the timer flush through the same announce/withdraw pair, or explain why that contender is intentionally left out? 2. **`injectorAcquiresLockWithinOnePollCycle`** — `completedPolls` is sampled before `injectorArriving()`, so the reader can start a fresh poll in that window and `pollsWaited <= 1` can fail under scheduling noise. Please sample after announcing (or widen the bound and document why). 3. **`awaitInjectors` sleep loop** — the `Thread.sleep(1)` polling ties reader resumption to OS timer granularity rather than to `injectorFinished()`. A park/unpark-style wakeup would be preferable; if you'd rather keep the sleep for simplicity, a short comment on the trade-off is fine. 4. **`SourceCheckpointLockHandoff` class javadoc** — "an injector never waits for the reader" is easy to misread as a latency guarantee; please clarify that it is a progress/non-blocking statement, and state explicitly whether the timer flush contender is covered by the handoff (ties back to item 1). On CI: run 36327199671 concluded `cancelled` again on the retry, so we still have no non-cancelled Build result for this head. Pushing the changes above should trigger a fresh workflow and give us a real result — please ping me when it's up. <!-- streview-comment:1927 --> -- 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]
