RockteMQ-AI commented on PR #10532: URL: https://github.com/apache/rocketmq/pull/10532#issuecomment-4753943877
## Review by github-manager-bot ### Summary Fixes a cache key correctness bug in `MessageRocksDBStorage` where `DELETE_KEY_CACHE_FOR_TIMER` used `byte[]` as the key type. Since `byte[].equals()` compares object identity rather than content, the cache lookup in the UPDATE path always returned `null`, causing ghost records to persist in RocksDB. ### Findings - **[Positive]** Correct root-cause fix — `ByteBuffer.wrap()` provides content-based `equals()`/`hashCode()`, which is the standard Java pattern for using byte arrays as map/cache keys. - **[Positive]** Minimal change surface (3 lines in 1 file), no public API impact, no new dependencies. - **[Info]** `ByteBuffer.wrap()` does not copy the underlying array — it creates a lightweight view, so there is negligible allocation overhead compared to the original `byte[]` key approach. - **[Info]** Consider adding a unit test that verifies the DELETE-then-UPDATE path correctly skips re-insertion, to prevent regression. The test could perform a DELETE followed by an UPDATE with the same key content (but different `byte[]` instances), and assert the key is not re-inserted. ### Verdict LGTM. Clean fix for a real correctness bug. Approved. --- *Automated review by github-manager-bot* -- 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]
