RockteMQ-AI commented on code in PR #11016:
URL: https://github.com/apache/rocketmq/pull/11016#discussion_r3939744422
##########
broker/src/main/java/org/apache/rocketmq/broker/pop/PopConsumerCache.java:
##########
@@ -166,6 +189,26 @@ public int cleanupRecords(Consumer<PopConsumerRecord>
consumer) {
return remain;
}
+ /**
+ * A staged record that left the staged set while {@code
#cleanupRecords(Consumer)} was
+ * writing it to the store was acked concurrently; the ack path has
already deleted the
+ * durable record, so delete it once more here to undo the re-persist
above.
+ */
+ private void deleteAckedStagedRecords(ConsumerRecords records,
List<PopConsumerRecord> writtenRecords) {
+ List<PopConsumerRecord> ackedList = null;
+ for (PopConsumerRecord record : writtenRecords) {
+ if (records.getRemoveTreeMap().get(record.getOffset()) != record) {
Review Comment:
The identity check (`!=` on object reference) is correct for detecting
whether the staged record was acked during the store write. If a future
refactor changes `removeTreeMap` to use value equality or introduces record
reuse at the same offset, this identity-based check could silently break.
Consider adding a brief inline comment explaining why identity comparison (not
`.equals()`) is intentional here.
--
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]
