Batch AdminClient config RPCs in ReplicationThrottleHelper

[acgtun]

Summary

1. Why: setThrottles() and clearThrottles() in ReplicationThrottleHelper make sequential, blocking AdminClient RPCs — one describeConfigs + one incrementalAlterConfigs per broker, then per topic. For a 250-broker cluster with ~100 topics and 1250-movement batches, this produces 600+ synchronous RPCs taking 30+ minutes before any data movement begins. Every call passes Collections.singletonMap() / Collections.singletonList() with exactly one ConfigResource, despite the AdminClient API supporting multi-resource operations.
2. What: Batch all describeConfigs and incrementalAlterConfigs calls across brokers and topics into single multi-resource AdminClient calls (chunked at 500 resources per request), and verify all configs in one batched polling loop:

Operation	Before	After
setThrottles() broker rates	B x describeConfigs + B x incrementalAlterConfigs	1 batch describeConfigs + 1 batch incrementalAlterConfigs
setThrottles() topic replicas	T x describeConfigs + T x incrementalAlterConfigs	1 batch describeConfigs + 1 batch incrementalAlterConfigs
clearThrottles()	Same sequential pattern	Same batching
waitForConfigs() verification	(B+T) x polling loops	1 batch polling loop

Design decisions:

• Broker set-path configs use .all() (brokers always exist, atomic batch); topic configs and the per-broker clear path use .values() so a topic deleted mid-operation or a broker that became unreachable only skips itself instead of failing the whole batch.
• waitForBatchConfigs drops a topic from verification only after confirming via a single lazily-fetched listTopics that it was really deleted; any other failure keeps the resource pending and retries, so an unverified throttle can never pass silently.
• Requests are chunked at 500 resources per call, since topic config requests are routed to a single broker and unbounded batches could get oversized on clusters with many topics.
• Same 30-second CLIENT_REQUEST_TIMEOUT_MS per call; the per-call timeout does not scale linearly with resource count.
• Read-modify-write window trade-off documented in code: the batch version reads all configs first, then writes all — a wider window than the sequential approach, acceptable given the performance gain and the assumption that CC is the sole writer of throttle configs during an operation.
• No semantic changes and no new configs — same set/clear points as today, just batched. The now-unused single-resource methods (changeTopicConfigs, changeBrokerConfigs, waitForConfigs, getEntityConfigs) are removed.

Expected Behavior

Throttle setup and removal for an execution completes in seconds regardless of cluster size, and replica movements begin promptly after the proposal is computed.

Actual Behavior

On large clusters, the executor spends 30+ minutes issuing sequential throttle config RPCs before the first replica movement starts, and again when clearing throttles between batches.

Steps to Reproduce

1. Run Cruise Control against a large cluster (hundreds of brokers, ~100 topics with moving replicas).
2. Trigger an execution that moves replicas with a replication throttle configured (e.g. rebalance?dryrun=false).
3. Observe the delay between proposal computation and the first replica movement: one describeConfigs + one incrementalAlterConfigs round-trip per broker and per topic.

Known Workarounds

