More documentation + small refactor for RepairService (#28933)

core/src/repair_generic_traversal.rs

@@ -40,6 +40,8 @@ impl<'a> Iterator for GenericTraversal<'a> {
     }
 }
 
+/// Does a generic traversal and inserts all slots that have a missing last index prioritized by how
+/// many shreds have been received
 pub fn get_unknown_last_index(
     tree: &HeaviestSubtreeForkChoice,
     blockstore: &Blockstore,
@@ -78,6 +80,8 @@ pub fn get_unknown_last_index(
         .collect()
 }
 
+/// Path of broken parents from start_slot to earliest ancestor not yet seen
+/// Uses blockstore for fork information
 fn get_unrepaired_path(
     start_slot: Slot,
     blockstore: &Blockstore,
@@ -103,6 +107,8 @@ fn get_unrepaired_path(
     path
 }
 
+/// Finds repairs for slots that are closest to completion (# of missing shreds).
+/// Additionaly we repair up to their oldest full ancestor (using blockstore fork info).
 pub fn get_closest_completion(
     tree: &HeaviestSubtreeForkChoice,
     blockstore: &Blockstore,

core/src/repair_service.rs

@@ -8,7 +8,6 @@ use {
         duplicate_repair_status::DuplicateSlotRepairStatus,
         outstanding_requests::OutstandingRequests,
         repair_weight::RepairWeight,
-        result::Result,
         serve_repair::{ServeRepair, ShredRepairType, REPAIR_PEERS_CACHE_CAPACITY},
     },
     crossbeam_channel::{Receiver as CrossbeamReceiver, Sender as CrossbeamSender},
@@ -481,39 +480,7 @@ impl RepairService {
         }
     }
 
-    // Generate repairs for all slots `x` in the repair_range.start <= x <= repair_range.end
+    /// If this slot is missing shreds generate repairs
-    pub fn generate_repairs_in_range(
-        blockstore: &Blockstore,
-        max_repairs: usize,
-        repair_range: &RepairSlotRange,
-    ) -> Result<Vec<ShredRepairType>> {
-        // Slot height and shred indexes for shreds we want to repair
-        let mut repairs: Vec<ShredRepairType> = vec![];
-        for slot in repair_range.start..=repair_range.end {
-            if repairs.len() >= max_repairs {
-                break;
-            }
-
-            let meta = blockstore
-                .meta(slot)
-                .expect("Unable to lookup slot meta")
-                .unwrap_or(SlotMeta {
-                    slot,
-                    ..SlotMeta::default()
-                });
-
-            let new_repairs = Self::generate_repairs_for_slot(
-                blockstore,
-                slot,
-                &meta,
-                max_repairs - repairs.len(),
-            );
-            repairs.extend(new_repairs);
-        }
-
-        Ok(repairs)
-    }
-
     pub fn generate_repairs_for_slot(
         blockstore: &Blockstore,
         slot: Slot,
@@ -538,7 +505,7 @@ impl RepairService {
         }
     }
 
-    /// Repairs any fork starting at the input slot
+    /// Repairs any fork starting at the input slot (uses blockstore for fork info)
     pub fn generate_repairs_for_fork<'a>(
         blockstore: &Blockstore,
         repairs: &mut Vec<ShredRepairType>,
@@ -569,6 +536,40 @@ impl RepairService {
         }
     }
 
+    /// Generate repairs for all slots `x` in the repair_range.start <= x <= repair_range.end
+    #[cfg(test)]
+    pub fn generate_repairs_in_range(
+        blockstore: &Blockstore,
+        max_repairs: usize,
+        repair_range: &RepairSlotRange,
+    ) -> crate::result::Result<Vec<ShredRepairType>> {
+        // Slot height and shred indexes for shreds we want to repair
+        let mut repairs: Vec<ShredRepairType> = vec![];
+        for slot in repair_range.start..=repair_range.end {
+            if repairs.len() >= max_repairs {
+                break;
+            }
+
+            let meta = blockstore
+                .meta(slot)
+                .expect("Unable to lookup slot meta")
+                .unwrap_or(SlotMeta {
+                    slot,
+                    ..SlotMeta::default()
+                });
+
+            let new_repairs = Self::generate_repairs_for_slot(
+                blockstore,
+                slot,
+                &meta,
+                max_repairs - repairs.len(),
+            );
+            repairs.extend(new_repairs);
+        }
+
+        Ok(repairs)
+    }
+
     #[cfg(test)]
     fn generate_duplicate_repairs_for_slot(
         blockstore: &Blockstore,
@@ -657,7 +658,7 @@ impl RepairService {
         repair_stats: &mut RepairStats,
         nonce: Nonce,
         identity_keypair: &Keypair,
-    ) -> Result<()> {
+    ) -> crate::result::Result<()> {
         let req = serve_repair.map_repair_request(
             repair_type,
             repair_pubkey,

core/src/repair_weight.rs

@@ -196,6 +196,11 @@ impl RepairWeight {
         repairs.extend(best_shreds_repairs);
         get_best_shreds_elapsed.stop();
 
+        // Although we have generated repairs for orphan roots and slots in the rooted subtree,
+        // if we have space we should generate repairs for slots in orphan trees in preparation for
+        // when they are no longer rooted. Here we generate repairs for slots with unknown last
+        // indices as well as slots that are close to completion.
+
         let mut get_unknown_last_index_elapsed = Measure::start("get_unknown_last_index");
         let pre_num_slots = processed_slots.len();
         let unknown_last_index_repairs = self.get_best_unknown_last_index(
@@ -314,7 +319,7 @@ impl RepairWeight {
         self.root = new_root;
     }
 
-    // Generate shred repairs for main subtree rooted at `self.slot`
+    // Generate shred repairs for main subtree rooted at `self.root`
     fn get_best_shreds<'a>(
         &mut self,
         blockstore: &Blockstore,
@@ -403,6 +408,8 @@ impl RepairWeight {
         }
     }
 
+    /// For all remaining trees (orphan and rooted), generate repairs for slots missing last_index info
+    /// prioritized by # shreds received.
     fn get_best_unknown_last_index(
         &mut self,
         blockstore: &Blockstore,
@@ -427,6 +434,10 @@ impl RepairWeight {
         repairs
     }
 
+    /// For all remaining trees (orphan and rooted), generate repairs for subtrees that have last
+    /// index info but are missing shreds prioritized by how close to completion they are. These
+    /// repairs are also prioritized by age of ancestors, so slots close to completion will first
+    /// start by repairing broken ancestors.
     fn get_best_closest_completion(
         &mut self,
         blockstore: &Blockstore,
@@ -451,9 +462,9 @@ impl RepairWeight {
         repairs
     }
 
-    // Attempts to chain the orphan subtree rooted at `orphan_tree_root`
+    /// Attempts to chain the orphan subtree rooted at `orphan_tree_root`
-    // to any earlier subtree with new ancestry information in `blockstore`.
+    /// to any earlier subtree with new ancestry information in `blockstore`.
-    // Returns the earliest known ancestor of `heaviest_tree_root`.
+    /// Returns the earliest known ancestor of `heaviest_tree_root`.
     fn update_orphan_ancestors(
         &mut self,
         blockstore: &Blockstore,

core/src/repair_weighted_traversal.rs

@@ -71,7 +71,9 @@ impl<'a> Iterator for RepairWeightTraversal<'a> {
     }
 }
 
-// Generate shred repairs for main subtree rooted at `self.slot`
+/// Generate shred repairs for `tree` starting at `tree.root`.
+/// Prioritized by stake weight, additionally considers children not present in `tree` but in
+/// blockstore.
 pub fn get_best_repair_shreds<'a>(
     tree: &HeaviestSubtreeForkChoice,
     blockstore: &Blockstore,

core/src/serve_repair.rs

@@ -86,8 +86,11 @@ static_assertions::const_assert_eq!(MAX_ANCESTOR_RESPONSES, 30);
 
 #[derive(Serialize, Deserialize, Debug, Clone, Copy, Hash, PartialEq, Eq)]
 pub enum ShredRepairType {
+    /// Requesting `MAX_ORPHAN_REPAIR_RESPONSES ` parent shreds
     Orphan(Slot),
+    /// Requesting any shred with index greater than or equal to the particular index
     HighestShred(Slot, u64),
+    /// Requesting the missing shred at a particular index
     Shred(Slot, u64),
 }