nahidupa commented on PR #17925:
URL: https://github.com/apache/iceberg/pull/17925#issuecomment-5600930892

   @wombatu-kun Following up on the consolidation comment: the branch has been 
rebuilt on main (`f05bf491a`) as `2e9d2e927`. Both the reset-on-rebalance 
approach and the skip-before-dispatch guard have been removed. `Channel.java` 
and the merged `TestChannel` are unchanged relative to that base.
   
   The remaining change is confined to readiness: each source topic/partition 
contributes at most once for the active commit ID. `Long::max` keeps offsets 
monotonic, but does not prevent repeated `DataComplete` assignments from 
inflating readiness. A dispatch guard and this set-based check overlap for 
ordinary record replay, but enforce different invariants: one avoids repeated 
dispatch, while the other also handles separate responses that report the same 
partition. This is not a replacement for all of the protection in #17713.
   
   I independently rechecked the tests at this head. Temporarily replacing the 
partition set with a duplicate-accepting list fails exactly the three new 
regressions out of 24 focused tests; restoring the set passes all 24. A fresh 
full connector unit run passed 144 tests across 19 suites, and the connector 
check gates passed. That corrects the 143-test count currently in the 
description.
   
   One qualification to the current test wording: the coordinator regression 
delivers duplicate payloads at newer Kafka offsets. It checks that no early 
snapshot is created and that, after the missing partition reports, the file is 
added once in one snapshot. It does not yet exercise a same-offset consumer 
rewind, and its null timestamps do not verify a non-null completeness 
watermark. No real-broker rebalance was tested. This change prevents duplicate 
partition contributions; it does not establish that the expected partition 
count is current or solve coordinator-replacement recovery and durable replay 
deduplication.
   
   Would you prefer this readiness change to remain here, or be included 
alongside #17713? The duplicated dispatch guard is no longer proposed in this 
PR.


-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to