blockstore: use u32 for fec_set_index in erasure set index store key #34268
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #34268 +/- ##
=========================================
- Coverage 81.9% 81.9% -0.1%
=========================================
Files 819 819
Lines 219893 219892 -1
=========================================
- Hits 180200 180158 -42
- Misses 39693 39734 +41 |
There was a problem hiding this comment.
The commit is good.
But wondering if there is any easy way to change ErasureMeta key to use u32 for fec_set_index. cc @steviez
ledger/src/blockstore.rs
Outdated
| @@ -464,11 +464,12 @@ impl Blockstore { | |||
| } | |||
|
|
|||
| fn erasure_meta(&self, erasure_set: ErasureSetId) -> Result<Option<ErasureMeta>> { | |||
| self.erasure_meta_cf.get(erasure_set.store_key()) | |||
| let (slot, fec_set_index) = erasure_set.store_key(); | |||
| self.erasure_meta_cf.get((slot, fec_set_index as u64)) | |||
There was a problem hiding this comment.
as is not type-safe.
Can we use u64::from here?
ledger/src/blockstore.rs
Outdated
| @@ -1018,11 +1019,12 @@ impl Blockstore { | |||
| )?; | |||
|
|
|||
| for (erasure_set, erasure_meta) in erasure_metas { | |||
| write_batch.put::<cf::ErasureMeta>(erasure_set.store_key(), &erasure_meta)?; | |||
| let (slot, fec_set_index) = erasure_set.store_key(); | |||
| write_batch.put::<cf::ErasureMeta>((slot, fec_set_index as u64), &erasure_meta)?; | |||
There was a problem hiding this comment.
Similarly here re as -> u64::from.
|
|
||
| pub(crate) fn key(&self) -> (Slot, /*fec_set_index:*/ u32) { | ||
| // Storage key for ErasureMeta and MerkleRootMeta in blockstore db. | ||
| pub(crate) fn store_key(&self) -> (Slot, /*fec_set_index:*/ u32) { |
There was a problem hiding this comment.
Can you please add a comment here that ErasureMeta uses u64 for fec_set_index?
Not so easy unfortunately due to backwards compatibility concerns. Tyera did something recently, altho she did it while she was explicitly fixing another bug: #33419 The basic gist is you need to write code that is capable of handling both key formats (one with |
AshwinSekar
force-pushed
the
erasure-set-id-store-key
branch
from
4a07245 to
2461df4
Compare
Problem
fec_set_indexisu32but in some places we useu64which makes the code confusing.Summary of Changes
Unify the type of
store_keyinErasureSetIdtou32and perform the type conversion at the call site.Contributes to #33644