lucasbru commented on code in PR #22676:
URL: https://github.com/apache/kafka/pull/22676#discussion_r3490172839
##########
group-coordinator/src/test/java/org/apache/kafka/coordinator/group/GroupCoordinatorServiceTopologyDescriptionTest.java:
##########
@@ -951,6 +952,32 @@ public void
testCleanupCycleClearsStoredEpochOnPluginSuccess() {
verify(runtime,
times(1)).scheduleWriteOperation(eq("clear-stored-topology-epoch"),
eq(GROUP_TP), any());
}
+ @Test
+ public void testCleanupCycleBatchesClearWritesPerPartition() {
+ // Two eligible groups land on the same partition's eligibility read —
they must trigger
+ // exactly one scheduleWriteOperation carrying both conditional clears
(dajac's per-shard
Review Comment:
Can we drop the reviewer attribution here? The comment should explain what
the test checks, not where the idea came from.
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupCoordinatorShard.java:
##########
@@ -1014,7 +1015,7 @@ public Map<String, Integer>
listStreamsGroupsNeedingTopologyCleanup(long committ
if (!streamsGroup.isEmpty(committedOffset)) continue;
int storedEpoch =
streamsGroup.storedDescriptionTopologyEpoch(committedOffset);
Review Comment:
This PR still has `deferStreamsGroupTombstoneForPluginCleanup` as a static
method in the shard — @dajac asked to put this per-group logic on
`shouldExpire` instead.
Suggestion: add `config` as a parameter —
`shouldExpire(GroupCoordinatorConfig config)` — so `StreamsGroup` overrides it
to return `false` when the plugin is configured and
`storedDescriptionTopologyEpoch != -1`. Other group types ignore the parameter
and keep their current behaviour. That removes
`deferStreamsGroupTombstoneForPluginCleanup` entirely and the call site in
`cleanupGroupMetadata` becomes a plain `if (!group.shouldExpire(config))
return;`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
