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

buffers data shreds to make larger erasure coded sets #15849

Merged
merged 2 commits into from
Mar 23, 2021

Conversation

behzadnouri
Copy link
Contributor

@behzadnouri behzadnouri commented Mar 14, 2021

Problem

Broadcast stage batches up to 8 entries:
https://github.com/solana-labs/solana/blob/79280b304/core/src/broadcast_stage/broadcast_utils.rs#L26-L29
which will be serialized into some number of shreds and chunked into FEC
sets of at most 32 shreds each:
https://github.com/solana-labs/solana/blob/79280b304/ledger/src/shred.rs#L576-L597
So depending on the size of entries, FEC sets can be small, which may
aggravate loss rate.
For example 16 FEC sets of 2:2 data/code shreds each have higher loss
rate than one 32:32 set.

Summary of Changes

This commit broadcasts data shreds immediately, but also buffers them
until it has a batch of 32 data shreds, at which point 32 coding shreds
are generated and broadcasted.

@codecov
Copy link

codecov bot commented Mar 14, 2021

Codecov Report

Merging #15849 (8d9c7f9) into master (ff2242d) will increase coverage by 0.0%.
The diff coverage is 88.8%.

@@           Coverage Diff            @@
##           master   #15849    +/-   ##
========================================
  Coverage    79.9%    79.9%            
========================================
  Files         409      409            
  Lines      107723   107822    +99     
========================================
+ Hits        86142    86245   +103     
+ Misses      21581    21577     -4     

@behzadnouri behzadnouri force-pushed the erasure-batch branch 3 times, most recently from 9cb0653 to 6c13c4b Compare March 15, 2021 17:36
@aeyakovenko
Copy link
Member

does this accumulate data shreds until we see 32?

@behzadnouri
Copy link
Contributor Author

does this accumulate data shreds until we see 32?

@aeyakovenko yes, data_shreds_buffer is buffering data-shreds, and make_coding_shreds is consuming from the buffer a multiple of MAX_DATA_SHREDS_PER_FEC_BLOCK which is 32.

@behzadnouri behzadnouri force-pushed the erasure-batch branch 5 times, most recently from ec89a27 to 9e52898 Compare March 16, 2021 17:38
@behzadnouri behzadnouri changed the title batches FEC sets to 32:32 shreds each buffers data shreds to make larger erasure coded sets Mar 16, 2021
@behzadnouri behzadnouri marked this pull request as ready for review March 16, 2021 17:39
@aeyakovenko
Copy link
Member

aeyakovenko commented Mar 17, 2021

once this works, we should be able to add a new confirmation status
https://docs.solana.com/developing/clients/jsonrpc-api#configuring-state-commitment

  • executed - Each of the turbine batches have over 51% of the stake successfully retransmitting the shreds + this block is attached to an optimistic or executed block + this block was replayed successfully without an error + no forks ahead of the last optimistic block (tag @carllin)

I think with these assumptions we can bet that all the other nodes will also execute this block and vote before a fork can form and this block will be optimistically confirmed.

}
None => Vec::default(),
};
data_shreds_buffer.extend(data_shreds.clone());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice! could we add a simple test entries_to_data_shreds() -> make_coding_shreds() with a few iterative batches and then check at the end that one big batch was made?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added more test coverage checking the batching behavior.

Comment on lines +113 to +117
None => match blockstore.meta(slot).unwrap() {
Some(slot_meta) => {
let shreds_consumed = slot_meta.consumed as u32;
(shreds_consumed, shreds_consumed)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we may actually be able to remove this outdated case, since if the blockstore has metadata for that slot, it should not recreate the slot due to this check https://github.com/solana-labs/solana/blob/master/ledger/src/leader_schedule_cache.rs#L141-L144

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are probably, but I think we better have a separate commit for that so that this one does not change the logic here, just in case there are unknown consequences.

@@ -418,6 +469,8 @@ impl BroadcastRun for StandardBroadcastRun {
blockstore_sender: &Sender<(Arc<Vec<Shred>>, Option<BroadcastShredBatchInfo>)>,
) -> Result<()> {
let receive_results = broadcast_utils::recv_slot_entries(receiver)?;
// TODO: Confirm that last chunk of coding shreds
Copy link
Contributor

@carllin carllin Mar 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good way to confirm this might be by monitoring the broadcast-transmit-shreds-stats metric, end_to_end_elapsed field. It doesn't look like it should be too big of a deal because even with these changes, it looks like at most we will have at most an additional MAX_DATA_SHREDS_PER_FEC_BLOCK data shreds to generate coding shreds from on the last iteration of process_receive_results for this slot compared to the code today (assuming it's not an interrupted slot, interrupted slots may take forever because they wait for the next slot, but that's true even today 😃 )


// Create and send coding shreds
let coding_shreds = shredder
.data_shreds_to_coding_shreds(&data_shreds[0..last_data_shred], &mut process_stats);
let coding_shreds = make_coding_shreds(
Copy link
Contributor

@carllin carllin Mar 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, this is a clean way to avoid changing the num_batches and num_expected_batches logic above because it always returns a vector, even if that vector is empty.

@carllin
Copy link
Contributor

carllin commented Mar 18, 2021

@behzadnouri whoo, finally knocking this one out! Really clean work 🧠 , just a few nits 😃

Broadcast stage batches up to 8 entries:
https://github.com/solana-labs/solana/blob/79280b304/core/src/broadcast_stage/broadcast_utils.rs#L26-L29
which will be serialized into some number of shreds and chunked into FEC
sets of at most 32 shreds each:
https://github.com/solana-labs/solana/blob/79280b304/ledger/src/shred.rs#L576-L597
So depending on the size of entries, FEC sets can be small, which may
aggravate loss rate.
For example 16 FEC sets of 2:2 data/code shreds each have higher loss
rate than one 32:32 set.

This commit broadcasts data shreds immediately, but also buffers them
until it has a batch of 32 data shreds, at which point 32 coding shreds
are generated and broadcasted.
@behzadnouri behzadnouri merged commit 4f82b89 into solana-labs:master Mar 23, 2021
@behzadnouri behzadnouri deleted the erasure-batch branch March 23, 2021 14:52
mergify bot pushed a commit that referenced this pull request Mar 23, 2021
Broadcast stage batches up to 8 entries:
https://github.com/solana-labs/solana/blob/79280b304/core/src/broadcast_stage/broadcast_utils.rs#L26-L29
which will be serialized into some number of shreds and chunked into FEC
sets of at most 32 shreds each:
https://github.com/solana-labs/solana/blob/79280b304/ledger/src/shred.rs#L576-L597
So depending on the size of entries, FEC sets can be small, which may
aggravate loss rate.
For example 16 FEC sets of 2:2 data/code shreds each have higher loss
rate than one 32:32 set.

This commit broadcasts data shreds immediately, but also buffers them
until it has a batch of 32 data shreds, at which point 32 coding shreds
are generated and broadcasted.

(cherry picked from commit 4f82b89)

# Conflicts:
#	ledger/src/shred.rs
jackcmay pushed a commit that referenced this pull request Mar 23, 2021
Broadcast stage batches up to 8 entries:
https://github.com/solana-labs/solana/blob/79280b304/core/src/broadcast_stage/broadcast_utils.rs#L26-L29
which will be serialized into some number of shreds and chunked into FEC
sets of at most 32 shreds each:
https://github.com/solana-labs/solana/blob/79280b304/ledger/src/shred.rs#L576-L597
So depending on the size of entries, FEC sets can be small, which may
aggravate loss rate.
For example 16 FEC sets of 2:2 data/code shreds each have higher loss
rate than one 32:32 set.

This commit broadcasts data shreds immediately, but also buffers them
until it has a batch of 32 data shreds, at which point 32 coding shreds
are generated and broadcasted.

(cherry picked from commit 4f82b89)

# Conflicts:
#	ledger/src/shred.rs
mergify bot added a commit that referenced this pull request Mar 23, 2021
…6074)

* buffers data shreds to make larger erasure coded sets (#15849)

Broadcast stage batches up to 8 entries:
https://github.com/solana-labs/solana/blob/79280b304/core/src/broadcast_stage/broadcast_utils.rs#L26-L29
which will be serialized into some number of shreds and chunked into FEC
sets of at most 32 shreds each:
https://github.com/solana-labs/solana/blob/79280b304/ledger/src/shred.rs#L576-L597
So depending on the size of entries, FEC sets can be small, which may
aggravate loss rate.
For example 16 FEC sets of 2:2 data/code shreds each have higher loss
rate than one 32:32 set.

This commit broadcasts data shreds immediately, but also buffers them
until it has a batch of 32 data shreds, at which point 32 coding shreds
are generated and broadcasted.

(cherry picked from commit 4f82b89)

# Conflicts:
#	ledger/src/shred.rs

* removes backport merge conflicts

Co-authored-by: behzad nouri <behzadnouri@gmail.com>
mergify bot pushed a commit that referenced this pull request Apr 20, 2021
Broadcast stage batches up to 8 entries:
https://github.com/solana-labs/solana/blob/79280b304/core/src/broadcast_stage/broadcast_utils.rs#L26-L29
which will be serialized into some number of shreds and chunked into FEC
sets of at most 32 shreds each:
https://github.com/solana-labs/solana/blob/79280b304/ledger/src/shred.rs#L576-L597
So depending on the size of entries, FEC sets can be small, which may
aggravate loss rate.
For example 16 FEC sets of 2:2 data/code shreds each have higher loss
rate than one 32:32 set.

This commit broadcasts data shreds immediately, but also buffers them
until it has a batch of 32 data shreds, at which point 32 coding shreds
are generated and broadcasted.

(cherry picked from commit 4f82b89)
mergify bot added a commit that referenced this pull request Apr 20, 2021
Broadcast stage batches up to 8 entries:
https://github.com/solana-labs/solana/blob/79280b304/core/src/broadcast_stage/broadcast_utils.rs#L26-L29
which will be serialized into some number of shreds and chunked into FEC
sets of at most 32 shreds each:
https://github.com/solana-labs/solana/blob/79280b304/ledger/src/shred.rs#L576-L597
So depending on the size of entries, FEC sets can be small, which may
aggravate loss rate.
For example 16 FEC sets of 2:2 data/code shreds each have higher loss
rate than one 32:32 set.

This commit broadcasts data shreds immediately, but also buffers them
until it has a batch of 32 data shreds, at which point 32 coding shreds
are generated and broadcasted.

(cherry picked from commit 4f82b89)

Co-authored-by: behzad nouri <behzadnouri@gmail.com>
@brooksprumo brooksprumo mentioned this pull request Aug 23, 2021
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.

3 participants