You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "chickenchickenlove (via GitHub)" <gi...@apache.org> on 2026/01/10 12:25:40 UTC

[PR] KAFKA-20058: Fix race condition on backoffDeadlineMs on RPCProducerIdManager causing premature retries [kafka]

chickenchickenlove opened a new pull request, #21279:
URL: https://github.com/apache/kafka/pull/21279

   ### Description
   This PR fixes a race condition in `RPCProducerIdManager.maybeRequestNextBlock()` that can clobber a newly-set retry backoff and cause premature retries.
   
   The Problem `maybeRequestNextBlock()` sends the controller request asynchronously and then unconditionally resets `backoffDeadlineMs` to `NO_RETRY`. On the response path, `handleUnsuccessfulResponse()` sets `backoffDeadlineMs = now + RETRY_BACKOFF_MS`.
   
   Because the send is asynchronous, the unconditional reset in the request path can execute after the failure handler has already set the backoff. This overwrites the valid backoff with `NO_RETRY`. Consequently, a subsequent `generateProducerId()` call can re-send immediately, leading to unnecessary controller traffic and flaky test behavior.
   
   The Fix The fix replaces the unconditional reset with a conditional compareAndSet. We now only reset the backoff if it has not been updated concurrently by the response handler.
   
   ### Sequence Diagram
   <img width="7840" height="8830" alt="ddd-1" src="https://github.com/user-attachments/assets/b7e079af-7dd3-4854-8719-503ed7cb0925" />
   
   ### Flaky Tests fixed by this changes.
   - https://develocity.apache.org/scans/tests?search.rootProjectNames=kafka&search.timeZoneId=Asia%2FTaipei&tests.container=org.apache.kafka.coordinator.transaction.ProducerIdManagerTest&tests.sortField=FLAKY
   - `ProducerIdManagerTest#testRetryBackoffOnNoResponse`
   - `ProducerIdManagerTest#testRetryBackoffOnAuthException`
   - `ProducerIdManagerTest#testRetryBackoffOnVersionMismatch`
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20058: Fix race condition on backoffDeadlineMs on RPCProducerIdManager causing premature retries [kafka]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #21279:
URL: https://github.com/apache/kafka/pull/21279#issuecomment-3808773172

   A label of 'needs-attention' was automatically added to this PR in order to raise the
   attention of the committers. Once this issue has been triaged, the `triage` label
   should be removed to prevent this automation from happening again.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20058: Fix race condition on backoffDeadlineMs on RPCProducerIdManager causing premature retries [kafka]

