Suppress JoinGroup V4+ response error log when memberId is empty #860
Conversation
This file contains 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
This allows us to remove the special case for ApiVersions from the Connection while also making the member_id_required suppression be version aware, as we should only disable it for V4 when memberId is empty.
We suppress response errors for later versions, as the broker may not understand those versions and we should downgrade as a normal part of the protocol negotiation, but if they don't understand V0 we have no further recourse and should log the error.
tulios
approved these changes
Sep 9, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The first time a consumer joins the group, the broker will respond with an error telling us that we need to use a memberId that is returned together with the error. We catch this error and re-issue the request with the new memberId. However, since is is a protocol error, we would log an error that would cause some users to think that something was wrong, even though this is the normal operation when joining the group.
This refactoring moves the decision of whether or not to log the error response into the protocol itself, which allows us to get rid of the special case for
ApiVersionsinConnection.send. It also allows us to decide whether or not to log based on the request or response itself, as well as the version of the request. This is relevant in this case, as we don't want to log if there's an error in response toJoinGroupV4and the memberId is empty, but we do want to log for olderJoinGroupversions or when the memberId is in fact set already.Fixes #856