KAFKA-19891: Bump group epoch when member regex subscription transitions from non-empty to empty #21013
Conversation
…ons from non-empty to empty This patch fixes an issue in where was incorrectly set to when a member’s regex subscription transitions from non-empty to empty. Because does not trigger a group epoch bump in , the group metadata could remain stale. The fix updates the method to return in this case, ensuring the group epoch is correctly incremented. The patch also updates tests that relied on the previous behavior and were failing due to the corrected epoch bump logic.
squah-confluent
left a comment
squah-confluent
left a comment
There was a problem hiding this comment.
Thanks for the patch! I don't think the fix is in the correct place.
|
|
||
| if (isNotEmpty(oldSubscribedTopicRegex) && group.numSubscribedMembers(oldSubscribedTopicRegex) != 0) { | ||
| updateRegularExpressionsResult = UpdateRegularExpressionsResult.REGEX_UPDATED_AND_RESOLVED; | ||
| } |
There was a problem hiding this comment.
We're changing the return value in the wrong branch of the code. We want the branch where newSubscribedTopicRegex is empty.
There was a problem hiding this comment.
does this mean the regex itself is empty?
There was a problem hiding this comment.
and not the subscribers counts is empty for the new regex?
There was a problem hiding this comment.
Yes. When we trigger a refresh we do not want to bump the group epoch until the refresh is complete. The refresh will bump the group epoch for us once it has mapped the regex to a list of topics.
There was a problem hiding this comment.
got it, will make the changes accordingly
| mkTopicAssignment(fooTopicId, 3, 4, 5), | ||
| mkTopicAssignment(barTopicId, 2))) |
There was a problem hiding this comment.
Could we avoid changing the formatting of unrelated tests?
There was a problem hiding this comment.
sure, correcting this
…cRegex is empty This patch corrects the return value in for the case where a member's becomes empty. Previously, the branch handling this condition returned , which does not trigger a group epoch bump during . As a result, the coordinator could retain stale group metadata when a member unsubscribed from all regex-based topics. The fix updates the method to return specifically when the new subscribed regex string is empty, ensuring that the coordinator clears the member's assignment and correctly bumps the group epoch.
…ting regex empty-branch logic This commit reverts test modifications that were made under the earlier, incorrect assumption about the behavior when becomes empty. Now that the fix applies to the correct branch, the tests no longer need their previous adjustments, and their original expectations are valid again.
…cRegex is empty This patch corrects the return value in for the case where a member's becomes empty. Previously, the branch handling this condition returned , which does not trigger a group epoch bump during . As a result, the coordinator could retain stale group metadata when a member unsubscribed from all regex-based topics. The fix updates the method to return REGEX_UPDATED_AND_RESOLVED.
|
@AntonVasant Could you please add a unit test to cover the change? |
3 participants
This PR fixes an issue in GroupMetadataManager#maybeUpdateRegularExpressions where a member’s regex subscription transition from non-empty → empty did not trigger a group epoch bump.
The method previously returned REGEX_UPDATED, which does not cause consumerGroupHeartbeat to increment the group epoch.
Fix
The patch updates the logic to return:
REGEX_UPDATED_AND_RESOLVED
when:
the updated regex subscription text is empty, and
the previous subscription was non-empty.
This ensures that consumerGroupHeartbeat correctly bumps the group epoch, keeping the group metadata consistent.
Tests
Several tests were updated to align with the corrected behavior.
Tests that previously expected no epoch bump were failing, and have now been adjusted to expect the new, correct logic.
JIRA
https://issues.apache.org/jira/browse/KAFKA-19891
Impact
Fixes coordinator state correctness for regex-subscribing consumer groups
Ensures group epoch bumps happen for all relevant subscription transitions
Backward compatible
No public API changes