From eacb9183d42cc619a0a2c3db85bbce950b6a012a Mon Sep 17 00:00:00 2001 From: behzad nouri Date: Thu, 16 Jun 2022 13:59:15 +0000 Subject: [PATCH] patches bug where the 1st coding shred is not inserted into blockstore (#25916) StandardBroadcastRun::insert skips 1st shred with index zero because the 1st *data* shred is inserted synchronously: https://github.com/solana-labs/solana/blob/53695ecd2/core/src/broadcast_stage/standard_broadcast_run.rs#L239-L246 https://github.com/solana-labs/solana/blob/53695ecd2/core/src/broadcast_stage/standard_broadcast_run.rs#L334-L339 https://github.com/solana-labs/solana/pull/7481 which added this code was not inserting coding shreds into blockstore. Starting with https://github.com/solana-labs/solana/pull/8899 coding shreds are inserted into blockstore as well as data shreds, but the insert logic erroneously skips first coding shred because it does not check if shred is code or data. --- .../broadcast_stage/standard_broadcast_run.rs | 43 ++++++++++++------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/core/src/broadcast_stage/standard_broadcast_run.rs b/core/src/broadcast_stage/standard_broadcast_run.rs index b43362cf98..3acb1dcc57 100644 --- a/core/src/broadcast_stage/standard_broadcast_run.rs +++ b/core/src/broadcast_stage/standard_broadcast_run.rs @@ -236,13 +236,21 @@ impl StandardBroadcastRun { is_last_in_slot, &mut process_stats, ); - // Insert the first shred so blockstore stores that the leader started this block - // This must be done before the blocks are sent out over the wire. - if !data_shreds.is_empty() && data_shreds[0].index() == 0 { - let first = vec![data_shreds[0].clone()]; - blockstore - .insert_shreds(first, None, true) - .expect("Failed to insert shreds in blockstore"); + // Insert the first data shred synchronously so that blockstore stores + // that the leader started this block. This must be done before the + // blocks are sent out over the wire. By contrast Self::insert skips + // the 1st data shred with index zero. + // https://github.com/solana-labs/solana/blob/53695ecd2/core/src/broadcast_stage/standard_broadcast_run.rs#L334-L339 + if let Some(shred) = data_shreds.first() { + if shred.index() == 0 { + blockstore + .insert_shreds( + vec![shred.clone()], + None, // leader_schedule + true, // is_trusted + ) + .expect("Failed to insert shreds in blockstore"); + } } to_shreds_time.stop(); @@ -331,19 +339,24 @@ impl StandardBroadcastRun { ) { // Insert shreds into blockstore let insert_shreds_start = Instant::now(); - // The first shred is inserted synchronously - let data_shreds = if !shreds.is_empty() && shreds[0].index() == 0 { - shreds[1..].to_vec() - } else { - shreds.to_vec() - }; + let mut shreds = Arc::try_unwrap(shreds).unwrap_or_else(|shreds| (*shreds).clone()); + // The first data shred is inserted synchronously. + // https://github.com/solana-labs/solana/blob/53695ecd2/core/src/broadcast_stage/standard_broadcast_run.rs#L239-L246 + if let Some(shred) = shreds.first() { + if shred.is_data() && shred.index() == 0 { + shreds.swap_remove(0); + } + } + let num_shreds = shreds.len(); blockstore - .insert_shreds(data_shreds, None, true) + .insert_shreds( + shreds, /*leader_schedule:*/ None, /*is_trusted:*/ true, + ) .expect("Failed to insert shreds in blockstore"); let insert_shreds_elapsed = insert_shreds_start.elapsed(); let new_insert_shreds_stats = InsertShredsStats { insert_shreds_elapsed: duration_as_us(&insert_shreds_elapsed), - num_shreds: shreds.len(), + num_shreds, }; self.update_insertion_metrics(&new_insert_shreds_stats, &broadcast_shred_batch_info); }