KAFKA-18854:Move DynamicConfig to server module #19019

gongxuanzhang · 2025-02-24T08:43:53Z

move dynamicConfig to server module.
delete DynamicConfig.scala

frankvicky

Hi @gongxuanzhang
Thanks for the patch.
Could you please fix the build error?
I could pass build without test on my local, so you may need to do further check in your environment.

chia7712 · 2025-02-24T10:01:46Z

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

            // This indicates whether unreleased MetadataVersions should be enabled on this node.
            .defineInternal(UNSTABLE_FEATURE_VERSIONS_ENABLE_CONFIG, BOOLEAN, false, HIGH);
+
+    public static final Set<String> RECONFIGURABLE_CONFIGS = Set.of(


Could you please move it to DynamicConfig?

chia7712

@gongxuanzhang please remove DynamicConfig.scala from this PR

frankvicky

Thanks for the patch.
I have left some comments. PTAL

frankvicky · 2025-02-25T07:50:05Z

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

+            for (Map.Entry<String, ConfigDef.ConfigKey> entry :
+                    AbstractKafkaConfig.CONFIG_DEF.configKeys().entrySet()) {
+                String configName = entry.getKey();
+                if (ALL_DYNAMIC_CONFIGS.contains(configName)) {
+                    configs.define(entry.getValue());
+                }
+            }


nit:

AbstractKafkaConfig.CONFIG_DEF.configKeys().forEach((configName, value) -> { if (ALL_DYNAMIC_CONFIGS.contains(configName)) { configs.define(value); } });

you are right

frankvicky · 2025-02-25T07:58:19Z

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

+        Set<String> propKeys = new HashSet<>();
+        for (Object key : props.keySet()) {
+            propKeys.add((String) key);
+        }


Does this block can move into the if condition?

something like:

if (!customPropsAllowed) { Set<String> names = configDef.names(); Set<String> propKeys = new HashSet<>(); for (Object key : props.keySet()) { propKeys.add((String) key); } Set<String> unknownKeys = new HashSet<>(propKeys); unknownKeys.removeAll(names); if (!unknownKeys.isEmpty()) { throw new IllegalArgumentException("Unknown Dynamic Configuration: " + unknownKeys); } } Properties propResolved = resolveVariableConfigs(props); // Validate Values return configDef.parse(propResolved);

In the original scala code, this code was cast.I wrote that on purpose

gongxuanzhang · 2025-02-26T01:34:14Z

@chia7712 PTAL

chia7712 · 2025-02-27T10:08:22Z

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

  private[server] val DynamicProducerStateManagerConfig = Set(TransactionLogConfig.PRODUCER_ID_EXPIRATION_MS_CONFIG, TransactionLogConfig.TRANSACTION_PARTITION_VERIFICATION_ENABLE_CONFIG)

-  val AllDynamicConfigs = DynamicSecurityConfigs ++
-    LogCleaner.ReconfigurableConfigs ++


Could you please move ReconfigurableConfigs to CleanerConfig?

@chia7712 PTAL

chia7712 · 2025-03-02T19:31:47Z

clients/src/main/java/org/apache/kafka/common/config/SaslConfigs.java

    public static final String SASL_OAUTHBEARER_HEADER_URLENCODE_DOC = "The (optional) setting to enable the OAuth client to URL-encode the client_id and client_secret in the authorization header"
            + " in accordance with RFC6749, see <a href=\"https://datatracker.ietf.org/doc/html/rfc6749#section-2.3.1\">here</a> for more details. The default value is set to 'false' for backward compatibility";
+
+    public static final Set<String> RECONFIGURABLE_CONFIGS = Set.of(


SaslConfigs.java is public, so maybe we can move this set to DynamicConfig?

chia7712 · 2025-03-02T19:31:51Z

clients/src/main/java/org/apache/kafka/common/config/SslConfigs.java

            SslConfigs.SSL_KEYSTORE_KEY_CONFIG,
            SslConfigs.SSL_TRUSTSTORE_CERTIFICATES_CONFIG);
+
+    public static final Set<String> DYNAMIC_LISTENER_CONFIGS = Set.of(


chia7712 · 2025-03-02T23:33:02Z

it seems AllDynamicConfigs causes trouble on migration. Maybe we should move other "small" dynamic classes first

This PR is a umbrella of [KAFKA-18854. ](https://issues.apache.org/jira/browse/KAFKA-18854) The previous PR encountered some compatibility issues, so we decided to split it and proceed with the migration step by step. see #19019 Reviewers: PoAn Yang <payang@apache.org>, Chia-Ping Tsai <chia7712@gmail.com>

github-actions · 2025-06-04T03:46:33Z

This PR is being marked as stale since it has not had any activity in 90 days. If you
would like to keep this PR alive, please leave a comment asking for a review. If the PR has
merge conflicts, update it with the latest from the base branch.

If you are having difficulty finding a reviewer, please reach out on the [mailing list](https://kafka.apache.org/contact).

If this PR is no longer valid or desired, please feel free to close it. If no activity occurs in the next 30 days, it will be automatically closed.

github-actions · 2025-07-04T03:47:33Z

This PR has been closed since it has not had any activity in 120 days. If you feel like this
was a mistake, or you would like to continue working on it, please feel free to re-open the
PR and ask for a review.

move config

4bb291b

github-actions bot added triage PRs from the community core Kafka Broker storage Pull requests that target the storage module tiered-storage Related to the Tiered Storage feature transactions Transactions and EOS clients labels Feb 24, 2025

FrankYang0529 added the ci-approved label Feb 24, 2025

gongxuanzhang added 3 commits February 24, 2025 16:58

update to unmodifiable set

dde6632

update to unmodifiable set

89af1ba

Merge remote-tracking branch 'origin/update_dynamic' into update_dynamic

0256112

frankvicky reviewed Feb 24, 2025

View reviewed changes

chia7712 reviewed Feb 24, 2025

View reviewed changes

gongxuanzhang added 4 commits February 24, 2025 18:48

move server configs to dynamic config

f7e1e1b

update to unmodifiable set

951353d

update to unmodifiable set

e08acb7

update to unmodifiable set

205132c

github-actions bot removed the triage PRs from the community label Feb 25, 2025

gongxuanzhang added 3 commits February 25, 2025 12:42

add remain configs

4365505

remove ServerConfigs other configs

944518e

remove ServerConfigs other configs

73ef6af

chia7712 reviewed Feb 25, 2025

View reviewed changes

frankvicky reviewed Feb 25, 2025

View reviewed changes

gongxuanzhang added 4 commits February 25, 2025 16:07

remove dynamic config scala

8955fd9

Merge branch 'trunk' into update_dynamic

c0cdb44

fix conflict

245c657

fix conflict

d566e9b

chia7712 reviewed Feb 27, 2025

View reviewed changes

format

fd90393

chia7712 reviewed Mar 2, 2025

View reviewed changes

Merge branch 'trunk' into update_dynamic

96f7834

FrankYang0529 mentioned this pull request Apr 14, 2025

KAFKA-15853: Move static functions from KafkaConfig to AbstractKafkaConfig (wip) #19466

Closed

gongxuanzhang mentioned this pull request Apr 16, 2025

KAFKA-18854: remove DynamicConfig inner class #19487

Merged

github-actions bot added the stale Stale PRs label Jun 4, 2025

github-actions bot added the closed-stale PRs that were closed due to inactivity label Jul 4, 2025

github-actions bot closed this Jul 4, 2025

KAFKA-18854:Move DynamicConfig to server module #19019

KAFKA-18854:Move DynamicConfig to server module #19019

Uh oh!

Conversation

gongxuanzhang commented Feb 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

frankvicky left a comment

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!

frankvicky 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!

gongxuanzhang commented Feb 26, 2025

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 commented Mar 2, 2025

Uh oh!

github-actions bot commented Jun 4, 2025

Uh oh!

github-actions bot commented Jul 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gongxuanzhang commented Feb 24, 2025 •

edited

Loading