Skip to content

refactor(sei-db): return cleanup closure from newTSReadOptions#2929

Open
pdrobnjak wants to merge 1 commit intopd/fix-rocks-db-ci-failfrom
pd/rocksdb-readopts-lifecycle
Open

refactor(sei-db): return cleanup closure from newTSReadOptions#2929
pdrobnjak wants to merge 1 commit intopd/fix-rocks-db-ci-failfrom
pd/rocksdb-readopts-lifecycle

Conversation

@pdrobnjak
Copy link
Contributor

Stacked on #2928.

Summary

  • Pushes ReadOptions creation into the iterator constructors so callers never handle ReadOptions directly — eliminates the possibility of forgetting to Destroy() them
  • newIterator() creates a versioned iterator with internally managed ReadOptions, used by Iterator() and ReverseIterator()
  • newRangeIterator() creates a timestamp-range iterator with internally managed ReadOptions, used by RawIterate()
  • newTSReadOptions() now returns a cleanup closure alongside the ReadOptions — for non-iterator uses (getSlice), the compiler forces you to either use or explicitly discard the second return value
  • The iterator stores a cleanup func() instead of *ReadOptions, called on Close()

Test plan

  • No behavioral changes — purely structural refactor
  • All RocksDB tests should pass (same constructors, same lifecycle, just encapsulated)

🤖 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, 10:14 AM

@codecov
Copy link

codecov bot commented Feb 19, 2026

Codecov Report

❌ Patch coverage is 96.55172% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 57.34%. Comparing base (8f3bc86) to head (73de9e4).

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

Impacted file tree graph

@@                   Coverage Diff                    @@
##           pd/fix-rocks-db-ci-fail    #2929   +/-   ##
========================================================
  Coverage                    57.34%   57.34%           
========================================================
  Files                         2095     2095           
  Lines                       172896   172896           
========================================================
+ Hits                         99142    99145    +3     
- Misses                       64867    64868    +1     
+ Partials                      8887     8883    -4     
Flag Coverage Δ
sei-chain 52.84% <ø> (+<0.01%) ⬆️
sei-cosmos 48.19% <ø> (ø)
sei-db 69.18% <96.55%> (ø)

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%> (ø)
sei-db/db_engine/rocksdb/mvcc/db.go 58.23% <88.88%> (ø)

... and 29 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.

newTSReadOptions now returns a cleanup function alongside the
ReadOptions, making it harder to forget Destroy() — the compiler
forces callers to use or explicitly discard the second return value.

The iterator stores a cleanup func() instead of *ReadOptions,
called on Close().

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@pdrobnjak pdrobnjak force-pushed the pd/rocksdb-readopts-lifecycle branch from 936b2f7 to 73de9e4 Compare February 19, 2026 10:13
@pdrobnjak pdrobnjak changed the title refactor(sei-db): encapsulate ReadOptions lifecycle in iterator constructors refactor(sei-db): return cleanup closure from newTSReadOptions Feb 19, 2026
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.

1 participant

Comments