KAFKA-20790: add AssignmentConfigs interface by gabriellefu · Pull Request #23092 · apache/kafka

gabriellefu · 2026-08-05T20:44:27Z

Adding AssignmentConfigs a public API for KIP-1357.

Reviewers: Sean Quah squah@confluent.io, Matthias J. Sax
matthias@confluent.io

squah-confluent

Thanks for updating the PR!

squah-confluent · 2026-08-10T12:51:50Z

+    /**
+     * The configs used for a group that has none of them set.
+     */
+    public static final AssignmentConfigsImpl DEFAULT = new AssignmentConfigsImpl(0, List.of());


Where do we plan to define the default num.standby.replicas and rack.aware.assignment.tags? Can we have a single source of truth for the defaults?

i added AssignmentConfigsImpl.DEFAULT which is the default value of all the single config in the configs(), and also added some helper to construct AssignmentConfigsImpl with all default value except certain configs

squah-confluent · 2026-08-10T12:56:47Z


        GroupAssignment result = assignor.assign(
-            new GroupSpecImpl(members, mkMap(mkEntry(NUM_STANDBY_REPLICAS_CONFIG, String.valueOf(numStandbyReplicas)))),
+            new GroupSpecImpl(members, new AssignmentConfigsImpl(numStandbyReplicas, List.of())),


These constructor calls are still going to be annoying to update when we add new configs.
We could leave them be or we could try to tidy things up some more, for example, by adding helper methods to construct an AssignmentConfigsImpl with default values but a specific num.standby.replicas or rack.aware.assignment.tags (eg. a withNumStandbyReplicas)

(Only applies to calls where we would want to take the defaults for future configs.)

I have another pr is to avoid epoch bump when there's new config being introduced. So I was thinking about implement the default part maybe after this pr is merged? #23088

I can close the other pr and put the change in this pr too

hm I think the not-bumping logic belongs in separate PR. Maybe we could bring the constant definition forward? But I don't really mind as long as the final state is clean.

How do you imagine these new AssignmentConfigsImpl calls to look at the end of it all?

sounds good, i can do that. In the tests we can only pass the configs we actually care about , otherwise they will use there default value, for replicas is 0 and for tag is "".

gabriellefu · 2026-08-10T18:24:36Z

+    public static final AssignmentConfigsImpl DEFAULT = new AssignmentConfigsImpl(
+        GroupCoordinatorConfig.STREAMS_GROUP_NUM_STANDBY_REPLICAS_DEFAULT,
+        // The parsed form of STREAMS_GROUP_RACK_AWARE_ASSIGNMENT_TAGS_DEFAULT, which ConfigDef spells as "".
+        List.of()


the default of "rack.aware.assignment.tags" is "", and if we directly parse it into a List, it will give a [""] instead of a empty list [], so we still hardcode the default here

Hmmm... This still seems to be error prone? In the end, we should have a single source of truth... Should we rather use STREAMS_GROUP_RACK_AWARE_ASSIGNMENT_TAGS_DEFAULT and parse it into empty List? If the default changes, this code would update immediatly?

But on the other hand, it seem we are using DEFAULT only in tests, what make it somewhat questionable, if it's the right place to add it here?

we also use DEFAULT in fromMap(), if the config is a empty map, it returns DEFAULT otherwise it parse the config. But I agree with the STREAMS_GROUP_RACK_AWARE_ASSIGNMENT_TAGS_DEFAULT part I will update accordingly.

I have added a helper here to parse STREAMS_GROUP_RACK_AWARE_ASSIGNMENT_TAGS_DEFAULT

mjsax · 2026-08-11T00:12:17Z

+        // ConfigDef has already validated to be non-empty and free of surrounding whitespace.
+        String rackAwareAssignmentTags = configs.get(RACK_AWARE_ASSIGNMENT_TAGS_CONFIG);
+        return new AssignmentConfigsImpl(
+            Integer.parseInt(configs.get(NUM_STANDBY_REPLICAS_CONFIG)),


Don't we need null check here and translate it to 0 ?

In the original code, for replicas, there are two path, one is the whole map is empty, then it's ok to not having replicas set and here we return DEFAULT, other than that, the replicas should always be in the config

Ok. Seems we are scattered logic, making it hard to reason about this... No critical for 4.4 release, but wondering if we could do a follow up cleanup PR, streamlining this a little bit better to make it easier as a (human 🤣) reviewer to follow...

Maybe we should "unify" the raw Map<String, String> and AssignmentConfigsImpl somehow, to have a single object we use and pass around, an avoid converting the one into the other multiple times along the way, at different places.

I have a similar question here. Maybe we could leverage the default value?

String numStandby = configs.getOrDefault( NUM_STANDBY_REPLICAS_CONFIG, String.valueOf(GroupCoordinatorConfig.STREAMS_GROUP_NUM_STANDBY_REPLICAS_DEFAULT) ); return new AssignmentConfigsImpl( Integer.parseInt(numStandby), parseRackAwareAssignmentTags(rackAwareAssignmentTags) );

have updated in this following pr:#23165 thanks

mjsax · 2026-08-11T19:30:29Z

+     * parses a {@code LIST} configuration: an empty value is an empty list, not a list holding an empty string.
+     */
+    private static List<String> parseRackAwareAssignmentTags(String rackAwareAssignmentTags) {
+        return rackAwareAssignmentTags.isEmpty() ? List.of() : List.of(rackAwareAssignmentTags.split(","));


isEmpty() checks for length() == 0, right? What about " " (or similar). Do we need to to do rackAwareAssignmentTags.trim().isEmpty()` ?

the input of this will be the output of String.join(",", tags), in this case, there won't be " ". But I do feel like here the type has been a bit confusing, I will submit a follow up pr to clean up those things

squah-confluent

Thanks, looks good!

squah-confluent · 2026-08-12T10:01:31Z

+public class AssignmentConfigsImplTest {
+
+    @Test
+    void testDefaultHoldsTheDefaultOfEveryConfig() {


nit: I'd just call this testDefault.

squah-confluent · 2026-08-12T10:02:07Z

+    }
+
+    @Test
+    void testWithersLeaveTheOtherConfigsUntouched() {


nit: I'd just call this testWithers

have changed! thanks

Resolves conflicts with KAFKA-20719 (apache#22783): keep the taskOffsets benchmark plumbing from trunk and the AssignmentConfigs API from this branch; convert the new StickyTaskAssignorTest cases to AssignmentConfigsImpl. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

squah-confluent

Thanks!

Adding AssignmentConfigs a public API for KIP-1357. Reviewers: Sean Quah <squah@confluent.io>, Matthias J. Sax <matthias@confluent.io>

mjsax · 2026-08-12T19:24:17Z

Merged to trunk and cherry-picked to 4.4 branch.

add AssignmentConfigs interface

7a09a49

github-actions Bot added triage PRs from the community performance group-coordinator labels Aug 5, 2026

gabriellefu added 2 commits August 5, 2026 16:53

replias is not optional

bad3070

bug fix

f5324e2

squah-confluent reviewed Aug 6, 2026

View reviewed changes

Comment thread ...inator/src/main/java/org/apache/kafka/coordinator/group/streams/TargetAssignmentBuilder.java Outdated

Comment thread ...ator/src/test/java/org/apache/kafka/coordinator/group/streams/assignor/MockAssignorTest.java Outdated

Merge branch 'trunk' into assignmentconfig

63259d4

github-actions Bot removed the triage PRs from the community label Aug 7, 2026

gabriellefu added 3 commits August 7, 2026 12:51

Merge branch 'trunk' into assignmentconfig

74c97bc

added AssignmentConfigsImpl

2117499

don't broden the scope

5016b90

squah-confluent reviewed Aug 10, 2026

View reviewed changes

gabriellefu added 5 commits August 10, 2026 11:58

adding default

a1fae8b

either empty map or map must has replias in config()

0bf596c

clear redundant validation

fd18f5f

adding comment

3a90c24

change testutils

e9549a4

gabriellefu commented Aug 10, 2026

View reviewed changes

mjsax reviewed Aug 11, 2026

View reviewed changes

Comment thread ...p-coordinator/src/test/java/org/apache/kafka/coordinator/group/streams/MockTaskAssignor.java Outdated

update pr

347c5a7

mjsax reviewed Aug 11, 2026

View reviewed changes

mjsax added streams kip Requires or implements a KIP labels Aug 11, 2026

mjsax approved these changes Aug 11, 2026

View reviewed changes

gabriellefu added 2 commits August 11, 2026 15:57

Merge branch 'trunk' into assignmentconfig

5407888

resolve test fail

897f454

squah-confluent reviewed Aug 12, 2026

View reviewed changes

Merge branch 'trunk' into assignmentconfig

73c22c9

gabriellefu and others added 2 commits August 12, 2026 11:53

change name

261257d

squah-confluent approved these changes Aug 12, 2026

View reviewed changes

mjsax merged commit 89a6c9e into apache:trunk Aug 12, 2026
20 checks passed

mjsax pushed a commit that referenced this pull request Aug 12, 2026

KAFKA-20790: add AssignmentConfigs interface (#23092)

e6a6752

Adding AssignmentConfigs a public API for KIP-1357. Reviewers: Sean Quah <squah@confluent.io>, Matthias J. Sax <matthias@confluent.io>

Uh oh!

Conversation

gabriellefu commented Aug 5, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

squah-confluent 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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

squah-confluent 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!

Choose a reason for hiding this comment

Uh oh!

squah-confluent left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mjsax commented Aug 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gabriellefu commented Aug 5, 2026 •

edited by github-actions Bot

Loading