KAFKA-20110 Fix generated doc and Javadoc for dynamic configs by see-quick · Pull Request #21384 · apache/kafka

see-quick · 2026-02-02T12:01:25Z

This PR fixes a problem with a few broker properties. More can be found
https://issues.apache.org/jira/browse/KAFKA-20110.

Reviewers: Chia-Ping Tsai chia7712@gmail.com

chia7712 · 2026-02-02T12:35:25Z

is this duplicate to #21131?

see-quick · 2026-02-02T12:47:56Z

is this duplicate to #21131?

Hmmm, partially (as it fixes the isReadOnly)? However, my PR also adds to PER_BROKER_CONFIGS and AbstractKafkaConfig.CONFIG_DEFto display it on the Kafka site within the broker [1] (currently it's not there).

Does #21131 fix the website issue? 🤔

[1] - https://kafka.apache.org/41/configuration/

chia7712 · 2026-02-04T20:35:10Z

server-common/src/main/java/org/apache/kafka/server/config/QuotaConfig.java

-                .define(QuotaConfig.REPLICA_ALTER_LOG_DIRS_IO_MAX_BYTES_PER_SECOND_CONFIG, ConfigDef.Type.LONG,
-                        QuotaConfig.QUOTA_BYTES_PER_SECOND_DEFAULT, ConfigDef.Range.atLeast(0),
-                        ConfigDef.Importance.MEDIUM, QuotaConfig.REPLICA_ALTER_LOG_DIRS_IO_MAX_BYTES_PER_SECOND_DOC);
+        return BROKER_QUOTA_CONFIG_DEF;


A new ConfigDef is required for each call because the one from brokerQuotaConfigs is subject to modification

Hmm, I think problem is more deeper ... In the, we can see

kafka/server/src/main/java/org/apache/kafka/server/config/DynamicConfig.java

Lines 36 to 43 in 9d5bbf5

ConfigDef configs = QuotaConfig.brokerQuotaConfigs();

// Filter and define all dynamic configurations

AbstractKafkaConfig.CONFIG_DEF.configKeys().forEach((name, value) -> {

if (DynamicBrokerConfig.ALL_DYNAMIC_CONFIGS.contains(name)) {

configs.define(value);

}

});

BROKER_CONFIGS = configs;

I don't why there is QuotaConfig.brokerQuotaConfigs(); as other configs is loaded from ALL_DYNAMIC_CONFIGS. Maybe that's because someone forgot to include it inside the ALL_DYNAMIC_CONFIGS backthen...

Anyway, I have removed/replaced ConfigDef configs = QuotaConfig.brokerQuotaConfigs() as it is already included and defined in the ALL_DYNAMIC_CONFIGS, and it seems to work. Also checked the website and showing the right values.

Let me know what you think.

Maybe that's because someone forgot to include it inside the ALL_DYNAMIC_CONFIGS backthen...

yes, that's indeed an issue, and #21131 tries to fix it :)

Yeah, but still this line of code.

kafka/server/src/main/java/org/apache/kafka/server/config/DynamicConfig.java

Line 36 in 9d5bbf5

ConfigDef configs = QuotaConfig.brokerQuotaConfigs();

I don't get why we have just 3 properties, which are configured only dynamically. Why not also dual-mode (i.e., static || dynamic)? Like for instance num.io.threads works fine in dual-mode but probably I should read a KIP, which introduced those properties... but it's bit weird.

At least I think we should add comment here that in such line DynamicConfig.java#L36 those configs are dynamic-only and not dual-mode as others.

I don't get why we have just 3 properties, which are configured only dynamically. Why not also dual-mode (i.e., static || dynamic)? Like for instance num.io.threads works fine in dual-mode but probably I should read a KIP, which introduced those properties... but it's bit weird.

You might want to check out the KIP-1051, which address the exact issue

At least I think we should add comment here that in such line DynamicConfig.java#L36 those configs are dynamic-only and not dual-mode as others.

Actually, the comment is already there. see https://github.com/apache/kafka/blob/trunk/server/src/main/java/org/apache/kafka/server/config/DynamicConfig.java#L27

You might want to check out the KIP-1051, which address the exact issue

So it's still under-discussion? I am bit a lost in the JIRAs like there is always at least one JIRA for issue I found :D . Because that KIP just wants to move such configuration from DynamicConfig to KafkaConfig which is not a case.

Actually, the comment is already there. see https://github.com/apache/kafka/blob/trunk/server/src/main/java/org/apache/kafka/server/config/DynamicConfig.java#L27

Class used to hold dynamic configs. These are configs which have no physical manifestation in the server.properties and can only be set dynamically.

used to (past)? It's still holding dynamic-only configs (i.e., QuotaConfig.brokerQuotaConfigs();)? And then you have also dual-mode configs which are filtered from the AbstractKafkaConfig i.e., :

// Filter and define all dynamic configurations AbstractKafkaConfig.CONFIG_DEF.configKeys().forEach((name, value) -> { if (DynamicBrokerConfig.ALL_DYNAMIC_CONFIGS.contains(name)) { configs.define(value); } });

I would probably fix that doc something like:

Holds dynamic configs, including both dynamic-only configs (e.g., quotas) and dual-mode configs that can be set statically or dynamically.

I think it would help with overall understanding.

server/src/main/java/org/apache/kafka/server/config/AbstractKafkaConfig.java

chia7712 · 2026-02-10T00:12:13Z

@see-quick could you refine the title?

Signed-off-by: see-quick <maros.orsak159@gmail.com> # Conflicts: # server/src/main/java/org/apache/kafka/server/config/DynamicBrokerConfig.java

Signed-off-by: see-quick <maros.orsak159@gmail.com>

… + refine Signed-off-by: see-quick <maros.orsak159@gmail.com>

Signed-off-by: see-quick <maros.orsak159@gmail.com>

chia7712 · 2026-02-11T10:47:32Z

server/src/main/java/org/apache/kafka/server/config/DynamicBrokerConfig.java

                ReplicationConfigs.FOLLOWER_FETCH_LAST_TIERED_OFFSET_ENABLE_CONFIG);
    }
