KAFKA-20297: Cleanup `org.apache.kafka.common.utils.CollectionUtils` by unknowntpo · Pull Request #21818 · apache/kafka

unknowntpo · 2026-03-19T10:12:58Z

This PR removes CollectionUtils entirely (KAFKA-20297) by replacing
all usages with alternatives.

Reviewers: Chia-Ping Tsai chia7712@gmail.com

chia7712 · 2026-03-19T17:18:51Z

@unknowntpo would you mind rebasing code?

unknowntpo · 2026-03-21T00:32:37Z

@unknowntpo would you mind rebasing code?

Ok, rebased.

unknowntpo · 2026-03-21T08:37:19Z

and ci errors are fixed.

chia7712 · 2026-03-21T19:27:36Z

clients/src/main/java/org/apache/kafka/clients/consumer/StickyAssignor.java

        return new MemberData(partitions, generation);
    }
+
+    private static Map<String, List<Integer>> groupPartitionsByTopic(Collection<TopicPartition> partitions) {


Could you inline it?

chia7712 · 2026-03-21T19:30:33Z

...ava/org/apache/kafka/common/security/oauthbearer/OAuthBearerExtensionsValidatorCallback.java

@@ -90,6 +89,12 @@ public Map<String, String> ignoredExtensions() {
        return Collections.unmodifiableMap(subtractMap(subtractMap(inputExtensions.map(), invalidExtensions), validatedExtensions));


The original logic will create map repeatedly. Could you use for-loop to streamline it?

public Map<String, String> ignoredExtensions() { Map<String, String> ignored = new HashMap<>(); for (Map.Entry<String, String> entry : inputExtensions.map().entrySet()) { String key = entry.getKey(); if (!invalidExtensions.containsKey(key) && !validatedExtensions.containsKey(key)) { ignored.put(key, entry.getValue()); } } return Collections.unmodifiableMap(ignored); }

I thought the subtractMap is more Functional Programming style, more declarative, and this path is unlikely a hot path, which is not suitable for repeating memory allocation, but your approach is okay for me, I'll change it.

chia7712 · 2026-03-21T19:36:46Z

...nts/src/test/java/org/apache/kafka/clients/admin/internals/DescribeProducersHandlerTest.java

        return new DescribeProducersResponse(response);
    }

+    private static Map<String, Map<Integer, PartitionResponse>> groupPartitionDataByTopic(


we don't use such complex structure in tests actually. Maybe we could use describeProducersResponse(TopicPartition partition, PartitionResponse partitionResponse) instead

Done, since caller only pass single TopicPartition and PartitionResponse, so the Map input was unnecessary.

chia7712 · 2026-03-21T19:41:35Z

...ts/src/test/java/org/apache/kafka/clients/consumer/internals/AbstractStickyAssignorTest.java


-                Map<String, List<Integer>> map = CollectionUtils.groupPartitionsByTopic(partitions);
-                Map<String, List<Integer>> otherMap = CollectionUtils.groupPartitionsByTopic(otherPartitions);
+                Map<String, List<Integer>> map = groupPartitionsByTopic(partitions);


we could streamline the test.

Set<String> otherTopics = otherPartitions.stream() .map(TopicPartition::topic) .collect(Collectors.toSet()); for (TopicPartition tp : partitions) { assertFalse(otherTopics.contains(tp.topic()), "Error: Some partitions can be moved..."); }

Okay, done, this is simple because we only use the topic name, lesson learned.

…from CollectionUtils Replace all callers with idiomatic alternatives: - DescribeProducersHandler: add mapKey:true to DescribeProducersRequest.json and use find() on TopicRequestCollection - ListOffsetsHandler: use HashMap+computeIfAbsent Also fix Scala test callers affected by the DescribeProducersRequest mapKey change.

Inline subtractMap as a private helper in OAuthBearerExtensionsValidatorCallback, then delete the now-empty CollectionUtils class and its test.

…ethod Extract repeated stream-based grouping logic into a private groupPartitionsByTopic helper in StickyAssignor and AbstractStickyAssignorTest.

…ucersHandlerTest

…Partition and PartitionResponse

…nsionsValidatorCallback

…ignorTest

…sHandlerTest

…allback

chia7712

@unknowntpo thanks for this update

chia7712 · 2026-03-29T01:56:08Z

clients/src/main/java/org/apache/kafka/clients/consumer/StickyAssignor.java

        Struct struct = new Struct(STICKY_ASSIGNOR_USER_DATA_V1);
        List<Struct> topicAssignments = new ArrayList<>();
-        for (Map.Entry<String, List<Integer>> topicEntry : CollectionUtils.groupPartitionsByTopic(memberData.partitions).entrySet()) {
+        Map<String, List<Integer>> partitionsByTopic = memberData.partitions.stream()


I'm a bit concerned about the performance. Would you mind using for loop instead?

var partitionsByTopic = new HashMap<String, List<Integer>>(); for (TopicPartition tp : memberData.partitions) { partitionsByTopic.computeIfAbsent(tp.topic(), t -> new ArrayList<>()).add(tp.partition()); }

…icPartitionAssignment

github-actions bot added triage PRs from the community consumer clients labels Mar 19, 2026

unknowntpo force-pushed the cleanup-common-utils-20297 branch from 7e2ef79 to 10b13ba Compare March 21, 2026 00:30

github-actions bot added the core Kafka Broker label Mar 21, 2026

unknowntpo force-pushed the cleanup-common-utils-20297 branch from 82b9572 to 4aea5f0 Compare March 21, 2026 05:07

chia7712 reviewed Mar 21, 2026

View reviewed changes

github-actions bot removed the triage PRs from the community label Mar 22, 2026

unknowntpo added 10 commits March 26, 2026 07:37

KAFKA-20297: Inline subtractMap and delete CollectionUtils

3c488a2

Inline subtractMap as a private helper in OAuthBearerExtensionsValidatorCallback, then delete the now-empty CollectionUtils class and its test.

KAFKA-20297: Simplify StickyAssignor partition grouping with helper m…

538c897

…ethod Extract repeated stream-based grouping logic into a private groupPartitionsByTopic helper in StickyAssignor and AbstractStickyAssignorTest.

KAFKA-20297: Extract groupPartitionDataByTopic helper in DescribeProd…

53ed6fe

…ucersHandlerTest

refactor: simplify input of describeProducersResponse to single Topic…

f465e64

…Partition and PartitionResponse

refactor: inline groupPartitionsByTopic in StickyAssignor

dcf6bc5

refactor: simplify ignoredExtensions with for-loop in OAuthBearerExte…

73e27ee

…nsionsValidatorCallback

refactor: simplify partition-topic overlap check in AbstractStickyAss…

678d200

…ignorTest

refactor: remove unused groupPartitionDataByTopic in DescribeProducer…

07ae5c0

…sHandlerTest

style: fix import group blank line in OAuthBearerExtensionsValidatorC…

ceb7923

…allback

unknowntpo force-pushed the cleanup-common-utils-20297 branch from 05ad35d to ceb7923 Compare March 26, 2026 00:14

chia7712 reviewed Mar 29, 2026

View reviewed changes

refactor: replace stream with for-loop in StickyAssignor.serializeTop…

41640f9

…icPartitionAssignment

chia7712 approved these changes Mar 30, 2026

View reviewed changes

chia7712 merged commit a1c155e into apache:trunk Mar 30, 2026
24 checks passed

unknowntpo deleted the cleanup-common-utils-20297 branch March 30, 2026 10:31

		@@ -90,6 +89,12 @@ public Map<String, String> ignoredExtensions() {
		return Collections.unmodifiableMap(subtractMap(subtractMap(inputExtensions.map(), invalidExtensions), validatedExtensions));

Conversation

unknowntpo commented Mar 19, 2026 • edited by chia7712 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chia7712 commented Mar 19, 2026

Uh oh!

unknowntpo commented Mar 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

unknowntpo commented Mar 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

unknowntpo Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chia7712 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

unknowntpo commented Mar 19, 2026 •

edited by chia7712

Loading

unknowntpo commented Mar 21, 2026 •

edited

Loading

unknowntpo Mar 26, 2026 •

edited

Loading