KAFKA-20232: Fix WakeupException in awaitMetadataUpdate()#21592
Conversation
github-actions
Bot
added
triage
|
A label of 'needs-attention' was automatically added to this PR in order to raise the |
lianetm
added
ci-approved
and removed
triage
Nikita-Shupletsov
left a comment
Nikita-Shupletsov
left a comment
There was a problem hiding this comment.
LGTM, thank you
|
Hi @Nikita-Shupletsov , thanks for the review. Wondering what the next step is here to get another approval |
|
@lianetm could you please take a look? thank you! |
lianetm
left a comment
lianetm
left a comment
There was a problem hiding this comment.
Thanks! Just minor comments around java docs
| * @param disableWakeup true if we should not check for wakeups, false otherwise | ||
| * @return true if update succeeded, false otherwise. | ||
| */ | ||
| public boolean awaitMetadataUpdate(Timer timer, boolean disableWakeup) { |
There was a problem hiding this comment.
could we add the "throws WakeUp" java doc?
| @@ -161,9 +161,20 @@ public boolean hasReadyNodes(long now) { | |||
| * @return true if update succeeded, false otherwise. | |||
| */ | |||
| public boolean awaitMetadataUpdate(Timer timer) { | |||
There was a problem hiding this comment.
here too, probably worth clarifying that this one "throws WakeUp"
(consistent with the java doc of the poll in this same file btw)
|
Also @Baisang could you please merge trunk latest changes too? thanks! |
3 participants
Asynchronous offset commits may throw an unexpected WakeupException.
This was addressed for some code paths previously in
#12626, but left out for the
awaitMetadataUpdate()code path. This patch fixes the problem bypassing through a flag to
awaitMetadataUpdate()to indicate whetherwakeups should be disabled. This is used to disable wakeups in the
context of asynchronous offset commits. All other uses leave wakeups
enabled.
Reviewers: Nikita Shupletsov nikita@shupletsov.ca, Lianet Magrans
lmagrans@confluent.io