-
Notifications
You must be signed in to change notification settings - Fork 14.9k
MINOR : Tolerate GroupIdNotFoundException in consumer when leaving a group. #21239
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 3 commits
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 |
|---|---|---|
|
|
@@ -432,6 +432,23 @@ private void onErrorResponse(final R response, final long currentTimeMs) { | |
| "subscribe. " + errorMessage)); | ||
| break; | ||
|
|
||
| case GROUP_ID_NOT_FOUND: | ||
| // If the group doesn't exist (e.g., member never joined due to InvalidTopicException), | ||
| // GROUP_ID_NOT_FOUND should be ignored - the leave is effectively complete. | ||
| // When a leave heartbeat (epoch=-1) is sent, the state transitions synchronously | ||
| // from LEAVING to UNSUBSCRIBED in onHeartbeatRequestGenerated() before the request is sent. | ||
| if (membershipManager().state() == MemberState.UNSUBSCRIBED) { | ||
| logger.info("{} received GROUP_ID_NOT_FOUND for group {} while unsubscribed. " + | ||
| "Not treating as fatal since consumer is leaving group.", | ||
| heartbeatRequestName(), membershipManager().groupId()); | ||
| membershipManager().onHeartbeatRequestSkipped(); | ||
| } else { | ||
| // Else, this is a fatal error, we should throw it and transition to fatal state. | ||
| logger.error("{} failed due to unexpected error {}: {}", | ||
|
||
| heartbeatRequestName(), error, errorMessage); | ||
| handleFatalFailure(error.exception(errorMessage)); | ||
| } | ||
| break; | ||
|
Member
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. nit: Let's have a blank line following the break to match the other cases in this switch. |
||
| default: | ||
| if (!handleSpecificExceptionInResponse(response, currentTimeMs)) { | ||
| // If the manager receives an unknown error - there could be a bug in the code or a new error code | ||
|
|
||
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.
I would remove the "Not treating as fatal since consumer is leaving group.". There's no sense in using "fatal" in an information log line. This is nothing to worry about in the slightest.