KAFKA-19315: Move ControllerMutationQuotaManager to server module #19807

YutaLin · 2025-05-25T05:30:41Z

Migrate ControllerMutationQuotaManager to Java implementation and move
to server module, including ClientQuotaManager and associated files.

Reviewers: PoAn Yang [email protected],
Ken Huang [email protected], TengYao
Chi [email protected], Chia-Ping
Tsai [email protected]

…ionQuotaManager

… mock issue in KafkaApisTest

server/src/main/java/org/apache/kafka/server/ClientQuotaManager.java

server/src/main/java/org/apache/kafka/server/UnboundedControllerMutationQuota.java

server/src/main/java/org/apache/kafka/server/AbstractControllerMutationQuota.java

server/src/main/java/org/apache/kafka/server/PermissiveControllerMutationQuota.java

server/src/main/java/org/apache/kafka/server/ClientSensors.java

m1a2st

Thanks @YutaLin for this patch, left some comments

server/src/main/java/org/apache/kafka/server/StrictControllerMutationQuota.java

core/src/main/scala/kafka/server/metadata/ClientQuotaMetadataManager.scala

core/src/test/scala/unit/kafka/server/ClientRequestQuotaManagerTest.scala

server/src/main/java/org/apache/kafka/server/ClientQuotaManager.java

core/src/test/scala/integration/kafka/server/DynamicBrokerReconfigurationTest.scala

frankvicky

@YutaLin: Please fix the conflicts

m1a2st

Thanks @YutaLin for update, few more comments

server/src/main/java/org/apache/kafka/server/ControllerMutationQuotaManager.java

server/src/main/java/org/apache/kafka/server/AbstractControllerMutationQuota.java

core/src/test/scala/unit/kafka/server/ClientQuotaManagerTest.scala

server/src/main/java/org/apache/kafka/server/ClientSensors.java

YutaLin · 2025-05-29T15:20:03Z

We will continue tracking the failing test PlaintextConsumerCommitTest > testCommitAsyncCompletedBeforeConsumerCloses [1] Type=Raft-Isolated, which is recorded in https://issues.apache.org/jira/browse/KAFKA-19352.

chia7712

@YutaLin thanks for this patch. a couple of comments are left

chia7712 · 2025-05-29T16:06:44Z

server/src/main/java/org/apache/kafka/server/ClientQuotaManager.java

+    protected final Metrics metrics;
+    private final QuotaType quotaType;
+    protected final Time time;
+    private final Optional<Plugin<ClientQuotaCallback>> clientQuotaCallbackPlugin;


we don't need to keep it as member of ClientQuotaManager

I assume you will remove this variable, but you don't. Could you please share the reason with me?

server/src/main/java/org/apache/kafka/server/ClientQuotaManager.java

chia7712

@YutaLin thanks for this patch. a couple of comments remain

server/src/main/java/org/apache/kafka/server/PermissiveControllerMutationQuota.java

server/src/main/java/org/apache/kafka/server/ControllerMutationQuotaManager.java

chia7712 · 2025-06-05T18:18:24Z

server/src/main/java/org/apache/kafka/server/ClientQuotaManager.java

