kgo: fix mark <=> epoch interaction, make epoch handling more resilient #1203
+38
−15
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MarkCommit{Records,Offsets} compared the epoch/offset you were marking
against the existing head epoch/offset. If the there was no internally
existing epoch/offset, or only `dirty` was set (via being consumed)
while `head` / `committed` were both the default struct value {0,0},
then trying to mark a record with a negative epoch would be ignored.
Note that returning -1 via the broker epoch requires a broker to both
(a) SUPPORT epochs, i.e. implement all the Kafka APIs with leader epoch
support, and then (b) NOT SUPPORT epochs, i.e. return / use -1
everywhere. This has only been seen against Azure Event hubs.
Anyway, now, when initializing an EpochOffset internally (for an
uncommit {dirty,head,committed}, the epoch is explicitly initialized
with -1. Further, for added robustness, MarkCommit{Records,Offsets}
only compares against existing values -- if a value does not exist,
we auto-accept the mark.
This commit also:
* Improves EpochOffset.Less to assume all negative epochs are -1
* Simplifies the logic in CommitRecords to use EpochOffset.Less
* Adds debug lines in metadata when new partitions are added
* Adds the epoch to an existing debug log line when updating uncommitted
* Sets the LeaderEpoch to -1 for records when producing, and clarifies
in docs that -1 should have always been the case.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
MarkCommit{Records,Offsets} compared the epoch/offset you were marking against the existing head epoch/offset. If the there was no internally existing epoch/offset, or only
dirtywas set (via being consumed) whilehead/committedwere both the default struct value {0,0}, then trying to mark a record with a negative epoch would be ignored.Note that returning -1 via the broker epoch requires a broker to both (a) SUPPORT epochs, i.e. implement all the Kafka APIs with leader epoch support, and then (b) NOT SUPPORT epochs, i.e. return / use -1 everywhere. This has only been seen against Azure Event hubs.
Anyway, now, when initializing an EpochOffset internally (for an uncommit {dirty,head,committed}, the epoch is explicitly initialized with -1. Further, for added robustness, MarkCommit{Records,Offsets} only compares against existing values -- if a value does not exist, we auto-accept the mark.
This commit also: