Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: RocksDB storage and self-contained RevIndex with internal storage #3250

Merged
merged 17 commits into from
Jul 27, 2024

Conversation

luizirber
Copy link
Member

@luizirber luizirber commented Jul 13, 2024

Implement a RocksDB storage for making a self-contained RevIndex (containing both the revindex and the sigs needed for gather) and support more flexible RocksDB sketch storage.

  • Remove branchwater dependency on rkyv, make it optional
  • Move most RocksDB initialization to the new storage::rocksdb module
  • Disable prepare_for_bulk_load, it is a footgun for large index construction
  • make sourmash::storage::StorageError non-exhaustive
  • remove unused StorageInfo (rust lints beta CI check is failing on latest #3261)

Copy link

codecov bot commented Jul 13, 2024

Codecov Report

Attention: Patch coverage is 88.88889% with 9 lines in your changes missing coverage. Please review.

Project coverage is 86.75%. Comparing base (f621726) to head (da76ba9).
Report is 64 commits behind head on latest.

Files with missing lines Patch % Lines
src/core/src/storage/mod.rs 63.63% 4 Missing ⚠️
src/core/src/storage/rocksdb.rs 93.87% 3 Missing ⚠️
src/core/src/collection.rs 50.00% 1 Missing ⚠️
src/core/src/index/revindex/disk_revindex.rs 94.44% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           latest    #3250      +/-   ##
==========================================
+ Coverage   86.74%   86.75%   +0.01%     
==========================================
  Files         136      137       +1     
  Lines       15873    15920      +47     
  Branches     2728     2728              
==========================================
+ Hits        13769    13812      +43     
- Misses       1795     1799       +4     
  Partials      309      309              
Flag Coverage Δ
hypothesis-py 25.40% <ø> (ø)
python 92.37% <ø> (ø)
rust 62.67% <88.88%> (+0.45%) ⬆️

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

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

@ctb
Copy link
Contributor

ctb commented Jul 14, 2024

(may I suggest renaming this to something like "support more flexible RocksDB sketch storage, including internal stores?" :)

@luizirber luizirber force-pushed the lirber/rocksdb_storage branch from c4da16d to 6684126 Compare July 14, 2024 21:31
@luizirber luizirber force-pushed the lirber/rocksdb_storage branch from e57927d to dc2dc3c Compare July 24, 2024 01:23
@luizirber luizirber changed the title feat: RocksDB storage feat: RocksDB storage and self-contained RevIndex with internal storage Jul 24, 2024
@luizirber luizirber marked this pull request as ready for review July 24, 2024 04:42
@luizirber luizirber requested a review from a team July 24, 2024 04:47
@luizirber luizirber merged commit d516811 into latest Jul 27, 2024
41 checks passed
@luizirber luizirber deleted the lirber/rocksdb_storage branch July 27, 2024 22:38
luizirber added a commit that referenced this pull request Jul 31, 2024
Minor new features:

- RocksDB storage and self-contained RevIndex with internal storage (#3250)

Bug fixes:

Cleanup and documentation updates:

- Update dev env instructions (#2990)

Developer updates:

- Enable codspeed for Rust perf tracking (#3231)
- bump cibuildwheel for linux fixes, move wasm to use cibuildwheel too (#3241)

Dependabot updates:

- Break ahash dep cycle by bumping to 0.7.8, nix flake update (#3243)
- pre-commit autoupdate (#3270) (#3260) (#3255) (#3238)
- Bump DeterminateSystems/nix-installer-action from 12 to 13 (#3259)
- Update pytest requirement from <8.3.0,>=6.2.4 to >=6.2.4,<8.4.0 (#3258)
- Bump thiserror from 1.0.62 to 1.0.63 (#3257)
- Bump thiserror from 1.0.61 to 1.0.62 (#3254)
- Bump roaring from 0.10.5 to 0.10.6 (#3245)
- Bump serde from 1.0.203 to 1.0.204 (#3244)
- Bump counter from 0.5.7 to 0.6.0 (#3235)
- Bump log from 0.4.21 to 0.4.22 (#3236)
- Bump serde_json from 1.0.117 to 1.0.120 (#3234)
luizirber added a commit that referenced this pull request Jul 31, 2024
Minor new features:

- RocksDB storage and self-contained RevIndex with internal storage
(#3250)

Bug fixes:

- Break ahash dep cycle by bumping to 0.7.8, nix flake update (#3243)

Cleanup and documentation updates:

- Update dev env instructions (#2990)

Developer updates:

- Enable codspeed for Rust perf tracking (#3231)
- bump cibuildwheel for linux fixes, move wasm to use cibuildwheel too
(#3241)

Dependabot updates:

- Bump actions/checkout from 3 to 4 (#3265)
- Bump moonrepo/setup-rust from 0 to 1 (#3266)
- Bump CodSpeedHQ/action from 2 to 3 (#3264)
- pre-commit autoupdate (#3270) (#3260) (#3255) (#3238)
- Bump DeterminateSystems/nix-installer-action from 12 to 13 (#3259)
- Update pytest requirement from <8.3.0,>=6.2.4 to >=6.2.4,<8.4.0
(#3258)
- Bump thiserror from 1.0.62 to 1.0.63 (#3257)
- Bump thiserror from 1.0.61 to 1.0.62 (#3254)
- Bump roaring from 0.10.5 to 0.10.6 (#3245)
- Bump serde from 1.0.203 to 1.0.204 (#3244)
- Bump counter from 0.5.7 to 0.6.0 (#3235)
- Bump log from 0.4.21 to 0.4.22 (#3236)
- Bump serde_json from 1.0.117 to 1.0.120 (#3234)
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