Skip to content

feat(inkless:consolidation): make fetch quota config dynamic#637

Merged
viktorsomogyi merged 1 commit into
mainfrom
jeqo/ts-consolidation-quota-dynamic
Jun 10, 2026
Merged

feat(inkless:consolidation): make fetch quota config dynamic#637
viktorsomogyi merged 1 commit into
mainfrom
jeqo/ts-consolidation-quota-dynamic

Conversation

@jeqo

@jeqo jeqo commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Make diskless.consolidation.fetch.rate.limit.bytes.per.second a dynamic broker config, aligned with the leader/follower replication throttle pattern:

  • Add to AllDynamicConfigs so KAdmin accepts it as a dynamic update
  • Move ReplicationQuotaManager ownership to QuotaManagers (seeded from static config on startup via QuotaFactory.instantiate)
  • Apply dynamic updates in BrokerConfigHandler.processConfigChanges, reading from currentKafkaConfig to pick up the merged static+dynamic value (avoids clobbering a static limit on unrelated config changes)
  • Relax atLeast(1) to atLeast(0) and document as dynamically updatable
  • Add DynamicConfigChangeUnitTest cases covering dynamic update and static-value-preserved-on-unrelated-change scenarios

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR makes the diskless consolidation fetch quota (diskless.consolidation.fetch.rate.limit.bytes.per.second) dynamically updatable at runtime, aligning it with the existing broker replication throttle update flow.

Changes:

  • Adds the consolidation fetch rate limit to the broker’s dynamic-config allowlist and documents it as dynamically updatable.
  • Moves consolidation quota manager ownership into QuotaFactory.QuotaManagers and updates ReplicaManager to use it.
  • Applies dynamic updates via BrokerConfigHandler, using currentKafkaConfig to preserve statically configured values across unrelated dynamic updates, and adds unit tests for both dynamic-update and static-preservation scenarios.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
server-common/src/main/java/org/apache/kafka/server/config/ServerConfigs.java Documents dynamic updatability and relaxes validation to allow 0 as a valid rate limit.
core/src/main/scala/kafka/server/DynamicBrokerConfig.scala Allows the consolidation fetch rate limit to be updated dynamically.
core/src/main/scala/kafka/server/ConfigHandler.scala Updates the consolidation quota from the merged (static + dynamic) currentKafkaConfig.
core/src/main/java/kafka/server/QuotaFactory.java Adds a dedicated consolidation ReplicationQuotaManager to QuotaManagers and seeds it from static config.
core/src/main/scala/kafka/server/ReplicaManager.scala Switches consolidation quota usage to the QuotaManagers-owned manager.
core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala Adds tests for dynamic update and “don’t clobber static value on unrelated change”.
core/src/test/scala/unit/kafka/server/KafkaApisTest.scala Updates QuotaManagers construction to match the new parameter list.
core/src/test/scala/unit/kafka/server/ControllerApisTest.scala Updates QuotaManagers construction to match the new parameter list.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread core/src/main/java/kafka/server/QuotaFactory.java
@jeqo
jeqo force-pushed the jeqo/ts-consolidation-quota-dynamic branch from de9cd34 to 10b53e5 Compare June 10, 2026 11:22
@jeqo
jeqo requested a review from Copilot June 10, 2026 11:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Comment thread server-common/src/main/java/org/apache/kafka/server/config/ServerConfigs.java Outdated
Make diskless.consolidation.fetch.rate.limit.bytes.per.second a dynamic
broker config, aligned with the leader/follower replication throttle pattern:

- Add to AllDynamicConfigs so KAdmin accepts it as a dynamic update
- Move ReplicationQuotaManager ownership to QuotaManagers (seeded from
  static config on startup via QuotaFactory.instantiate)
- Apply dynamic updates in BrokerConfigHandler.processConfigChanges,
  reading from currentKafkaConfig to pick up the merged static+dynamic
  value (avoids clobbering a static limit on unrelated config changes)
- Relax atLeast(1) to atLeast(0) and document as dynamically updatable
- Add DynamicConfigChangeUnitTest cases covering dynamic update and
  static-value-preserved-on-unrelated-change scenarios

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@jeqo
jeqo force-pushed the jeqo/ts-consolidation-quota-dynamic branch from 10b53e5 to 7757deb Compare June 10, 2026 11:38
@jeqo
jeqo requested a review from Copilot June 10, 2026 11:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

@jeqo
jeqo marked this pull request as ready for review June 10, 2026 12:10
@jeqo
jeqo requested a review from viktorsomogyi June 10, 2026 12:11
@viktorsomogyi
viktorsomogyi merged commit 6758f8f into main Jun 10, 2026
9 checks passed
@viktorsomogyi
viktorsomogyi deleted the jeqo/ts-consolidation-quota-dynamic branch June 10, 2026 15:37
jeqo added a commit that referenced this pull request Jun 10, 2026
Make diskless.consolidation.fetch.rate.limit.bytes.per.second a dynamic
broker config, aligned with the leader/follower replication throttle pattern:

- Add to AllDynamicConfigs so KAdmin accepts it as a dynamic update
- Move ReplicationQuotaManager ownership to QuotaManagers (seeded from
  static config on startup via QuotaFactory.instantiate)
- Apply dynamic updates in BrokerConfigHandler.processConfigChanges,
  reading from currentKafkaConfig to pick up the merged static+dynamic
  value (avoids clobbering a static limit on unrelated config changes)
- Relax atLeast(1) to atLeast(0) and document as dynamically updatable
- Add DynamicConfigChangeUnitTest cases covering dynamic update and
  static-value-preserved-on-unrelated-change scenarios

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
jeqo added a commit that referenced this pull request Jun 10, 2026
Make diskless.consolidation.fetch.rate.limit.bytes.per.second a dynamic
broker config, aligned with the leader/follower replication throttle pattern:

- Add to AllDynamicConfigs so KAdmin accepts it as a dynamic update
- Move ReplicationQuotaManager ownership to QuotaManagers (seeded from
  static config on startup via QuotaFactory.instantiate)
- Apply dynamic updates in BrokerConfigHandler.processConfigChanges,
  reading from currentKafkaConfig to pick up the merged static+dynamic
  value (avoids clobbering a static limit on unrelated config changes)
- Relax atLeast(1) to atLeast(0) and document as dynamically updatable
- Add DynamicConfigChangeUnitTest cases covering dynamic update and
  static-value-preserved-on-unrelated-change scenarios

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants