-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Minor refactor on Shreds column family descriptor construction. #23103
Minor refactor on Shreds column family descriptor construction. #23103
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just the one minor thing - other than that, good code de-dupe and LGTM!
ledger/src/blockstore_db.rs
Outdated
warn!( | ||
" is must be greater than {} for RocksFifo.", | ||
FIFO_WRITE_BUFFER_SIZE | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Looks like you missed adding the column name back in when you consolidated:
warn!("{} cf_size must be greater than {} when using ShredStorageType::RocksFifo",
C::NAME, FIFO_WRITE_BUFFER_SIZE);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yep! Thanks for spotting that. Let me fix it!
Pull request has been modified.
Codecov Report
@@ Coverage Diff @@
## master #23103 +/- ##
=========================================
- Coverage 81.3% 81.3% -0.1%
=========================================
Files 564 567 +3
Lines 153290 154368 +1078
=========================================
+ Hits 124726 125554 +828
- Misses 28564 28814 +250 |
Summary of Changes
Avoid duplicate code by having a template function to create shreds column family descriptor.