[Bug] POP revive checkpoints may be processed out of order due to reviveOffset comparator overflow
- Dominant language
- Java
- Stars
- 22.6k
- Forks
- 12k
- Avg merge
- 3d 1h
- Merged PRs (30d)
- 27
Description
### Before Creating the Bug Report
- [x] I found a bug, not just asking a question, which should be created in [GitHub Discussions](https://github.com/apache/rocketmq/discussions).
- [x] I have searched the [GitHub Issues](https://github.com/apache/rocketmq/issues) and [GitHub Discussions](https://github.com/apache/rocketmq/discussions) of this repository and believe that this is not a duplicate.
- [x] I have confirmed that this bug belongs to the current repository, not other repositories of RocketMQ.
### Runtime platform environment
Ubuntu
### RocketMQ version
Branch: develop
### JDK Version
_No response_
### Describe the Bug
PopReviveService.ConsumeReviveObj#genSortList() sorts PopCheckPoint objects by
subtracting two long reviveOffset values and casting the result to int.
sortList.sort((o1, o2) -> (int) (o1.getReviveOffset() - o2.getReviveOffset()));
When the difference between two reviveOffset values is greater than
Integer.MAX_VALUE, the cast can overflow and return an incorrect comparison
result. This may cause POP revive checkpoints to be processed in the wrong
order.
### Steps to Reproduce
1. Create two PopCheckPoint instances.
2. Set their reviveOffset values to 1 and Integer.MAX_VALUE + 2L.
3. Add both checkpoints into PopReviveService.ConsumeReviveObj#map.
4. Call genSortList().
5. Observe that the old comparator may order the larger reviveOffset before the
smaller one.
### What Did You Expect to See?
PopCheckPoint objects should always be sorted by reviveOffset in ascending
numeric order, regardless of how large the long offset difference is.
### What Did You See Instead?
The current comparator can overflow after casting the long offset difference to
int, causing incorrect checkpoint ordering.
### Additional Context
This affects POP revive checkpoint ordering. Since the sorted list is later
used by mergeAndRevive(), incorrect ordering may affect checkpoint processing
and revive offset advancement.
A safe implementation is:
sortList.sort(Comparator.comparingLong(PopCheckPoint::getReviveOffset));
Contributor guide
Research direction
Start at PopReviveService.ConsumeReviveObj#genSortList() and inspect how its PopCheckPoint list is sorted before mergeAndRevive(). Reproduce the two-offset case from the issue, then verify that large long offsets are ordered numerically in ascending order without comparator overflow.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- distributed-systems
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100