RockteMQ-AI commented on issue #10736:
URL: https://github.com/apache/rocketmq/issues/10736#issuecomment-5149534533

   **Fix Spec (v1)**
   
   **Root Cause:** `ConsumerRecords.delete` (`PopConsumerCache.java:230-232`) 
only removes from `recordTreeMap`. When `cleanupRecords` stages a record into 
`removeTreeMap` via `stageExpiredRecords`, a concurrent ACK calling 
`deleteRecords` cannot find it and falls through to the KV store delete. The 
in-progress cleanup write then re-persists the stale snapshot.
   
   **Proposed Fix:** Update `ConsumerRecords.delete` to also attempt removal 
from `removeTreeMap`:
   
   ```java
   public boolean delete(PopConsumerRecord record) {
       return recordTreeMap.remove(record.getOffset()) != null
           || removeTreeMap.remove(record.getOffset()) != null;
   }
   ```
   
   This lets an ACK cancel a staged checkpoint before `cleanupRecords` persists 
it. `getMinOffsetInBuffer` and `getInFlightRecordCount` already account for 
both maps, so they remain consistent.
   
   **Impact:** Prevents stale checkpoint resurrection after ACK. Minimal change 
— single method, no new synchronization.
   
   **Coordination Note:** PR #10519 touches adjacent `PopConsumerCache` code. 
This fix should be reviewed for compatibility with that PR.
   
   **Test Strategy:** Latch-controlled `PopConsumerCacheTest` reproducing the 
staging → ACK → cleanup race without sleeps or sockets.
   
   Reply `/approve` to proceed with PR generation, `/revise <feedback>` to 
request changes, or `/reject` to decline.
   
   ---
   *Automated spec by github-manager*


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