-
Notifications
You must be signed in to change notification settings - Fork 15.4k
KAFKA-8421: Still return data during rebalance #7312
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 1 commit
37b5c63
4a34116
d964cea
f5b8f96
b07bac2
29f8942
3000d6f
8464fa7
f804b3d
3c62029
284ef77
9825063
929048a
58adc09
c79a93d
75c4e88
95ef3fb
3b844ff
781514d
8b08a3e
8fa5d74
d75fcb9
b176e4e
7a826f9
31e372e
366739b
faf787e
c71af82
ffe6146
518049c
30d5e53
fa3456d
0220d96
27d013a
94611cc
b529847
33fd785
1df9808
3057405
21f531d
effb29d
6fbd976
f4e7111
91b6606
0ad52ec
27f76b7
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 |
|---|---|---|
|
|
@@ -761,7 +761,11 @@ public boolean refreshCommittedOffsetsIfNeeded(Timer timer) { | |
|
|
||
| log.info("Setting offset for partition {} to the committed offset {}", tp, position); | ||
| entry.getValue().leaderEpoch().ifPresent(epoch -> this.metadata.updateLastSeenEpochIfNewer(entry.getKey(), epoch)); | ||
| this.subscriptions.seekUnvalidated(tp, position); | ||
|
|
||
| // it's possible that the partition is no longer assigned when the response is received, | ||
| // so we need to ignore seeking if that's the case | ||
| if (this.subscriptions.isAssigned(tp)) | ||
|
Contributor
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. @hachikuji This is the change I make for auto offset reset. I thought about adding a unit test but since the logic is wrapped in a single
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. Does it make sense to move this check above a little bit? if (offsetAndMetadata != null && subscriptions.isAssigned(tp)) {That will make the log message less confusing.
Contributor
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. Hmm, but I thought even if the partition is no longer assigned, we may still want to update its epoch; on the other hand your concern is valid. Will tweak it a bit more. |
||
| this.subscriptions.seekUnvalidated(tp, position); | ||
| } | ||
| } | ||
| return true; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think it's worth adding a debug message saying that we're ignoring the fetched offset?