You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "unbridled-41 (via GitHub)" <gi...@apache.org> on 2026/08/29 17:38:03 UTC
[PR] [ISSUE #10989] Fix transactional message escape retry backoff (2 ^ n computed with XOR) [rocketmq]
unbridled-41 opened a new pull request, #10993:
URL: https://github.com/apache/rocketmq/pull/10993
<!-- Please make sure the target branch is right. In most case, the target branch should be `develop`. -->
### Which Issue(s) This PR Fixes
- Fixes #10989
### Brief Description
In `TransactionalMessageServiceImpl#check`, the retry delay between failed escape attempts (broker with `enableSlaveActingMaster`, `escapeMessage` failing) was computed as
```java
Thread.sleep(100L * (2 ^ escapeFailCnt));
```
In Java `^` is XOR, not exponentiation, so the actual sleep sequence for `escapeFailCnt` = 1..10 was `300, 0, 100, 600, 700, 400, 500, 800, 900, 200` ms — non-monotonic and including a **0 ms** delay on the second failure, which defeats the backoff entirely and hammers the store with immediate re-puts.
This PR extracts the computation into a package-private helper `escapeRetryBackoffMillis(int)` that returns the intended exponential backoff `100L * (1 << escapeFailCnt)` (200, 400, 800, ... ms), and uses it at the call site. No behavior other than the delay values changes.
### How Did You Test This Change?
Added `TransactionalMessageServiceImplTest#testEscapeRetryBackoffMillisIsExponential`, which asserts `escapeRetryBackoffMillis(i) == 100L * (1L << i)` for i = 1..10. It fails on the old code (XOR sequence) and passes with this change.
`mvn -pl broker test -Dtest=TransactionalMessageServiceImplTest` passes (9/9).
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #10989] Fix transactional message escape retry backoff (2 ^ n computed with XOR) [rocketmq]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10993:
URL: https://github.com/apache/rocketmq/pull/10993#issuecomment-5544150489
## [Codecov](https://app.codecov.io/gh/apache/rocketmq/pull/10993?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `50.00000%` with `1 line` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 48.73%. Comparing base ([`ff8f6f7`](https://app.codecov.io/gh/apache/rocketmq/commit/ff8f6f74c560e391261ccd716707c6d20422e253?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`a2f6d67`](https://app.codecov.io/gh/apache/rocketmq/commit/a2f6d67ab5f3032cf7af8a74fbe9907f95e12e07?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/rocketmq/pull/10993?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...saction/queue/TransactionalMessageServiceImpl.java](https://app.codecov.io/gh/apache/rocketmq/pull/10993?src=pr&el=tree&filepath=broker%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Frocketmq%2Fbroker%2Ftransaction%2Fqueue%2FTransactionalMessageServiceImpl.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvdHJhbnNhY3Rpb24vcXVldWUvVHJhbnNhY3Rpb25hbE1lc3NhZ2VTZXJ2aWNlSW1wbC5qYXZh) | 50.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/rocketmq/pull/10993?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## develop #10993 +/- ##
=============================================
- Coverage 48.86% 48.73% -0.14%
+ Complexity 13796 13756 -40
=============================================
Files 1381 1381
Lines 101574 101575 +1
Branches 13213 13213
=============================================
- Hits 49636 49498 -138
- Misses 45925 46039 +114
- Partials 6013 6038 +25
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/rocketmq/pull/10993?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #10989] Fix transactional message escape retry backoff (2 ^ n computed with XOR) [rocketmq]
Posted by "unbridled-41 (via GitHub)" <gi...@apache.org>.
unbridled-41 commented on PR #10993:
URL: https://github.com/apache/rocketmq/pull/10993#issuecomment-5547356503
Evidence chain re-verified on 2026-09-05 (JDK 21, Maven 3.8.7), anchored to commits:
- **After fix** — fix commit 29d7ce294 (the develop-merge on the branch is from `update-branch`, no code change):
`mvn -pl broker test -Dtest=TransactionalMessageServiceImplTest` → `Tests run: 9, Failures: 0, Errors: 0, Skipped: 0`, including `testEscapeRetryBackoffMillisIsExponential` asserting `escapeRetryBackoffMillis(i) == 100L * (1L << i)` for i = 1..10.
- **Before fix** — base commit e348efa66, call site `TransactionalMessageServiceImpl.java:253`: `Thread.sleep(100L * (2 ^ escapeFailCnt))`. The new test cannot compile against the base (the helper is extracted by this PR), so the defect is shown at formula level (evaluated with jshell on the same JDK):
- old expression `100L * (2 ^ i)` for i = 1..10 → `300 0 100 600 700 400 500 1000 1100 800` ms — non-monotonic, 0 ms on the second failure (backoff defeated);
- fixed expression `100L * (1L << i)` → `200 400 800 1600 3200 6400 12800 25600 51200 102400` ms.
- **CI**: all 10 checks green on this PR after re-triggering the workflows via branch update (maven-compile linux/macos/windows, bazel, CodeQL, coverage, license, misspell).
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #10989] Fix transactional message escape retry backoff (2 ^ n computed with XOR) [rocketmq]
Posted by "unbridled-41 (via GitHub)" <gi...@apache.org>.
unbridled-41 commented on PR #10993:
URL: https://github.com/apache/rocketmq/pull/10993#issuecomment-5468007787
### Test evidence (before → after)
The sleep-sequence claim was verified by directly evaluating both expressions for `escapeFailCnt` = 1..10 (plain JDK 8):
```
old 100L * (2 ^ n) : 300, 0, 100, 600, 700, 400, 500, 1000, 1100, 800
new 100L * (1 << n) : 200, 400, 800, 1600, 3200, 6400, 12800, 25600, 51200, 102400
```
(The sequences in the description and in #10989 have been corrected accordingly — n=8/9/10 evaluate to 1000/1100/800, not 800/900/200.)
Key point: `2 ^ 2 == 0` in Java (XOR), so the second consecutive escape failure sleeps **0 ms** and immediately re-attempts the store put; and the sequence is non-monotonic throughout, so this cannot be an intentional policy.
`TransactionalMessageServiceImplTest#testEscapeRetryBackoffMillisIsExponential` asserts the new helper `escapeRetryBackoffMillis(i) == 100L * (1L << i)` for i = 1..10. Two notes on the test itself:
- The helper was introduced by this fix, so the test cannot compile against the unfixed code — the before/after evidence for the wrong values is the expression evaluation above.
- On the fixed code: `mvn -pl broker test -Dtest=TransactionalMessageServiceImplTest` → `Tests run: 9, Failures: 0, Errors: 0, Skipped: 0` (8 pre-existing + 1 new).
The production call site (`Thread.sleep(escapeRetryBackoffMillis(escapeFailCnt))`) is the only place the delay is computed, so unit-testing the helper covers the behavioral change fully.
Side note on CI: the workflow runs for this PR are in `action_required` state (first-time contributor) and will start once a maintainer approves them.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org