apache / apache/rocketmq

[Bug] POP revive checkpoints may be processed out of order due to reviveOffset comparator overflow

Open Beginner friendly
#10,582 1 comment 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.