Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

Change erasure set size to 8 #4129

Merged
merged 2 commits into from
May 3, 2019

Conversation

mark-solana
Copy link
Contributor

@mark-solana mark-solana commented May 2, 2019

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 and coding fields of the erasure metadata to prevent dependency on how exactly it's stored

Change tests to be agnostic to NUM_DATA and NUM_CODING.

Add convenience methods for setting presence on multiple indexes at once

Fixes #4130

change tests to be agnostic to exact set size, NUM_DATA vs NUM_CODING.

Add convenience methods for setting presence
@mark-solana mark-solana requested a review from rob-solana May 2, 2019 21:33
@@ -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;
Copy link
Contributor

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

@rob-solana
Copy link
Contributor

is there a bug addressed anywhere in here?

@mark-solana
Copy link
Contributor Author

is there a bug addressed anywhere in here?

Just made it #4130

@codecov
Copy link

codecov bot commented May 2, 2019

Codecov Report

Merging #4129 into master will decrease coverage by <.1%.
The diff coverage is 94.3%.

@@           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

@rob-solana
Copy link
Contributor

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
@mark-solana
Copy link
Contributor Author

mark-solana commented May 2, 2019

@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.

@aeyakovenko
Copy link
Member

@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

@mark-solana
Copy link
Contributor Author

mark-solana commented May 2, 2019

@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

@mark-solana mark-solana merged commit 916458e into solana-labs:master May 3, 2019
mark-solana added a commit to mark-solana/solana that referenced this pull request May 3, 2019
* 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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Erasure set size makes empty slots incomplete
3 participants