+
+    public static class DynamicQuotaConfig {


Is it a kind of decoration?

Yeah, I think it make code more readable at least for me. Like I didn't want to re-write all these properties into Dynamic<class-name>.java:

public static final Set<String> ALL_DYNAMIC_CONFIGS = Stream.of( DYNAMIC_SECURITY_CONFIGS, LogCleaner.RECONFIGURABLE_CONFIGS, DynamicLogConfig.RECONFIGURABLE_CONFIGS, DynamicThreadPool.RECONFIGURABLE_CONFIGS, List.of(MetricConfigs.METRIC_REPORTER_CLASSES_CONFIG), DynamicListenerConfig.RECONFIGURABLE_CONFIGS, SocketServer.RECONFIGURABLE_CONFIGS, DYNAMIC_PRODUCER_STATE_MANAGER_CONFIGS, DynamicRemoteLogConfig.RECONFIGURABLE_CONFIGS, DynamicReplicationConfig.RECONFIGURABLE_CONFIGS, List.of(AbstractConfig.CONFIG_PROVIDERS_CONFIG), GroupCoordinatorConfig.RECONFIGURABLE_CONFIGS, DynamicQuotaConfig.RECONFIGURABLE_CONFIGS, ShareCoordinatorConfig.RECONFIGURABLE_CONFIGS) .flatMap(Collection::stream) .collect(Collectors.toUnmodifiableSet());

because you can see that some of them using that pattern DynamicThreadPool.RECONFIGURABLE_CONFIGS or DynamicLogConfig.RECONFIGURABLE_CONFIGS. Like for instance when seeing QuotaConfig.BROKER_QUOTA_CONFIGS it doesn't give me a feeling that it's dynamic configuration I have to go deep into the code to see it actually.... (maybe I am just too picky sorry :D)

chia7712 · 2026-02-11T10:48:02Z

core/src/main/scala/kafka/server/KafkaConfig.scala

  def main(args: Array[String]): Unit = {
-    System.out.println(configDef.toHtml(4, (config: String) => "brokerconfigs_" + config,
+    val combined = new ConfigDef(configDef)
+    QuotaConfig.brokerQuotaConfigs().configKeys().forEach((_, v) => combined.define(v))


would you mind adding comment?

Yeah that would be good idea, I have added comment there.

Signed-off-by: see-quick <maros.orsak159@gmail.com>

chia7712

LGTM

github-actions bot added the triage PRs from the community label Feb 2, 2026

see-quick changed the title ~~[KAFKA-20110] Fix DynamicQuotaConfig~~ KAFKA-20110 Fix DynamicQuotaConfig Feb 2, 2026

github-actions bot added core Kafka Broker small Small PRs labels Feb 2, 2026

chia7712 added the ci-approved label Feb 4, 2026

chia7712 reviewed Feb 4, 2026

View reviewed changes

github-actions bot removed the triage PRs from the community label Feb 5, 2026

chia7712 reviewed Feb 5, 2026

View reviewed changes

server/src/main/java/org/apache/kafka/server/config/AbstractKafkaConfig.java Outdated Show resolved Hide resolved

see-quick changed the title ~~KAFKA-20110 Fix DynamicQuotaConfig~~ KAFKA-20110 Fix generated doc and Javadoc for dynamic configs Feb 10, 2026

see-quick added 4 commits February 10, 2026 10:42

[KAFKA-20110] Fix DynamicQuotaConfig

2422e85

Signed-off-by: see-quick <maros.orsak159@gmail.com> # Conflicts: # server/src/main/java/org/apache/kafka/server/config/DynamicBrokerConfig.java

fix the stuff

19e320e

Signed-off-by: see-quick <maros.orsak159@gmail.com>

update KafkaConfig to also generate dynamic-only configs into website…

f3e0032

… + refine Signed-off-by: see-quick <maros.orsak159@gmail.com>

rebase

36830ea

Signed-off-by: see-quick <maros.orsak159@gmail.com>

see-quick force-pushed the KAFKA-20110 branch from 6ac0660 to 36830ea Compare February 10, 2026 09:47

see-quick added 2 commits February 10, 2026 10:49

lining tweak

57e12f6

Signed-off-by: see-quick <maros.orsak159@gmail.com>

make it cluster-level not per-broker

01480f4

Signed-off-by: see-quick <maros.orsak159@gmail.com>

chia7712 reviewed Feb 11, 2026

View reviewed changes

update to comment

f69c1d5

Signed-off-by: see-quick <maros.orsak159@gmail.com>

chia7712 approved these changes Feb 12, 2026

View reviewed changes

chia7712 merged commit 5d85616 into apache:trunk Feb 12, 2026
24 checks passed

	ConfigDef configs = QuotaConfig.brokerQuotaConfigs();
	// Filter and define all dynamic configurations
	AbstractKafkaConfig.CONFIG_DEF.configKeys().forEach((name, value) -> {
	if (DynamicBrokerConfig.ALL_DYNAMIC_CONFIGS.contains(name)) {
	configs.define(value);
	}
	});
	BROKER_CONFIGS = configs;

Conversation

see-quick commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chia7712 commented Feb 2, 2026

Uh oh!

see-quick commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

chia7712 commented Feb 10, 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!

chia7712 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

see-quick commented Feb 2, 2026 •

edited

Loading

see-quick commented Feb 2, 2026 •

edited

Loading