KAFKA-20247: Controller registration needs to retry after request timeout by kevin-wu24 · Pull Request #21619 · apache/kafka

kevin-wu24 · 2026-03-03T11:00:50Z

Set pendingRpc to false when controller registration times out. If we do
not set this, controllers cannot send ControllerRegistrationRequests
thereafter. Instead, subsequent calls to
maybeSendControllerRegistration() will always log:
"maybeSendControllerRegistration: waiting for the previous RPC to
complete." The asserts on L294 and L300 fail when pendingRpc does not
get set in onTimeout.

Previously, RegistrationResponseHandler callbacks were invoked from the
NodeToControllerRequestThread. Instead, these callbacks should append an
event to the ControllerRegistrationManger event queue.

Added testRetransmitRegistrationAfterTimeout. This test times out a
controller registration, and checks that the registration manager
channel manager's request queue state is as expected.

Reviewers: José Armando García Sancio jsancio@apache.org

jsancio

Thanks for the fix @kevin-wu24

…on the registration manager's event queue

jsancio

The solution LGTM. Just some minor suggestions in the test.

jsancio · 2026-03-13T18:49:26Z


  private def newControllerRegistrationManager(
    context: RegistrationTestContext,
+    exponentialBackoff: ExponentialBackoff = new ExponentialBackoff(1, 2, 100, 0.02)


Let's avoid having default values in the parameter. There are many benefits to this including easier translation to Java in the near future. Looking at the imports it looks like this class can be easily translated to Java in a future PR.

jsancio · 2026-03-13T18:57:42Z

-    val manager = newControllerRegistrationManager(context)
+    // Use a large retry backoff with no jitter so we can reliably observe the
+    // intermediate state after receiving the error response before the scheduled retry fires.
+    val retryBackoffMs = 1000L


Is the comment true? You changed the time used by KafkaEventTime to be a mocked time.

I think the retry timeout can be smaller, but the expression (long) (jitter * initial_interval) evaluated as part of ExponentialBackoff#backoff MUST be > 0 for the retry logic to be deterministic because we append an event to observe the intermediate rpcStats state. The values of initial_interval = 1 and jitter = 0.02 do not satisfy this, because half the time the event gets scheduled immediately with backoff = 0, so pendingRpc can get set to true before rpcStats event can observe it getting set to false.

Not really sure if this is a bug with the class. It doesn't seem to me that this behavior is "wrong," just annoying to write a test around if you pick initial_interval = 1 like was chosen here.

jsancio · 2026-03-13T18:59:28Z

+    // Use a large retry backoff with no jitter so we can reliably observe the
+    // intermediate state after timeout before the scheduled retry fires.
+    val retryBackoffMs = 1000L


Same comment here. Now the time is mocked. Reschedule event should be more reliable.

jsancio

LGTM. Thanks for the fix

…eout (#21619) Set pendingRpc to false when controller registration times out. If we do not set this, controllers cannot send ControllerRegistrationRequests thereafter. Instead, subsequent calls to maybeSendControllerRegistration() will always log: "maybeSendControllerRegistration: waiting for the previous RPC to complete." The asserts on L294 and L300 fail when pendingRpc does not get set in onTimeout. Previously, RegistrationResponseHandler callbacks were invoked from the NodeToControllerRequestThread. Instead, these callbacks should append an event to the ControllerRegistrationManger event queue. Added testRetransmitRegistrationAfterTimeout. This test times out a controller registration, and checks that the registration manager channel manager's request queue state is as expected. Reviewers: José Armando García Sancio <jsancio@apache.org>

…eout (apache#21619) Set pendingRpc to false when controller registration times out. If we do not set this, controllers cannot send ControllerRegistrationRequests thereafter. Instead, subsequent calls to maybeSendControllerRegistration() will always log: "maybeSendControllerRegistration: waiting for the previous RPC to complete." The asserts on L294 and L300 fail when pendingRpc does not get set in onTimeout. Previously, RegistrationResponseHandler callbacks were invoked from the NodeToControllerRequestThread. Instead, these callbacks should append an event to the ControllerRegistrationManger event queue. Added testRetransmitRegistrationAfterTimeout. This test times out a controller registration, and checks that the registration manager channel manager's request queue state is as expected. Reviewers: José Armando García Sancio <jsancio@apache.org>

kevin-wu24 added 4 commits March 3, 2026 10:42

controller registration needs to retry after request timeout

11f8e1b

fix flakiness

04f14d8

updating test comments

4615338

add asserts on channel manager queue size

f9786b0

github-actions bot added triage PRs from the community core Kafka Broker small Small PRs labels Mar 3, 2026

jsancio reviewed Mar 3, 2026

View reviewed changes

Comment thread core/src/main/scala/kafka/server/ControllerRegistrationManager.scala

jsancio self-assigned this Mar 3, 2026

jsancio added the ci-approved label Mar 3, 2026

fix flaky testRetransmitRegistration

64aa7f6

github-actions bot removed the triage PRs from the community label Mar 4, 2026

ControllerRegistrationRequest response + timeout needs to be handled …

8151cca

…on the registration manager's event queue

github-actions bot removed the small Small PRs label Mar 9, 2026

fix last commit

9679b73

github-actions bot added the small Small PRs label Mar 9, 2026

cleanup log message

f0981e8

jsancio reviewed Mar 13, 2026

View reviewed changes

remove jitter for deterministic unit test assertions with retries

01e5029

jsancio approved these changes Mar 16, 2026

View reviewed changes

jsancio merged commit 32c0e1c into apache:trunk Mar 16, 2026
26 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KAFKA-20247: Controller registration needs to retry after request timeout#21619

KAFKA-20247: Controller registration needs to retry after request timeout#21619
jsancio merged 9 commits intoapache:trunkfrom
kevin-wu24:KAFKA-20247

kevin-wu24 commented Mar 3, 2026 •

edited by github-actions bot

Loading

Uh oh!

jsancio left a comment

Uh oh!

Uh oh!

jsancio left a comment

Uh oh!

jsancio Mar 13, 2026

Uh oh!

jsancio Mar 13, 2026

Uh oh!

kevin-wu24 Mar 14, 2026 •

edited

Loading

Uh oh!

jsancio Mar 13, 2026

Uh oh!

jsancio left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

kevin-wu24 commented Mar 3, 2026 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jsancio left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jsancio left a comment

Choose a reason for hiding this comment

Uh oh!

jsancio Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

jsancio Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

kevin-wu24 Mar 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jsancio Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

jsancio left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kevin-wu24 commented Mar 3, 2026 •

edited by github-actions bot

Loading

kevin-wu24 Mar 14, 2026 •

edited

Loading