None within Cruise Control. Related PRs (#2214, #2304) reduce how often the helper is invoked at the Executor layer, which is complementary but does not remove the per-resource RPC pattern inside the helper.

Additional evidence

1. Environment: verified on a production cluster (hundreds of brokers, ~100K partitions), deployed via Strimzi with the patched cruise-control jar, running a full rebalance with tens of thousands of replica movements.
2. Timeline from the trigger on that cluster: proposal computed at ~75s; throttle rate described + altered on every broker in ~1s (single batched call); broker + topic configs verified with throttle setup complete in ~23s (nearly all of it the two fixed 10s verification backoff sleeps); first replica movements finished at ~100s. Previously this cluster showed the familiar ~30-minute stall before any movement began. Details in this comment.
3. Testing: all ReplicationThrottleHelperTest tests pass (unit + embedded-Kafka integration), including new testWaitForBatchConfigs cases (retry exhaustion, immediate match, partial match with retries, ExecutionException/TimeoutException during verification, deleted-topic skip, transient-failure retry, persistent-broker-failure exhaustion) and testSetThrottleOnMixedTopicExistence; checkstyle clean.

Categorization

• documentation
• bugfix
• new feature
• refactor
• security/CVE
• other

[imans777]

It's interesting that we'd also seen this ~30min waiting before movements begin, but we didn't know the reason. This should be a very nice and practical fix. (I'm not contributor but I might test this in our environment and give feedback for it. Because these open PRs looks really practical to our org, I've started testing them one by one and pick the good ones to use)

[acgtun]

It's interesting that we'd also seen this ~30min waiting before movements begin, but we didn't know the reason. This should be a very nice and practical fix. (I'm not contributor but I might test this in our environment and give feedback for it. Because these open PRs looks really practical to our org, I've started testing them one by one and pick the good ones to use)

Thanks @imans777 for the comment and working on testing. appreciated

[imans777]

Hey again
I've seen similar PRs like #2305 , #2304 and #2214. What makes unique about this one?
Also if you have enough time, you can pick whatever your change lacks from those changes, and make this change as the best of 4, so we try to merge this and close other three.

[acgtun]

Thanks for pulling these together — I hadn't seen all of them, and it's encouraging that several people independently hit the same bottleneck. I went through all three carefully.

#2214 and #2304 take a smart approach at a different layer: they optimize when throttles are set and cleared in the Executor (once per broker / once per execution), which cuts down how often the helper is invoked. This PR works at the layer below — batching the RPCs inside ReplicationThrottleHelper itself — so the two approaches are actually complementary rather than competing. If either of those lands as well, the batching here would make their throttle calls faster too.

#2305 is very close in spirit to this PR — same goal, same file — and it got several important things right, including bulk verification and removing the old single-resource methods. The main differences in this PR are around failure handling:

1. Per-resource verification. ReplicationThrottleHelper: Add support for AdminClient bulk operations #2305 verifies with describeConfigs(...).all(), so a topic deleted mid-verification causes every poll attempt to throw until retries exhaust and the execution fails. This PR verifies via .values() per resource: a topic is only dropped from verification after confirming via listTopics that it was really deleted, while any other failure (transient error, broker read failure) keeps the resource pending and retries. That way a deleted topic can't abort an execution, and an unverified throttle can't slip through silently either.
2. Per-broker clear path. clearThrottles reads broker configs via per-broker futures, so a broker that becomes unreachable mid-execution only skips throttle removal for itself rather than leaving throttles behind on all the healthy brokers.
3. No semantic changes and no new configs — same set/clear points as today, just batched, so it's a drop-in for existing deployments.

I also took your "best of 4" suggestion seriously and pushed 5195d1d to close the remaining gaps, borrowing what the other PRs did well:

• Chunk describeConfigs/incrementalAlterConfigs at 500 resources per call, since topic config requests are routed to a single broker and unbounded batches could get oversized on clusters with many topics.
• Resolve deleted topics with a single listTopics per batch instead of one existence check per topic.
• Remove the now-unused single-resource methods (changeTopicConfigs, changeBrokerConfigs, waitForConfigs, getEntityConfigs) — following ReplicationThrottleHelper: Add support for AdminClient bulk operations #2305's lead here.
• Add tests for the new failure modes: deleted-topic skip during verification, transient-failure retry, and persistent broker-failure exhaustion.

All 11 tests (unit + embedded-Kafka integration) pass and checkstyle is clean. And to be clear — if the maintainers prefer #2305 as the base, I'd be glad to help review it or port the per-resource verification handling over there instead; the goal is getting the fix merged, whichever vehicle it takes.

Since you mentioned you're testing these branches in your environment, feedback from a real cluster run would be hugely appreciated — happy to fix anything you hit.

[acgtun]

Real-cluster test results

Following up with results from running this branch (through 5195d1d) on a real production cluster (hundreds of brokers, ~100K partitions), deployed via Strimzi with the patched cruise-control jar.

Triggered a full rebalance with tens of thousands of replica movements (tens of TB of data to move). Timeline from the trigger:

Elapsed	Event
0s	rebalance?dryrun=false accepted
~75s	proposal computed, executor starts first batch (~400 tasks)
~76s	throttle rate described + altered on every broker in the cluster (~1s, single batched call)
~98s	broker + topic configs verified; throttle setup complete in ~23s
~100s	first replica movements finished

Throttle setup for the whole cluster took ~23 seconds, and nearly all of that is the two fixed 10-second verification backoff sleeps (one for broker configs, one for topic configs — the first describeConfigs after an alter doesn't yet reflect the change, so the existing retry util sleeps its 10s minimum). The actual batched describe/alter RPCs took ~1 second. Previously this cluster showed the familiar ~30-minute stall before any movement began.

The skip logic and throttle clearing also behave as intended across execution batches — subsequent batches only alter brokers whose throttle rate isn't already set, and no verification failures or errors appeared during execution.

Happy to share more details if useful.

[acgtun]

Updated the description to follow the PR template format.

[Copilot]

Pull request overview

This PR refactors ReplicationThrottleHelper to batch Kafka AdminClient config RPCs (describe/alter/verify) across brokers and topics, reducing the number of synchronous requests and improving throttle set/clear latency on large clusters.

Changes:

• Batch broker and topic throttle config reads/writes using multi-resource AdminClient calls (with chunking support for large request sets).
• Replace per-resource verification loops with a single batched polling loop (waitForBatchConfigs) that handles per-resource failures.
• Update and expand ReplicationThrottleHelperTest to mock/validate batched behaviors and additional verification edge cases.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/executor/ReplicationThrottleHelper.java	Implements batched describe/alter/verify flows for broker/topic throttles and introduces chunking + batched verification helper logic.
cruise-control/src/test/java/com/linkedin/kafka/cruisecontrol/executor/ReplicationThrottleHelperTest.java	Updates mocks and adds coverage for new batched verification and mixed topic existence scenarios.

---

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

[acgtun]

Good catch. Fixed in 58fbba7 — the broker read in setThrottles() now goes through describeConfigsInChunks() like every other AdminClient call in this class. To preserve the fail-fast semantics that .all() gave us for brokers, the per-broker futures are still resolved eagerly and any failure is rethrown, so behavior is unchanged apart from respecting the 500-resource cap.

[acgtun]

With the chunking fix in 58fbba7, the broker config in setThrottles() now comes from resolving each KafkaFuture<Config> directly, which either returns a non-null Config or throws — there is no longer a map lookup that could return null, so a failure surfaces as the underlying AdminClient exception rather than an NPE.

[acgtun]

Good catch — the intent was indeed a single lazy listTopics() per attempt (matching batchGetTopicConfigs / batchAlterTopicConfigs), but calling the per-topic helper inside the failure path defeated that. Fixed in 58fbba7 by fetching the topic-name set lazily at most once per verification attempt and reusing it for all failing topics in that attempt. A failed listTopics() still conservatively keeps the topics pending, so verification never passes unverified.