+
+public class ClientQuotaManager {
+
+    static final class QuotaTypes {


Do we really need a struct class?

chia7712 · 2025-06-05T18:23:39Z

server/src/main/java/org/apache/kafka/server/ClientQuotaManager.java

+    }
+
+    private void start() {
+        throttledChannelReaper.start();


this method can be inlined to constructor

chia7712 · 2025-06-05T18:26:22Z

server/src/main/java/org/apache/kafka/server/ClientQuotaManager.java

+        this.quotaTypesEnabled = clientQuotaCallbackPlugin.isPresent() ?
+                QuotaTypes.CUSTOM_QUOTAS : QuotaTypes.NO_QUOTAS;
+
+        this.delayQueueSensor = metrics.sensor(quotaType.toString() + "-delayQueue");


toString is redunant

chia7712 · 2025-06-05T18:29:06Z

server/src/main/java/org/apache/kafka/server/ClientQuotaManager.java

+            int throttleTimeMs
+    ) {
+        if (throttleTimeMs > 0) {
+            ClientSensors clientSensors = getOrCreateQuotaSensors(session, clientId);


it would be cool if we can leverage var to cleanup the type declaration.

var clientSensors = getOrCreateQuotaSensors(session, clientId); clientSensors.throttleTimeSensor().record(throttleTimeMs); var throttledChannel = new ThrottledChannel(time, throttleTimeMs, throttleCallback);

chia7712 · 2025-06-05T18:29:30Z

server/src/main/java/org/apache/kafka/server/ClientQuotaManager.java

+     * Recording any larger value will always be throttled, even if no other values were recorded in the quota window.
+     * This is used for deciding the maximum bytes that can be fetched at once
+     */
+    public double getMaxValueInQuotaWindow(Session session, String clientId) {


could you please remove the prefix get

chia7712 · 2025-06-05T18:30:31Z

server/src/main/java/org/apache/kafka/server/ClientQuotaManager.java

+     * This is used for deciding the maximum bytes that can be fetched at once
+     */
+    public double getMaxValueInQuotaWindow(Session session, String clientId) {
+        if (quotasEnabled()) {


if (!quotasEnabled()) return Double.MAX_VALUE; var clientSensors = getOrCreateQuotaSensors(session, clientId); var limit = quotaCallback.quotaLimit(clientQuotaType, clientSensors.metricTags()); if (limit != null) return limit * (config.numQuotaSamples - 1) * config.quotaWindowSizeSeconds; return Double.MAX_VALUE;

YutaLin added 9 commits May 25, 2025 01:21

feat: new ControllerMutationQuotaManager

a11810c

feat: add ClientSensors

beeaa37

fix: style issue

1e39f88

feat: rewrite ClientQuotaManager and put in server module

ece698f

feat: complete ControllerMutationQuotaManager

0e9c42e

feat: move ControllerMutationQuota out

bdc3932

feat: replace scala version of ClientQuotaManager and ControllerMutat…

8b4d0f8

…ionQuotaManager

feat: remove ControllerMutationQuotaManager scala

03039d0

feat: remove ClientQuotaManager scala version and fix related files

Loading
Loading status checks…

6d0a9d7

github-actions bot added triage core performance labels May 25, 2025

m1a2st added the ci-approved label May 25, 2025

YutaLin added 6 commits May 25, 2025 10:46

fix: remove method overloads in ControllerMutationQuotaManager to fix…

3ec768e

… mock issue in KafkaApisTest

Merge branch 'trunk' into KAFKA-19315

Loading
Loading status checks…

f2d7168

fix: metricTagsToSensorSuffix in ClientQuotaManager

3d6ce9b

Merge branch 'trunk' into KAFKA-19315

Loading
Loading status checks…

188f04a

fix: fix matrictags with LinkedHashMap

Loading
Loading status checks…

1e8d590

fix: import style

Loading
Loading status checks…

068d4fd

chia7712 reviewed May 26, 2025

View reviewed changes

YutaLin changed the title ~~KAFK1-19315: Move ControllerMutationQuotaManager to server module~~ KAFKA-19315: Move ControllerMutationQuotaManager to server module May 26, 2025

YutaLin added 7 commits May 26, 2025 20:11

refactor: make metrictags immutable

8ca37a6

refactor: make UnboundedControllerMutationQuota inner

e491ab5

fix: java doc

12d2dd1

fix: make ClienSensors normal class

de33e8c

fix: add null check in PermissiveControllerMutationQuota

009729c

Merge branch 'trunk' into KAFKA-19315

Loading
Loading status checks…

fe24c23

style: import

2618d23

m1a2st reviewed May 27, 2025

View reviewed changes

fix: add null check in StrictControllerMutationQuota

8d3f9ff

YutaLin added 2 commits May 26, 2025 21:35

fix: remove unused comments

bc35ff5

fix: java docs and scala optional

Loading
Loading status checks…

a594c2c

github-actions bot removed the triage label May 27, 2025

frankvicky reviewed May 28, 2025

View reviewed changes

Merge branch trunk into KAFKA-19315

Loading
Loading status checks…

f02b95a

m1a2st reviewed May 28, 2025

View reviewed changes

YutaLin added 9 commits May 28, 2025 07:44

fix: javadoc format

b751ab6

fix: indentation

2001e6d

fix: javadocs and comments

Loading
Loading status checks…

d1d6f2f

Merge branch 'trunk' into KAFKA-19315

Loading
Loading status checks…

7820866

fix: remove equal method in ClientSensors

2f31618

Merge branch 'trunk' into KAFKA-19315

Loading
Loading status checks…

a4327fe

fix: add equal and hash method

Loading
Loading status checks…

db7a207

fix: metric tags suffix

b819738

Merge branch 'trunk' into KAFKA-19315

Loading
Loading status checks…

a3a93ff

chia7712 reviewed May 29, 2025

View reviewed changes

YutaLin added 11 commits May 29, 2025 15:44

refactor: move QuotaTypes in ClientQuotaManager

40e9ecd

refactor: coding convention

3ffb930

refactor: make entity record

46f4736

fix: flatten code

20a7b69

refactor: remove BaseUserEntity

Loading
Loading status checks…

c004886

Merge branch 'trunk' into KAFKA-19315

Loading
Loading status checks…

24dbb03

fix: NPComplexity

Loading
Loading status checks…

d33a8db

docs: update format

f186386

fix: remove Map copy of LinkedHashMap to prevent metric tags reorder

8c1a233

Merge branch 'trunk' into KAFKA-19315

Loading
Loading status checks…

8937e3a

Merge branch trunk into KAFKA-19315

Loading
Loading status checks…

b397043

chia7712 reviewed Jun 5, 2025

View reviewed changes


		public class ClientQuotaManager {

		static final class QuotaTypes {

KAFKA-19315: Move ControllerMutationQuotaManager to server module #19807

Are you sure you want to change the base?

KAFKA-19315: Move ControllerMutationQuotaManager to server module #19807

Conversation

YutaLin commented May 25, 2025 • 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

m1a2st left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

frankvicky left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

m1a2st left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

YutaLin commented May 29, 2025

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chia7712 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

YutaLin commented May 25, 2025 •

edited by github-actions bot

Loading