vbhanuchander-lang commented on PR #17933:
URL: https://github.com/apache/iceberg/pull/17933#issuecomment-5547431272
@danielcweeks @twthorn They overlap in symptom but not in cause, and neither
one subsumes the other — I checked by running each PR's tests against the
other's fix.
#17552 guards the write: a per-instance `committedOffsets` cache in
`commitConsumerOffsets()` that refuses to commit behind what *this* instance
last committed. That is the zombie-coordinator case in #17551 — coordinator A's
tracked position is legitimately its own, and the rewind comes from A
committing over B. This PR guards the value: `controlTopicOffsets` itself moves
backwards in `consumeAvailable()` when a partition is re-read, so it is already
wrong before any commit path sees it.
Applying #17552's `Channel.java` onto main, keeping main's `put` in
`consumeAvailable`, and running this PR's `TestChannel`:
```
committedControlTopicOffsetsDoNotRegressOnReplay FAILED expected offset=5,
was offset=3
controlTopicOffsetsTrackTheHighestPositionConsumed FAILED expected {0=5},
was {0=3}
```
#17552's guard does not fire here because the cache is empty on an
instance's first commit — `lastCommittedOffset == null` commits unconditionally
— which is exactly the state a coordinator is in right after the rebalance that
caused the re-read.
The reverse also holds. This PR's `Channel.java` on main with #17552's
`TestCoordinator`:
```
testCommitConsumerOffsetsDoesNotRewind FAILED expected 100, was 5
testCommitConsumerDuplicateDoesNotCommit FAILED expected 105, was
100
testCommitConsumerMixedPartitionsRewindOrAdvance FAILED expected 200, was
195
```
Making one instance's map monotonic says nothing about another coordinator's
position.
One further gap: `doCommit` passes `controlTopicOffsets()` to
`commitToTable` *before* `commitConsumerOffsets()` runs, and that value is
stamped into `kafka.connect.offsets.*`. The `Long::max` fold there protects a
table that already has an offset for the partition, but a table with no prior
entry for it gets the rewound value written — #17552's guard sits downstream of
that write and cannot reach it.
The two are complementary and do not conflict textually — different methods
in the same file. #17552 is further along, so I am happy to rebase this on top
of it once it lands.
--
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]
