zmuxuny opened a new issue, #11212:
URL: https://github.com/apache/rocketmq/issues/11212

   ### Before Creating the Bug Report
   
   - [x] This reports a broker POP offset defect, not a usage question.
   - [x] I searched issues and PRs for `getMinOffsetInBuffer`, staged POP 
checkpoints, and minimum cached offsets. #10667 addresses failed asynchronous 
revive, #10736 / #11016 address a concurrent ACK, and #10949 / #10964 change 
offset convergence; none compares the two live cache maps when reading their 
minimum.
   - [x] The code is in this repository's `broker` module.
   
   ### Runtime platform environment
   
   Ubuntu 22.04 / x86_64. Reproduced with an in-memory 
`PopConsumerCache.ConsumerRecords` test; no external cluster is needed.
   
   ### RocketMQ version
   
   `develop`, commit `78b96bc5e21216cd7896efae08f90c5cde4cae53` (2026-09-23).
   
   ### JDK Version
   
   OpenJDK 21.0.12.1.
   
   ### Describe the Bug
   
   `PopConsumerCache.ConsumerRecords.getMinOffsetInBuffer()` returns the first 
offset in `removeTreeMap` whenever that map is nonempty, without comparing it 
with the first offset still in `recordTreeMap`. During cleanup, 
`stageExpiredRecords` moves checkpoints into `removeTreeMap` before they are 
revived or written to RocksDB, so both maps can be nonempty at the same time.
   
   For example, an earlier queue offset 100 may still be an unexpired, 
memory-only checkpoint while later offset 200 is expired and staged. The method 
reports 200 even though the minimum cached offset is 100. 
`PopConsumerCache.getMinOffsetInCache()` exposes this value to the non-FIFO POP 
offset commit path in `PopConsumerService.handleGetMessageResult`. A concurrent 
POP can therefore commit past an unpersisted checkpoint while cleanup is in 
progress; a broker crash in that window risks losing its redelivery position.
   
   ### Steps to Reproduce
   
   1. Create `ConsumerRecords` for one group/topic/queue. Write an unexpired 
record at offset 100 and an expired record at offset 200.
   2. Call `stageExpiredRecords(now)`. Offset 200 is in `removeTreeMap`; offset 
100 remains in `recordTreeMap`.
   3. Call `getMinOffsetInBuffer()` before `clearStagedRecords()`.
   
   The regression test in the proposed PR will fix the times and assert both 
map contents and the reported minimum.
   
   ### What Did You Expect to See?
   
   The minimum offset across both maps, 100, until that checkpoint is ACKed or 
durably handed off.
   
   ### What Did You See Instead?
   
   The method reports 200 because it checks the staged map first and returns 
immediately.
   
   ### Additional Context
   
   The related #10964 PR also uses `getMinOffsetInCache()` to cap a pending POP 
offset commit. This fix is limited to the minimum-offset calculation and should 
remain compatible with that work. The existing `consumerRecordsTest` checks 
each map separately but not the mixed state.
   


-- 
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]

Reply via email to