Skip to content

fix(sei-db): fix flaky RocksDB parallel iteration test#2928

Open
pdrobnjak wants to merge 2 commits intomainfrom
pd/fix-rocks-db-ci-fail
Open

fix(sei-db): fix flaky RocksDB parallel iteration test#2928
pdrobnjak wants to merge 2 commits intomainfrom
pd/fix-rocks-db-ci-fail

Conversation

@pdrobnjak
Copy link
Contributor

Summary

  • Fixes a use-after-free race condition in the RocksDB iterator that caused TestDatabaseParallelIterationVersions to flake
  • The ReadOptions object (and its timestamp slice) created by newTSReadOptions() was passed to NewIteratorCF() but never stored, making it eligible for GC while the C++ iterator still held a dangling Slice pointer to the timestamp data
  • Under parallel access (99 goroutines in the test), GC pressure caused iterators to read with corrupted timestamps, returning values from wrong versions (e.g., expected val002-011 but got val002-017)
  • The fix stores ReadOptions in the iterator struct and calls Destroy() on Close(), ensuring the timestamp data remains valid for the iterator's lifetime
  • Also fixes the same ReadOptions leak in getSlice() (used by Get/Has)

Test plan

  • TestDatabaseParallelIterationVersions — the previously flaky test should now pass consistently
  • All other RocksDB tests unaffected (no API changes beyond the internal plumbing)

🤖 Generated with Claude Code

@github-actions
Copy link

github-actions bot commented Feb 19, 2026

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedFeb 19, 2026, 8:58 PM

@pdrobnjak pdrobnjak force-pushed the pd/fix-rocks-db-ci-fail branch from 5a7372d to 65d9a47 Compare February 19, 2026 09:33
@codecov
Copy link

codecov bot commented Feb 19, 2026

Codecov Report

❌ Patch coverage is 96.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 57.70%. Comparing base (8cf29d8) to head (f2d7ae3).

Files with missing lines Patch % Lines
sei-db/db_engine/rocksdb/mvcc/db.go 90.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #2928       +/-   ##
===========================================
- Coverage   68.42%   57.70%   -10.72%     
===========================================
  Files           5     2111     +2106     
  Lines         456   174218   +173762     
===========================================
+ Hits          312   100532   +100220     
- Misses        114    64698    +64584     
- Partials       30     8988     +8958     
Flag Coverage Δ
sei-chain 57.67% <ø> (?)
sei-db 69.18% <96.66%> (+0.75%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
sei-db/db_engine/rocksdb/mvcc/iterator.go 86.31% <100.00%> (+0.76%) ⬆️
sei-db/db_engine/rocksdb/mvcc/db.go 58.23% <90.00%> (+0.91%) ⬆️

... and 2106 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

The ReadOptions (and its timestamp slice) created by newTSReadOptions()
was not stored after being passed to NewIteratorCF(), making it eligible
for GC while the C++ iterator still held a dangling Slice pointer to the
timestamp data. Under parallel access this caused iterators to read with
corrupted timestamps, returning values from wrong versions.

Store ReadOptions in the iterator struct and Destroy() it on Close().
Also fix the same leak in getSlice() used by Get/Has.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@pdrobnjak pdrobnjak enabled auto-merge (squash) February 19, 2026 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments