fix(tree): update metrics only on canonical/side chain changes (#3671)

paradigmxyz · Jul 9, 2023 · b68116c · b68116c
1 parent f116040
commit b68116c
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 10 deletions.
diff --git a/crates/blockchain-tree/src/blockchain_tree.rs b/crates/blockchain-tree/src/blockchain_tree.rs
@@ -1084,8 +1084,12 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
         }
     }
 
-    /// Update blockchain tree and sync metrics
-    pub(crate) fn update_metrics(&mut self) {
+    /// Update blockchain tree chains (canonical and sidechains) and sync metrics.
+    ///
+    /// NOTE: this method should not be called during the pipeline sync, because otherwise the sync
+    /// checkpoint metric will get overwritten. Buffered blocks metrics are updated in
+    /// [BlockBuffer] during the pipeline sync.
+    pub(crate) fn update_chains_metrics(&mut self) {
         let height = self.canonical_chain().tip().number;
 
         self.metrics.sidechains.set(self.chains.len() as f64);

diff --git a/crates/blockchain-tree/src/shareable.rs b/crates/blockchain-tree/src/shareable.rs
@@ -43,9 +43,9 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTreeEngine
 {
     fn buffer_block(&self, block: SealedBlockWithSenders) -> Result<(), InsertBlockError> {
         let mut tree = self.tree.write();
-        let res = tree.buffer_block(block);
-        tree.update_metrics();
-        res
+        // Blockchain tree metrics shouldn't be updated here, see
+        // `BlockchainTree::update_chains_metrics` documentation.
+        tree.buffer_block(block)
     }
 
     fn insert_block(
@@ -55,38 +55,38 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTreeEngine
         trace!(target: "blockchain_tree", hash=?block.hash, number=block.number, parent_hash=?block.parent_hash, "Inserting block");
         let mut tree = self.tree.write();
         let res = tree.insert_block(block);
-        tree.update_metrics();
+        tree.update_chains_metrics();
         res
     }
 
     fn finalize_block(&self, finalized_block: BlockNumber) {
         trace!(target: "blockchain_tree", ?finalized_block, "Finalizing block");
         let mut tree = self.tree.write();
         tree.finalize_block(finalized_block);
-        tree.update_metrics();
+        tree.update_chains_metrics();
     }
 
     fn restore_canonical_hashes(&self, last_finalized_block: BlockNumber) -> Result<(), Error> {
         trace!(target: "blockchain_tree", ?last_finalized_block, "Restoring canonical hashes for last finalized block");
         let mut tree = self.tree.write();
         let res = tree.restore_canonical_hashes(last_finalized_block);
-        tree.update_metrics();
+        tree.update_chains_metrics();
         res
     }
 
     fn make_canonical(&self, block_hash: &BlockHash) -> Result<CanonicalOutcome, Error> {
         trace!(target: "blockchain_tree", ?block_hash, "Making block canonical");
         let mut tree = self.tree.write();
         let res = tree.make_canonical(block_hash);
-        tree.update_metrics();
+        tree.update_chains_metrics();
         res
     }
 
     fn unwind(&self, unwind_to: BlockNumber) -> Result<(), Error> {
         trace!(target: "blockchain_tree", ?unwind_to, "Unwinding to block number");
         let mut tree = self.tree.write();
         let res = tree.unwind(unwind_to);
-        tree.update_metrics();
+        tree.update_chains_metrics();
         res
     }
 }