-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Change erasure set size to 8 #4129
Change erasure set size to 8 #4129
Conversation
change tests to be agnostic to exact set size, NUM_DATA vs NUM_CODING. Add convenience methods for setting presence
core/src/erasure.rs
Outdated
@@ -49,7 +49,7 @@ use reed_solomon_erasure::ReedSolomon; | |||
|
|||
//TODO(sakridge) pick these values | |||
/// Number of data blobs | |||
pub const NUM_DATA: usize = 16; | |||
pub const NUM_DATA: usize = 8; | |||
/// Number of coding blobs; also the maximum number that can go missing. | |||
pub const NUM_CODING: usize = 4; |
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.
change NUM_CODING to 8, also
is there a bug addressed anywhere in here? |
Just made it #4130 |
Codecov Report
@@ Coverage Diff @@
## master #4129 +/- ##
========================================
- Coverage 77.7% 77.7% -0.1%
========================================
Files 164 164
Lines 27966 27967 +1
========================================
- Hits 21753 21752 -1
- Misses 6213 6215 +2 |
I don't mean that, I mean are there any changes to the logic such that we'll start to see erasures triggering now? or are you thinking that we're not seeing it being triggered because testnet-edge is basically idle? |
Remove assumptions about NUM_CODING from tests
@rob-solana We weren't seeing any trigger because the testnet was basically idle and the first blobs in an empty slot are never enough to do recovery, so erasure could never be useful or triggered for slots that represent idle time. With this change, recovery won't trigger unless some of those first blobs in a slot gets dropped, but now it's possible. Also tests aren't fragile to future changes. Otherwise no functional change should be present. |
@mark-solana can we add some dynamic erasure sizes? If the pipeline is full, use a higher ratio and count. 64:64 is less likely to fail than 8:8 x 8 |
@aeyakovenko That's up next. I'll make an issue for dynamic erasure set sizing and make a separate PR for that. Edit: Issue #4133 |
* Change erasure set size to 8:8 * Change tests to be agnostic to exact set size and ratio * Add convenience methods for setting presence Change NUM_CODING to 8 Remove assumptions about NUM_CODING from tests
Problem
With erasure set size of 16, empty slots consist of incomplete erasure sets and cause more erasure checks than necessary and confusing logs and metrics.
Summary of Changes
Change the number of data blobs in an erasure set to 8 from 16.
Hide the
data
andcoding
fields of the erasure metadata to prevent dependency on how exactly it's storedChange tests to be agnostic to NUM_DATA and NUM_CODING.
Add convenience methods for setting presence on multiple indexes at once
Fixes #4130