ai-yang opened a new issue, #10736:
URL: https://github.com/apache/rocketmq/issues/10736

   ### Before Creating the Bug Report
   
   - [x] This is a reproducible bug rather than a usage question.
   - [x] I searched open and closed issues, pull requests, and Discussions and 
found no duplicate.
   - [x] The affected code is in this RocketMQ repository.
   
   ### Runtime platform environment
   
   Linux x86_64. The reproduction is a deterministic broker unit test and does 
not require a running RocketMQ cluster.
   
   ### RocketMQ version
   
   - Branch: `develop`
   - Commit: `00e45b8a6db23efbe756d0306f10716156cfd4dd`
   
   ### JDK Version
   
   OpenJDK 8
   
   ### Describe the Bug
   
   `PopConsumerCache.cleanupRecords` first moves eligible records from 
`recordTreeMap` to the staged `removeTreeMap`, copies them into 
`writeConsumerRecords`, and then persists that snapshot before clearing the 
staged map.
   
   During the store write, an ACK for the same record calls 
`PopConsumerCache.deleteRecords`. That method checks only `recordTreeMap`, so 
it cannot find a staged record and delegates deletion to the durable KV store. 
The ACK can therefore delete the durable checkpoint and return success, after 
which the in-progress cleanup write persists its stale snapshot again.
   
   The ACK result is no longer reflected by durable state. A later scan can 
observe the restored checkpoint and process a message that was already 
acknowledged.
   
   ### Steps to Reproduce
   
   A latch-controlled `PopConsumerCacheTest` reproduces the following order 
without sleeps, sockets, or random scheduling:
   
   1. Put a checkpoint in the cache whose stay-buffer time has elapsed but 
whose visibility timeout is still in the future.
   2. Start `cleanupRecords`; it stages the checkpoint and blocks inside 
`PopConsumerKVStore.writeRecords`.
   3. ACK the same checkpoint. `deleteRecords` misses it in `recordTreeMap`, 
and the KV-store delete completes.
   4. Release the blocked cleanup write.
   5. Assert that the acknowledged checkpoint is absent from the KV store.
   
   The final assertion failed 5/5 times on the unmodified `develop` commit 
above: the checkpoint was stored again after the ACK-side delete.
   
   ### What Did You Expect to See?
   
   After ACK succeeds, cleanup must not re-persist the acknowledged checkpoint. 
The ACK/change-invisibility path needs a defined way to find, cancel, or 
supersede a staged checkpoint, or the cleanup write needs to verify ownership 
before it becomes durable.
   
   ### What Did You See Instead?
   
   The ACK returns success, but the blocked cleanup write subsequently restores 
the same checkpoint in the KV store.
   
   ### Additional Context
   
   This is an ownership and persistence-boundary question, so I have not opened 
an implementation PR yet. In particular, a fix should define whether staged 
records remain addressable by ACK and change-invisibility, and should preserve 
the ordering between checkpoint persistence and consumer-offset advancement.
   
   PR #10519 currently changes adjacent `PopConsumerCache` and 
change-invisibility code. Its author also noted that cleanup ownership should 
be handled separately for both single and batch paths with a dedicated design 
and benchmark: 
https://github.com/apache/rocketmq/pull/10519#issuecomment-4828618586. Any 
implementation for this issue should be coordinated with that PR to avoid 
conflicting semantics and changed files.
   


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