Skip to content

[ISSUE #11495] Disable inplace update for RocksDB consume queue offset column family - #11496

Open
leizhiyuan wants to merge 1 commit into
apache:developfrom
leizhiyuan:fix/rocksdb-offset-cf-inplace-update
Open

leizhiyuan wants to merge 1 commit into
apache:developfrom
leizhiyuan:fix/rocksdb-offset-cf-inplace-update

Conversation

@leizhiyuan

Copy link
Copy Markdown
Contributor

Which Issue(s) This PR Fixes

Brief Description

RocksDBOptionsFactory#createOffsetCFOptions() enabled setInplaceUpdateSupport(true) on the offset column family.

RocksDBConsumeQueueStore#putMessagePosition0 writes the CQ units (default CF) and the queue's MAX offset (offset CF) in one WriteBatch, and readers such as correctMinCqOffset rely on that batch being visible atomically. An in-place overwrite of an existing MAX key in the memtable is not protected by sequence-number visibility. A reader can therefore see the new MAX before the CQ units of the same batch. The first min lookup of a new queue then binary-searches a range whose entries are not visible yet and persists a min offset > 0, so consumers silently skip the first messages. See #11495 for the full analysis and standalone reproducers.

This PR removes setInplaceUpdateSupport(true) from the offset CF options. That is the only production change.

Trade-off: without in-place update, every MAX update adds a new version to the memtable instead of overwriting the old one. The offset CF only holds a few keys per queue (MAX/MIN), and it already uses a 64 MB write buffer. Obsolete versions are dropped on flush/compaction, and point lookups still return only the latest visible version. The extra memtable usage and write cost are small, and in return the batch is atomic again. Data already on disk stays compatible; this option only affects memtable behavior.

ConfigHelper#createConfigColumnFamilyOptions (metadata storage) also enables in-place update, but it does not have this cross-CF batch visibility dependency, so it is out of scope here.

How Did You Test This Change?

New tests in RocksDBOptionsFactoryTest:

  • testOffsetCFOptionsDisableInplaceUpdate: deterministic check that createOffsetCFOptions() does not enable in-place update.
  • testOffsetCFMaxOffsetVisibleWithConsumeQueueUnit: opens a RocksDB instance with the real createCQCFOptions / createOffsetCFOptions and broker-like DB options. A writer puts a CQ unit and MAX into one WriteBatch (like putMessagePosition0), and 4 readers check that CQ[MAX] is visible whenever MAX is visible. It runs for at most 3 seconds and stops at the first anomaly.

Before the fix (develop), 3 out of 3 runs failed:

RocksDBOptionsFactoryTest.testOffsetCFOptionsDisableInplaceUpdate  FAILURE
RocksDBOptionsFactoryTest.testOffsetCFMaxOffsetVisibleWithConsumeQueueUnit:165 max offset visible before its consume queue unit, reads: 6299 expected:<0> but was:<1>
RocksDBOptionsFactoryTest.testOffsetCFMaxOffsetVisibleWithConsumeQueueUnit:165 max offset visible before its consume queue unit, reads: 10427 expected:<0> but was:<51>
RocksDBOptionsFactoryTest.testOffsetCFMaxOffsetVisibleWithConsumeQueueUnit:165 max offset visible before its consume queue unit, reads: 16531 expected:<0> but was:<19>

After the fix: Tests run: 4, Failures: 0, stable over 6 runs.

Related store tests (checkstyle enabled):

mvn -pl store -am -DskipTests=false -Dsurefire.failIfNoSpecifiedTests=false -DfailIfNoTests=false \
  -Dtest='RocksDBOptionsFactoryTest,RocksDBConsumeQueue*Test,CombineConsumeQueueStoreTest,ConsumeQueueStoreTest,MessageRocksDBStorageTest,CqCompactionFilterJniTest' test
Tests run: 47, Failures: 0, Errors: 0, Skipped: 0 -- BUILD SUCCESS

I also ran an end-to-end check against the real RocksDBConsumeQueueStore. Each run did 1000 rounds of 200 fresh queues: batch1 = {offset 0}, then batch2 = {offsets 1..4} via putMessagePosition, while 4 threads called getMinOffsetInQueue. Before the fix it occasionally left min offset = 4 (failures in 1 of 8 runs and in a few earlier runs). After the fix, 8 out of 8 runs reported 0 wrong min offsets. It is too timing-dependent to be a reliable unit test, so it is not included.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.82%. Comparing base (592d59d) to head (56a2600).

Additional details and impacted files
@@            Coverage Diff             @@
##             develop   #11496   +/-   ##
==========================================
  Coverage      49.81%   49.82%           
+ Complexity     14405    14403    -2     
==========================================
  Files           1391     1391           
  Lines         103362   103361    -1     
  Branches       13519    13519           
==========================================
+ Hits           51493    51499    +6     
+ Misses         45652    45650    -2     
+ Partials        6217     6212    -5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

[Bug] RocksDB ConsumeQueue may persist a wrong min offset for new queues because offset CF enables inplace update

2 participants