blockstore: write only dirty erasure meta and merkle root metas #34269
Conversation
AshwinSekar
force-pushed
the
erasure-meta-dirty
branch
2 times, most recently
from
e149f88 to
4f95a50
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #34269 +/- ##
=======================================
Coverage 81.8% 81.8%
=======================================
Files 824 824
Lines 222128 222151 +23
=======================================
+ Hits 181742 181789 +47
+ Misses 40386 40362 -24 |
ledger/src/blockstore.rs
Outdated
| @@ -731,7 +731,7 @@ impl Blockstore { | |||
|
|
|||
| fn try_shred_recovery( | |||
| &self, | |||
| erasure_metas: &HashMap<ErasureSetId, ErasureMeta>, | |||
| erasure_metas: &HashMap<ErasureSetId, (ErasureMeta, bool)>, | |||
There was a problem hiding this comment.
Can we use an enum instead of bool? something like
enum Wrapper<T> {
New(T),
Old(T),
}
AshwinSekar
force-pushed
the
erasure-meta-dirty
branch
from
4f95a50 to
a9701c4
Compare
ledger/src/blockstore.rs
Outdated
| Dirty(T), | ||
| Clean(T), |
There was a problem hiding this comment.
Can you please add a comment here what each represents?
Clean means we read from blockstore, right?
Dirty means it should be stored to blockstore?!
ledger/src/blockstore.rs
Outdated
| fn unwrap(&self) -> &T { | ||
| match self { | ||
| Self::Dirty(value) => value, | ||
| Self::Clean(value) => value, | ||
| } | ||
| } |
There was a problem hiding this comment.
This seems a confusing overload of unwrap.
Can we instead implement From?
https://doc.rust-lang.org/std/convert/trait.From.html
something like:
impl<T> From<WorkingStatus<T>> for T { ...There was a problem hiding this comment.
I can't implement From as it violates the orphan rule. I've changed the names a bit to WorkingEntry and fn value hopefully that's less confusing.
AshwinSekar
force-pushed
the
erasure-meta-dirty
branch
from
a9701c4 to
28d7a20
Compare
ledger/src/blockstore.rs
Outdated
| /// Returns a reference to the underlying value | ||
| fn value(&self) -> &T { | ||
| match self { | ||
| Self::Dirty(value) => value, | ||
| Self::Clean(value) => value, | ||
| } | ||
| } |
There was a problem hiding this comment.
This could implement AsRef instead:
https://doc.rust-lang.org/std/convert/trait.AsRef.html
AshwinSekar
force-pushed
the
erasure-meta-dirty
branch
from
28d7a20 to
bf107c9
Compare
| write_batch.put::<cf::ErasureMeta>((slot, u64::from(fec_set_index)), &erasure_meta)?; | ||
| write_batch.put::<cf::ErasureMeta>( | ||
| (slot, u64::from(fec_set_index)), | ||
| working_erasure_meta.as_ref(), |
There was a problem hiding this comment.
I think you can just write & working_erasure_meta.
& will do the .as_ref().
There was a problem hiding this comment.
hmm it doesn't seem to work it only recognizes it as a &WorkingEntry<ErasureMeta>
Problem
We commit the in memory maps to blockstore every iteration which results in a lot of rewrites.
Summary of Changes
Explicitly track which entries are dirty and only commit those.
Contributes to #33644