You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "Metastarx (via GitHub)" <gi...@apache.org> on 2026/10/12 04:38:53 UTC
[PR] Compare 64-bit offsets without int truncation in pop revive ordering [rocketmq]
Metastarx opened a new pull request, #11361:
URL: https://github.com/apache/rocketmq/pull/11361
### Which Issue(s) This PR Fixes
(local scan, no link)
- Fixes #11360
### Brief Description
A checkpoint is compared by start offset in several places, and the comparison was written as `(int) (this.startOffset - o.startOffset)`. Because startOffset is a 64-bit queue offset, casting the 64-bit difference to int discards the high bits, so once two offsets differ by more than Integer.MAX_VALUE the sign can invert and the ordering stops being a valid total order. That matters because PopReviveService keys its inflightReviveRequestMap TreeMap by this type and relies on containsKey, remove and firstKey, and because checkpoints from many different original topics and groups sit in the same revive queue with independent offsets.
### How Did You Test This Change?
`mvn test -pl broker,store` - passed (Tests run: 834, Failures: 0, Errors: 0, Skipped: 4)
--
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] Compare 64-bit offsets without int truncation in pop revive ordering [rocketmq]
Posted by "Metastarx (via GitHub)" <gi...@apache.org>.
Metastarx commented on PR #11361:
URL: https://github.com/apache/rocketmq/pull/11361#issuecomment-6120278934
Thanks - `broker/null/config/delayOffset.json` was an empty file left behind by a local test run and slipped into the commit; removed in 143058dd8. I also added the note in `equals()` so its field set stays in sync with the `compareTo` tie-breakers. On the null ordering: nulls sorting first is intended, the checkpoints that can carry a null brokerName are the late-ack mocks and they only need a stable key.
--
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] Compare 64-bit offsets without int truncation in pop revive ordering [rocketmq]
Posted by "Metastarx (via GitHub)" <gi...@apache.org>.
Metastarx commented on PR #11361:
URL: https://github.com/apache/rocketmq/pull/11361#issuecomment-6137173367
`maven-compile (windows-latest, JDK-8)` failed on a timing assertion in a class this change does not touch: `ConsumerOrderInfoManagerLockFreeNotifyTest.testRecover` expects the notify to take at least 2 seconds, but the condition was already satisfied after 1.648s (`await().atLeast(...)`). The diff only covers `PopCheckPoint` and `PopReviveService`; on the same commit macOS, Ubuntu, bazel, check-license, misspell and CodeQL are all green (834 tests run, 1 error).
Would someone mind re-running the Windows job?
--
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] Compare 64-bit offsets without int truncation in pop revive ordering [rocketmq]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #11361:
URL: https://github.com/apache/rocketmq/pull/11361#issuecomment-6118097081
## [Codecov](https://app.codecov.io/gh/apache/rocketmq/pull/11361?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 `77.14286%` with `8 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 49.67%. Comparing base ([`e1eb99c`](https://app.codecov.io/gh/apache/rocketmq/commit/e1eb99c85296faba93ac43dd8edc8929f34fa126?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`8e38710`](https://app.codecov.io/gh/apache/rocketmq/commit/8e387101443e025bba47d7ebe776a4069b480964?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/11361?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...a/org/apache/rocketmq/store/pop/PopCheckPoint.java](https://app.codecov.io/gh/apache/rocketmq/pull/11361?src=pr&el=tree&filepath=store%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Frocketmq%2Fstore%2Fpop%2FPopCheckPoint.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL3BvcC9Qb3BDaGVja1BvaW50LmphdmE=) | 76.47% | [2 Missing and 6 partials :warning: ](https://app.codecov.io/gh/apache/rocketmq/pull/11361?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 #11361 +/- ##
=============================================
- Coverage 49.69% 49.67% -0.03%
- Complexity 14336 14342 +6
=============================================
Files 1390 1390
Lines 103248 103281 +33
Branches 13502 13515 +13
=============================================
- Hits 51313 51308 -5
- Misses 45734 45763 +29
- Partials 6201 6210 +9
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/rocketmq/pull/11361?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