From d1188d7c081aa14afd0f3e0cf1cfd79364098564 Mon Sep 17 00:00:00 2001
From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com>
Date: Wed, 2 Feb 2022 17:09:07 +0000
Subject: [PATCH] Keep all erasure coding shreds even on successful recovery
 (#20968) (#21052) (#22890)

Problem
Blockstore currently removes erasure shreds if the data shreds are
successfully recovered on insert, which is an issue if we want to
serve coding shreds over repair.

Summary of Changes
This diff keeps all coding shreds even on successful recovery and
changes change the signature of prev_inserted_codes to immutable
reference to ensure its immunity.

Fixes #20968

(cherry picked from commit 3aa49e2c69a5ef5eefaf8d70cd0740cb822cb53c)

Co-authored-by: Yueh-Hsuan Chiang <93241502+yhchiang-sol@users.noreply.github.com>
---
 ledger/src/blockstore.rs | 27 ++++++---------------------
 1 file changed, 6 insertions(+), 21 deletions(-)

diff --git a/ledger/src/blockstore.rs b/ledger/src/blockstore.rs
index 8d5dd18328..c472d291e0 100644
--- a/ledger/src/blockstore.rs
+++ b/ledger/src/blockstore.rs
@@ -660,17 +660,12 @@ impl Blockstore {
         index: &'a mut Index,
         slot: Slot,
         erasure_meta: &'a ErasureMeta,
-        prev_inserted_codes: &'a mut HashMap<(Slot, /*shred index:*/ u64), Shred>,
+        prev_inserted_codes: &'a HashMap<(Slot, /*shred index:*/ u64), Shred>,
         code_cf: &'a LedgerColumn<cf::ShredCode>,
     ) -> impl Iterator<Item = Shred> + 'a {
         erasure_meta.coding_shreds_indices().filter_map(move |i| {
-            if let Some(shred) = prev_inserted_codes.remove(&(slot, i)) {
-                // Remove from the index so it doesn't get committed. We know
-                // this is safe to do because everything in
-                // `prev_inserted_codes` does not yet exist in blockstore
-                // (guaranteed by `check_cache_coding_shred`)
-                index.coding_mut().set_present(i, false);
-                return Some(shred);
+            if let Some(shred) = prev_inserted_codes.get(&(slot, i)) {
+                return Some(shred.clone());
             }
             if !index.coding().is_present(i) {
                 return None;
@@ -689,7 +684,7 @@ impl Blockstore {
         index: &mut Index,
         erasure_meta: &ErasureMeta,
         prev_inserted_datas: &mut HashMap<(Slot, /*shred index:*/ u64), Shred>,
-        prev_inserted_codes: &mut HashMap<(Slot, /*shred index:*/ u64), Shred>,
+        prev_inserted_codes: &HashMap<(Slot, /*shred index:*/ u64), Shred>,
         recovered_data_shreds: &mut Vec<Shred>,
         data_cf: &LedgerColumn<cf::ShredData>,
         code_cf: &LedgerColumn<cf::ShredCode>,
@@ -740,7 +735,7 @@ impl Blockstore {
         erasure_metas: &HashMap<(Slot, /*fec set index:*/ u64), ErasureMeta>,
         index_working_set: &mut HashMap<u64, IndexMetaWorkingSetEntry>,
         prev_inserted_datas: &mut HashMap<(Slot, /*shred index:*/ u64), Shred>,
-        prev_inserted_codes: &mut HashMap<(Slot, /*shred index:*/ u64), Shred>,
+        prev_inserted_codes: &HashMap<(Slot, /*shred index:*/ u64), Shred>,
     ) -> Vec<Shred> {
         let data_cf = db.column::<cf::ShredData>();
         let code_cf = db.column::<cf::ShredCode>();
@@ -766,16 +761,6 @@ impl Blockstore {
                     );
                 }
                 ErasureMetaStatus::DataFull => {
-                    for i in erasure_meta.coding_shreds_indices() {
-                        // Remove saved coding shreds. We don't need these for future recovery.
-                        if prev_inserted_codes.remove(&(slot, i)).is_some() {
-                            // Remove from the index so it doesn't get committed. We know
-                            // this is safe to do because everything in
-                            // `prev_inserted_codes` does not yet exist in blockstore
-                            // (guaranteed by `check_cache_coding_shred`)
-                            index.coding_mut().set_present(i, false);
-                        }
-                    }
                     Self::submit_metrics(slot, erasure_meta, false, "complete".into(), 0);
                 }
                 ErasureMetaStatus::StillNeed(needed) => {
@@ -886,7 +871,7 @@ impl Blockstore {
                 &erasure_metas,
                 &mut index_working_set,
                 &mut just_inserted_data_shreds,
-                &mut just_inserted_coding_shreds,
+                &just_inserted_coding_shreds,
             );
 
             metrics.num_recovered += recovered_data_shreds.len();