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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
