surafel58 commented on PR #11827: URL: https://github.com/apache/seatunnel/pull/11827#issuecomment-5359165723
Thanks @SEZ9, these are good points. Addressed on the current head (9f321990): 1. Replay-safety comment: you are right that it overstated idempotency. Softened it. The comment now says the replayed flush is harmless only if the receiver tolerates re-sent samples; a receiver that rejects duplicate or out-of-order samples may fail the replayed flush, so the guarantee is at-least-once, not exactly-once (not unconditionally idempotent). 2. Retry/backoff on the flush: this one I would like to justify deferring rather than fold into this PR. flush() has always been single-shot, and every trigger uses it: the batch_size threshold, the Zeta timer flush, close(), and now prepareCommit(). So the no-retry behavior is pre-existing, not introduced here; this PR only adds one more caller. The engine restart strategy is the current recovery path for transient failures, and throwing (rather than silently swallowing) is the intended at-least-once behavior. A bounded retry should apply uniformly to all four flush paths, not just the checkpoint path, so I filed #11911 to add retry with backoff to flush() holistically and will follow up there. Happy to reconsider if you feel it must block this PR. 3. Buffer-cleared test gap: added. The prepareCommit test now asserts that a second prepareCommit() with no new rows sends nothing more, so a later checkpoint cannot re-deliver the same row. Thanks again for the careful review. -- 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]
