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

Refactoring rocksdb.rs #2938

Merged
merged 7 commits into from
Apr 27, 2024
Merged

Refactoring rocksdb.rs #2938

merged 7 commits into from
Apr 27, 2024

Conversation

yito88
Copy link
Member

@yito88 yito88 commented Mar 21, 2024

Describe your changes

  • Use constants for keys
  • Add helper functions
  • Restore the merkle tree outside of read_last_block
    • It is restored by rebuilding after read_last_block in many cases
  • Use batch for write_subspace_val and delete_subspace_val

Indicate on which release or other PRs this topic is based on

v0.33.0

Checklist before merging to draft

  • I have added a changelog
  • Git history is in acceptable state

Copy link

codecov bot commented Mar 21, 2024

Codecov Report

Attention: Patch coverage is 79.50192% with 107 lines in your changes are missing coverage. Please review.

Project coverage is 59.40%. Comparing base (97ec5b4) to head (fbba381).
Report is 4 commits behind head on main.

Files Patch % Lines
crates/storage/src/mockdb.rs 56.16% 64 Missing ⚠️
crates/apps/src/lib/node/ledger/storage/rocksdb.rs 88.31% 43 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main    #2938    +/-   ##
========================================
  Coverage   59.39%   59.40%            
========================================
  Files         298      298            
  Lines       92771    92478   -293     
========================================
- Hits        55104    54934   -170     
+ Misses      37667    37544   -123     

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

@yito88 yito88 force-pushed the yuji/refactoring-rocksdb branch from 2b1b87a to e68f74b Compare April 11, 2024 19:54
@Fraccaman
Copy link
Member

do u think we could also upgrade rocksdb dependency to 0.22.0? I started doing this is another PR but it was closed due to some other reasons. You can take a look here

@Fraccaman Fraccaman mentioned this pull request Apr 15, 2024
@yito88 yito88 force-pushed the yuji/refactoring-rocksdb branch from e68f74b to f5ed1c3 Compare April 15, 2024 17:18
@Fraccaman
Copy link
Member

is this ready?

@yito88 yito88 requested a review from tzemanovic April 16, 2024 21:41
@Fraccaman Fraccaman marked this pull request as ready for review April 17, 2024 12:20
brentstone added a commit that referenced this pull request Apr 18, 2024
* yuji/refactoring-rocksdb:
  add changelog
  fix after rebasing
  for dump and rollback
  refactor mockdb
  always use batch write
  WIP: refactor add_block_to_batch
  simplify read_last_block
brentstone added a commit that referenced this pull request Apr 18, 2024
* yuji/refactoring-rocksdb:
  add changelog
  fix after rebasing
  for dump and rollback
  refactor mockdb
  always use batch write
  WIP: refactor add_block_to_batch
  simplify read_last_block
@brentstone brentstone merged commit 52c482f into main Apr 27, 2024
18 of 19 checks passed
@brentstone brentstone deleted the yuji/refactoring-rocksdb branch April 27, 2024 00:49
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.

4 participants