-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Fixes a bug in Kafka auto reset #12008
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
16d4acb
86da238
4701b3f
fd849e2
cc7a704
2016807
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -126,38 +126,59 @@ private void possiblyResetOffsetsOrWait( | |
| TaskToolbox taskToolbox | ||
| ) throws InterruptedException, IOException | ||
| { | ||
| final Map<TopicPartition, Long> resetPartitions = new HashMap<>(); | ||
| boolean doReset = false; | ||
| final Map<TopicPartition, Long> newOffsetInMetadata = new HashMap<>(); | ||
|
|
||
| if (task.getTuningConfig().isResetOffsetAutomatically()) { | ||
| for (Map.Entry<TopicPartition, Long> outOfRangePartition : outOfRangePartitions.entrySet()) { | ||
| final TopicPartition topicPartition = outOfRangePartition.getKey(); | ||
| final long nextOffset = outOfRangePartition.getValue(); | ||
| // seek to the beginning to get the least available offset | ||
| final long outOfRangeOffset = outOfRangePartition.getValue(); | ||
|
|
||
| StreamPartition<Integer> streamPartition = StreamPartition.of( | ||
| topicPartition.topic(), | ||
| topicPartition.partition() | ||
| ); | ||
| final Long leastAvailableOffset = recordSupplier.getEarliestSequenceNumber(streamPartition); | ||
| if (leastAvailableOffset == null) { | ||
| throw new ISE( | ||
| "got null sequence number for partition[%s] when fetching from kafka!", | ||
| topicPartition.partition() | ||
| ); | ||
|
|
||
| final Long earliestAvailableOffset = recordSupplier.getEarliestSequenceNumber(streamPartition); | ||
| if (earliestAvailableOffset == null) { | ||
| throw new ISE("got null earliest sequence number for partition[%s] when fetching from kafka!", | ||
| topicPartition.partition()); | ||
| } | ||
| // reset the seek | ||
| recordSupplier.seek(streamPartition, nextOffset); | ||
| // Reset consumer offset if resetOffsetAutomatically is set to true | ||
| // and the current message offset in the kafka partition is more than the | ||
| // next message offset that we are trying to fetch | ||
| if (leastAvailableOffset > nextOffset) { | ||
| doReset = true; | ||
| resetPartitions.put(topicPartition, nextOffset); | ||
|
|
||
| if (outOfRangeOffset < earliestAvailableOffset) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this logic can further be refined here to call reset within this block itself. Also, it looks like an So it could look something like this:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @gianm - could you explain the scenario in which the offset we are asking the consumer to seek to could possibly be higher than the latest available offset in Kafka?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For a partition that we've already committed some offset for, the starting offset given to a task for that partition is going to be the committed offset plus 1. So if no new messages have been written since the last commit, the starting offset will not exist yet.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the clarification, @gianm! @FrankChen021 - I would probably get rid of the log line I mentioned then and have the else block as it is (but moved as else condition for
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @samarthjain I kept the code block as it was because original code reset the meta for all partition out of the partition loop and only reset for one time. |
||
| //In this case, it's probably because the messages are no longer in the Kafka cluster | ||
| // i.e. the messages in [outOfRangeOffset, earliestAvailableOffset) are lost. | ||
| // Since these lost messages can no longer be recovered, | ||
| // it's reasonable to reset the offset to the earliest available position to help ingestion resume. | ||
| log.warn("Automatically seeking Kafka offset to the earliest offset [%d]", earliestAvailableOffset); | ||
| recordSupplier.seek(streamPartition, earliestAvailableOffset); | ||
|
|
||
|
FrankChen021 marked this conversation as resolved.
|
||
| newOffsetInMetadata.put(topicPartition, earliestAvailableOffset); | ||
| } else { | ||
| // There are two cases in theory here | ||
| // 1. outOfRangeOffset is in the range of [earliestAvailableOffset, latestAvailableOffset] | ||
| // 2. outOfRangeOffset is larger than latestAvailableOffset | ||
| // | ||
| // for scenario 1, we do nothing but just wait for a period time to retry | ||
| // since current offset is valid but maybe due to some temporary problem | ||
| // | ||
| // for scenario 2, how could this happen? | ||
| // Well, if the task first consumes from a topic on cluster A, | ||
| // and then supervisor spec is changed to consume from a same topic, where there are messages in this topic, on cluster B, | ||
| // this can lead to this case. | ||
| // For such case, | ||
| // offsets stored in meta should be cleared when submitting the supervisor spec, | ||
| // so the problem won't be left to manual reset or auto reset. Thus, we don't need to handle this complicated case here | ||
| log.warn("Offset [%d] is out of range of the available offsets for partition [%s]. It is likely that a manual offset reset of the supervisor is needed.", | ||
| outOfRangeOffset, | ||
| topicPartition); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (doReset) { | ||
| sendResetRequestAndWait(CollectionUtils.mapKeys(resetPartitions, streamPartition -> StreamPartition.of( | ||
| if (!newOffsetInMetadata.isEmpty()) { | ||
| log.warn("Automatcally resetting offset in metadata to [%s]", newOffsetInMetadata.toString()); | ||
|
|
||
| sendResetRequestAndWait(CollectionUtils.mapKeys(newOffsetInMetadata, streamPartition -> StreamPartition.of( | ||
| streamPartition.topic(), | ||
| streamPartition.partition() | ||
| )), taskToolbox); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.