KAFKA-16106: revert classic state transitions if deletion fails #16511
+101
−37
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
jeffkbkim
force-pushed
the
KAFKA-16106-classic-group
branch
from
866ce8b
to
b73af9c
Compare
dongnuo123
reviewed
Jul 9, 2024
| @@ -575,18 +581,32 @@ public CoordinatorResult<OffsetDeleteResponseData, CoordinatorRecord> deleteOffs | |||
| public CoordinatorResult<Void, CoordinatorRecord> cleanupGroupMetadata() { | |||
| long startMs = time.milliseconds(); | |||
| List<CoordinatorRecord> records = new ArrayList<>(); | |||
| AtomicInteger deletedClassicGroupCount = new AtomicInteger(0); | |||
There was a problem hiding this comment.
Just to confirm, is the AtomicInteger used to match the following forEach loop?
There was a problem hiding this comment.
If you mean by using it to conform to the lambda expression, yes
dongnuo123
reviewed
Jul 9, 2024
|
|
||
| when(groupMetadataManager.groupIds()).thenReturn(mkSet("group-id", "other-group-id")); | ||
| when(offsetMetadataManager.cleanupExpiredOffsets(eq("group-id"), eq(new ArrayList<>()))).thenReturn(true); | ||
| when(groupMetadataManager.maybeDeleteGroup(eq("group-id"), eq(new ArrayList<>()))).thenReturn(true); |
There was a problem hiding this comment.
Should we add something to the record list and assert it's non null later?
There was a problem hiding this comment.
the test above, testCleanupGroupMetadata (https://github.com/apache/kafka/pull/16511/files/b73af9c786d4ad29259d0bfeb7c16db3324eff4b#diff-3a0b9cad0253e0f6d4665efd0d6f7efd5bd5dd96d3ba31005cab06fa728aad8fR990)
tests that the records we add are reflected. would this be sufficient?
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.
An expire-group-metadata operation generates tombstone records, updates the groups state and decrements group size counters, then performs a write to the log. If there is a __consumer_offsets partition reassignment, for instance, this operation fails. The groups state is reverted to an earlier snapshot but classic group size counters are not. This begins an inconsistency between the metrics and the actual groups size. This applies to all unsuccessful write operations that alter the classic group state.
However, some operations that alter the classic group state does not produce records. This means that we cannot rely on timeline data structures as we do for consumer group states.
Committer Checklist (excluded from commit message)