Posted by "chickenchickenlove (via GitHub)" <gi...@apache.org>.
chickenchickenlove commented on PR #21279:
URL: https://github.com/apache/kafka/pull/21279#issuecomment-3833222866

   > I also found a second race which can cause premature retries, where maybeRequestNextBlock reads a stale backoffDeadlineMs and then the in-flight request fails.
   
   @squah-confluent 
   Thanks a lot for the careful review and for pointing this out.  🙇‍♂️ 
   You’re right — there’s still another race here that I missed.
   
   If you’re okay with it, I can file an issue and follow up with a separate PR for this. If you were already planning to address it yourself, please let me know and I’ll hold off!
   
   Also, regarding the fix, I was thinking that reordering the operations as follows might address the issue, but we can discuss this further in the next PR.
   
   ```java
   private void maybeRequestNextBlock() {
       if (nextProducerIdBlock.get() != null) 
           return;
   
       if (!requestInFlight.compareAndSet(false, true)) 
           return;
   
       final long retryTimestamp = backoffDeadlineMs.get();
       final long now = time.milliseconds();
   
       if (retryTimestamp != NO_RETRY && now < retryTimestamp) {
           requestInFlight.set(false);
           return;
       }
   
       sendRequest();
   }
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20058: Fix race condition on backoffDeadlineMs on RPCProducerIdManager causing premature retries [kafka]

Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #21279:
URL: https://github.com/apache/kafka/pull/21279#discussion_r2758614472


##########
transaction-coordinator/src/main/java/org/apache/kafka/coordinator/transaction/RPCProducerIdManager.java:
##########
@@ -115,7 +115,7 @@ private void maybeRequestNextBlock() {
                     requestInFlight.compareAndSet(false, true)) {
                 sendRequest();
                 // Reset backoff after a successful send.
-                backoffDeadlineMs.set(NO_RETRY);
+                backoffDeadlineMs.compareAndSet(retryTimestamp, NO_RETRY);

Review Comment:
   oh, I just left a similar comment (https://github.com/apache/kafka/pull/21279#discussion_r2758606672)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20058: Fix race condition on backoffDeadlineMs on RPCProducerIdManager causing premature retries [kafka]

Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #21279:
URL: https://github.com/apache/kafka/pull/21279#issuecomment-3836643589

   @chickenchickenlove thanks for this fix. It makes sense to me. I have triggered the CI, and I will take a closer look shortly


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20058: Fix race condition on backoffDeadlineMs on RPCProducerIdManager causing premature retries [kafka]

Posted by "jolshan (via GitHub)" <gi...@apache.org>.
jolshan commented on PR #21279:
URL: https://github.com/apache/kafka/pull/21279#issuecomment-3837617354

   Ah ok -- will wait for Chia-Ping before merging.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20058: Fix race condition on backoffDeadlineMs on RPCProducerIdManager causing premature retries [kafka]

Posted by "squah-confluent (via GitHub)" <gi...@apache.org>.
squah-confluent commented on PR #21279:
URL: https://github.com/apache/kafka/pull/21279#issuecomment-3836617935

   @chickenchickenlove We forgot to update the PR description


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20058: Fix race condition on backoffDeadlineMs on RPCProducerIdManager causing premature retries [kafka]

Posted by "chickenchickenlove (via GitHub)" <gi...@apache.org>.
chickenchickenlove commented on PR #21279:
URL: https://github.com/apache/kafka/pull/21279#issuecomment-3760382687

   @chia7712 @jolshan Hi!
   Sorry for sudden mention. 
   
   While investigating a flaky test, I identified a race condition in the transaction code path (around `RPCProducerIdManager`) and opened a PR with a fix.
   
   Since you’re closest to the current context in this area, I’d really appreciate it if you could take a look and share any feedback when you have bandwidth 🙇‍♂️ 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20058: Fix race condition on backoffDeadlineMs on RPCProducerIdManager causing premature retries [kafka]

Posted by "squah-confluent (via GitHub)" <gi...@apache.org>.
squah-confluent commented on code in PR #21279:
URL: https://github.com/apache/kafka/pull/21279#discussion_r2751814180


##########
transaction-coordinator/src/main/java/org/apache/kafka/coordinator/transaction/RPCProducerIdManager.java:
##########
@@ -115,7 +115,7 @@ private void maybeRequestNextBlock() {
                     requestInFlight.compareAndSet(false, true)) {
                 sendRequest();
                 // Reset backoff after a successful send.
-                backoffDeadlineMs.set(NO_RETRY);
+                backoffDeadlineMs.compareAndSet(retryTimestamp, NO_RETRY);

Review Comment:
   Thank you for fixing the bug!
   
   Could we consider only updating `backoffDeadlineMs` together with the clearing of `requestInFlight`? That way we don't have to think about the race when setting `backoffDeadlineMs` at all, since it would be only set at the end of the in-flight request.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20058: Fix race condition on backoffDeadlineMs on RPCProducerIdManager causing premature retries [kafka]

Posted by "chickenchickenlove (via GitHub)" <gi...@apache.org>.
chickenchickenlove commented on PR #21279:
URL: https://github.com/apache/kafka/pull/21279#issuecomment-3837914178

   @squah-confluent 
   Thank you for taking a look.
   I've opened KAFKA-20114 for the other race condition. 
   Let's discuss it there before starting on a fix!
   
   @jolshan , @chia7712 
   Thank you for the review! 
   `PR Linter` has been failed. 😢 
   How can I fix it? 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20058: Fix race condition on backoffDeadlineMs on RPCProducerIdManager causing premature retries [kafka]

Posted by "chickenchickenlove (via GitHub)" <gi...@apache.org>.
chickenchickenlove commented on PR #21279:
URL: https://github.com/apache/kafka/pull/21279#issuecomment-3802364910

   @jolshan @chia7712 
   Sorry to bother you, gently ping! 
   When you have bandwidth, please take a look 🙇‍♂️ 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20058: Fix race condition on backoffDeadlineMs on RPCProducerIdManager causing premature retries [kafka]

Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #21279:
URL: https://github.com/apache/kafka/pull/21279#issuecomment-3840589951

   > I noticed the PR Linter failed.
   
   The linter failure was caused by the missing "Reviewers" field in the description. Fixed


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20058: Fix race condition on backoffDeadlineMs on RPCProducerIdManager causing premature retries [kafka]

Posted by "jolshan (via GitHub)" <gi...@apache.org>.
jolshan commented on PR #21279:
URL: https://github.com/apache/kafka/pull/21279#issuecomment-3836550614

   Hey folks -- sorry my email notifications don't work very well. I can take a look. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20058: Fix race condition on backoffDeadlineMs on RPCProducerIdManager causing premature retries [kafka]

Posted by "squah-confluent (via GitHub)" <gi...@apache.org>.
squah-confluent commented on PR #21279:
URL: https://github.com/apache/kafka/pull/21279#issuecomment-3836196135

   > > I also found a second race which can cause premature retries, where maybeRequestNextBlock reads a stale backoffDeadlineMs and then the in-flight request fails.
   > 
   > If you’re okay with it, I can file an issue and follow up with a separate PR for this. If you were already planning to address it yourself, please let me know and I’ll hold off!
   > 
   > Also, regarding the fix, I was thinking that reordering the operations as follows might address the issue, but we can discuss this further in the next PR.
   
   Thanks for the fix. Please go ahead and file the issue and PR!
   
   I can think of two ways to fix it:
   1. Pack `backoffDeadlineMs` and `requestInFlight` into the same atomic, by creating a record class to hold them.
   2. Set `requestInFlight` and then unset it if `backoffDeadlineMs` is not satisfied, which is your fix. I think this option could be nicer.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20058: Fix race condition on backoffDeadlineMs on RPCProducerIdManager causing premature retries [kafka]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #21279:
URL: https://github.com/apache/kafka/pull/21279#issuecomment-3764864923

   A label of 'needs-attention' was automatically added to this PR in order to raise the
   attention of the committers. Once this issue has been triaged, the `triage` label
   should be removed to prevent this automation from happening again.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20058: Fix race condition on backoffDeadlineMs on RPCProducerIdManager causing premature retries [kafka]

Posted by "chickenchickenlove (via GitHub)" <gi...@apache.org>.
chickenchickenlove commented on code in PR #21279:
URL: https://github.com/apache/kafka/pull/21279#discussion_r2752969628


##########
transaction-coordinator/src/main/java/org/apache/kafka/coordinator/transaction/RPCProducerIdManager.java:
##########
@@ -115,7 +115,7 @@ private void maybeRequestNextBlock() {
                     requestInFlight.compareAndSet(false, true)) {
                 sendRequest();
                 // Reset backoff after a successful send.
-                backoffDeadlineMs.set(NO_RETRY);
+                backoffDeadlineMs.compareAndSet(retryTimestamp, NO_RETRY);

Review Comment:
   Thanks for your comments! Good Idea 👍 
   I made an commit based on your comment.
   
   to preserve the existing semantics, I added code to set `backoffDeadlineMs` to `NO_RETRY` on the timeout path.
   
   A more conservative approach could be to call `handleUnsuccessfulResponse()` on `TIMEOUT` as well, so that we apply the same retry backoff. However, since the previous code path did not update `backoffDeadlineMs` on `onTimeout()`, I kept that behavior here to minimize any behavioral change in this PR.
   
   When you have bandwidth, please take another look. 🙇‍♂️ 
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20058: Fix race condition on backoffDeadlineMs on RPCProducerIdManager causing premature retries [kafka]

Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 merged PR #21279:
URL: https://github.com/apache/kafka/pull/21279


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20058: Fix race condition on backoffDeadlineMs on RPCProducerIdManager causing premature retries [kafka]

Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #21279:
URL: https://github.com/apache/kafka/pull/21279#discussion_r2758606672


##########
transaction-coordinator/src/main/java/org/apache/kafka/coordinator/transaction/RPCProducerIdManager.java:
##########
@@ -114,8 +117,6 @@ private void maybeRequestNextBlock() {
             if (nextProducerIdBlock.get() == null &&
                     requestInFlight.compareAndSet(false, true)) {
                 sendRequest();
-                // Reset backoff after a successful send.

Review Comment:
   I was thinking about using `compareAndSet` to resolve the race condition, but your approach is much cleaner.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org