dong0713 opened a new pull request, #10675:
URL: https://github.com/apache/rocketmq/pull/10675
## What is the purpose of the change
PR #10579 fixed a long-to-int subtraction overflow in `DefaultElectPolicy`'s
comparator. A systematic scan found **three more comparators** with the same
`(int)(longA - longB)` overflow pattern. When the `long` delta exceeds
`Integer.MAX_VALUE`, the cast truncates the result and flips the sign,
corrupting sort/ordered-set invariants.
This PR fixes all three by replacing the subtraction-cast-to-`int` idiom
with `Long.compare`, mirroring the #10579 fix.
## Brief changelog
### 1. `PopRequest.COMPARATOR` — broker, long-polling (most severe)
- `expired` is a `long` timestamp; `op` is a `long` counter starting at
`Long.MIN_VALUE`.
- **Overflow is essentially guaranteed** for any non-trivial run: the `op`
counter walks from `Long.MIN_VALUE` upward, so deltas between early and late
requests easily exceed `Integer.MAX_VALUE`.
- This comparator backs a `ConcurrentSkipListSet` for pop long-polling. A
broken comparator corrupts the set's ordering invariants, causing requests to
be misplaced, starved, or delivered out of order.
- Fix: `Long.compare(o1.getExpired(), o2.getExpired())` +
`Long.compare(o1.op, o2.op)`.
### 2. `PopCheckPoint.compareTo` — store, pop checkpoint
- `startOffset` is a `long` queue offset. Long-running brokers can produce
offsets whose delta exceeds `Integer.MAX_VALUE`.
- Fix: `Long.compare(this.getStartOffset(), o.getStartOffset())`.
### 3. `PopReviveService.genSortList` — broker, pop revival
- `reviveOffset` is a `long` commit-log offset (same class as #10579).
- Fix: `Long.compare(o1.getReviveOffset(), o2.getReviveOffset())`.
## How to test
Added regression tests covering the overflow condition:
- `PopRequestComparatorTest` (broker module, 3 tests):
- `comparatorDoesNotOverflowOnOpDelta` — reflects `op` field to force a
delta beyond `Integer.MAX_VALUE`; verifies correct ordering.
- `comparatorDoesNotOverflowOnExpiredDelta` — `expired` timestamps differ
by more than `Integer.MAX_VALUE`.
- `comparatorIsConsistentForEqualExpired` — equal `expired` tie-broken by
`op` (FIFO).
- `PopCheckPointComparatorTest` (store module, 3 tests):
- `compareToDoesNotOverflowOnLargeOffsetDelta` — `startOffset` delta
exceeds `Integer.MAX_VALUE`.
- `compareToOrdersNearMinValue` — offsets near `Long.MIN_VALUE`.
- `compareToReturnsZeroForEqualOffsets` — equal offsets compare as zero.
All 6 tests pass with the fix; the old code fails the overflow tests.
## Verifying this change
- [x] Make sure there is a GitHub issue field for the change (usually
something like `#10674`).
Closes #10674
--
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]
