Skip to content

[ISSUE #10450] Selective double-write in CombineConsumeQueueStore#10452

Open
imzs wants to merge 1 commit into
apache:developfrom
imzs:develop
Open

[ISSUE #10450] Selective double-write in CombineConsumeQueueStore#10452
imzs wants to merge 1 commit into
apache:developfrom
imzs:develop

Conversation

@imzs

@imzs imzs commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

#10450 Selective double-write in CombineConsumeQueueStore

@oss-sentinel-ai oss-sentinel-ai left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review by github-manager-bot

Summary

This PR implements selective double-write in CombineConsumeQueueStore as described in #10450. The enhancement introduces a new configuration switch rocksdbCQSelectiveDoubleWriteEnable to control which topics are double-written to RocksDB ConsumeQueue, addressing the performance overhead of unnecessary double-write for topics that don't need it.

Findings

  • [Good] MessageStoreConfig.java:476 — Added rocksdbCQSelectiveDoubleWriteEnable configuration with clear documentation explaining it's a secondary switch for rocksdbCQDoubleWriteEnable.
  • [Good] CombineConsumeQueueStore.java:131-138 — Added validation to ensure selective double-write is only enabled with compatible configurations (assignOffsetStore and currentReadStore must be consumeQueueStore, not rocksDB).
  • [Good] CombineConsumeQueueStore.java:262-265 — In verifyAndInitOffsetForAllStore, skips RocksDB store initialization for topics that don't need double-write.
  • [Good] CombineConsumeQueueStore.java:365-368 — In putMessagePositionInfoWrapper, skips writing to RocksDB store for topics that don't need double-write. This is the core optimization.
  • [Good] LiteManagerProcessor.java:112-113 — Updated store type reporting to return "Combine" when double-write is enabled, providing accurate runtime information.
  • [Good] CombineConsumeQueueStoreTest.java:110-127 — Added tests for the new validation logic ensuring selective double-write fails fast with incompatible configurations.
  • [Good] Test refactoring — Replaced Assert.assertEquals with static import assertEquals for consistency.

Analysis

The implementation is well-structured:

  1. Configuration: Clear opt-in switch with backward compatibility (default false)
  2. Validation: Fail-fast checks prevent misconfiguration
  3. Performance: Selective double-write reduces overhead for topics that don't need RocksDB CQ
  4. Test coverage: Comprehensive tests for validation logic and edge cases

The approach correctly identifies that LITE topics need RocksDB CQ (for LMQ) while ordinary topics don't, allowing selective optimization.

Suggestions

  1. Consider adding metrics to track how many topics are being double-written vs. selectively written, to help users monitor the optimization's impact.
  2. The validation logic in the constructor is good, but consider adding a warning log when selective double-write is enabled with no LITE topics, as it would have no effect.

Verdict

Approved — Well-designed enhancement with proper validation, comprehensive tests, and clear performance benefits for deployments with mixed topic types.


Automated review by github-manager-bot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants