add tx count and thread id to stats (solana-labs#20451)

each stat reports and resets when slot changes
tao-stones · Oct 5, 2021 · dff729f · dff729f
1 parent f1c48a2
commit dff729f
Show file tree

Hide file tree

Showing 6 changed files with 102 additions and 83 deletions.
diff --git a/core/benches/banking_stage.rs b/core/benches/banking_stage.rs
@@ -9,7 +9,8 @@ use rand::{thread_rng, Rng};
 use rayon::prelude::*;
 use solana_core::banking_stage::{BankingStage, BankingStageStats};
 use solana_core::cost_model::CostModel;
-use solana_core::cost_tracker::{CostTracker, CostTrackerStats};
+use solana_core::cost_tracker::CostTracker;
+use solana_core::cost_tracker_stats::CostTrackerStats;
 use solana_gossip::cluster_info::ClusterInfo;
 use solana_gossip::cluster_info::Node;
 use solana_ledger::blockstore_processor::process_entries;

diff --git a/core/src/banking_stage.rs b/core/src/banking_stage.rs
@@ -2,7 +2,7 @@
 //! to contruct a software pipeline. The stage uses all available CPU cores and
 //! can do its processing in parallel with signature verification on the GPU.
 use crate::{
-    cost_tracker::CostTracker, cost_tracker::CostTrackerStats, packet_hasher::PacketHasher,
+    cost_tracker::CostTracker, cost_tracker_stats::CostTrackerStats, packet_hasher::PacketHasher,
 };
 use crossbeam_channel::{Receiver as CrossbeamReceiver, RecvTimeoutError};
 use itertools::Itertools;
@@ -433,13 +433,16 @@ impl BankingStage {
             return;
         }
 
-        cost_tracker
+        if cost_tracker
             .write()
             .unwrap()
-            .reset_if_new_bank(bank.slot(), cost_tracker_stats);
-        banking_stage_stats
-            .reset_cost_tracker_count
-            .fetch_add(1, Ordering::Relaxed);
+            .reset_if_new_bank(bank.slot(), cost_tracker_stats)
+        {
+            // only increase counter when bank changed
+            banking_stage_stats
+                .reset_cost_tracker_count
+                .fetch_add(1, Ordering::Relaxed);
+        }
     }
 
     #[allow(clippy::too_many_arguments)]
@@ -741,7 +744,7 @@ impl BankingStage {
         let socket = UdpSocket::bind("0.0.0.0:0").unwrap();
         let mut buffered_packets = VecDeque::with_capacity(batch_limit);
         let banking_stage_stats = BankingStageStats::new(id);
-        let mut cost_tracker_stats = CostTrackerStats::default();
+        let mut cost_tracker_stats = CostTrackerStats::new(id, 0);
         loop {
             while !buffered_packets.is_empty() {
                 let decision = Self::process_buffered_packets(

diff --git a/core/src/cost_tracker.rs b/core/src/cost_tracker.rs
@@ -5,6 +5,7 @@
 //! - add_transaction_cost(&tx), mutable function to accumulate `tx` cost to tracker.
 //!
 use crate::cost_model::{CostModel, TransactionCost};
+use crate::cost_tracker_stats::CostTrackerStats;
 use solana_sdk::{clock::Slot, pubkey::Pubkey, transaction::Transaction};
 use std::{
     collections::HashMap,
@@ -13,76 +14,6 @@ use std::{
 
 const WRITABLE_ACCOUNTS_PER_BLOCK: usize = 512;
 
-// cist tracker stats reset for each bank
-#[derive(Debug, Default)]
-pub struct CostTrackerStats {
-    transaction_cost_histogram: histogram::Histogram,
-    writable_accounts_cost_histogram: histogram::Histogram,
-    block_cost: u64,
-    bank_slot: u64,
-}
-
-impl CostTrackerStats {
-    pub fn new(bank_slot: Slot) -> Self {
-        CostTrackerStats {
-            bank_slot,
-            ..CostTrackerStats::default()
-        }
-    }
-
-    fn report(&self) {
-        datapoint_info!(
-            "cost_tracker_stats",
-            (
-                "transaction_cost_unit_min",
-                self.transaction_cost_histogram.minimum().unwrap_or(0),
-                i64
-            ),
-            (
-                "transaction_cost_unit_max",
-                self.transaction_cost_histogram.maximum().unwrap_or(0),
-                i64
-            ),
-            (
-                "transaction_cost_unit_mean",
-                self.transaction_cost_histogram.mean().unwrap_or(0),
-                i64
-            ),
-            (
-                "transaction_cost_unit_2nd_std",
-                self.transaction_cost_histogram
-                    .percentile(95.0)
-                    .unwrap_or(0),
-                i64
-            ),
-            (
-                "writable_accounts_cost_min",
-                self.writable_accounts_cost_histogram.minimum().unwrap_or(0),
-                i64
-            ),
-            (
-                "writable_accounts_cost_max",
-                self.writable_accounts_cost_histogram.maximum().unwrap_or(0),
-                i64
-            ),
-            (
-                "writable_accounts_cost_mean",
-                self.writable_accounts_cost_histogram.mean().unwrap_or(0),
-                i64
-            ),
-            (
-                "writable_accounts_cost_2nd_std",
-                self.writable_accounts_cost_histogram
-                    .percentile(95.0)
-                    .unwrap_or(0),
-                i64
-            ),
-            ("block_cost", self.block_cost, i64),
-            ("bank_slot", self.bank_slot, i64),
-        );
-    }
-}
-
 #[derive(Debug)]
 pub struct CostTracker {
     cost_model: Arc<RwLock<CostModel>>,
@@ -141,18 +72,25 @@ impl CostTracker {
         }
         self.block_cost += cost;
 
+        stats.transaction_count += 1;
         stats.block_cost += cost;
     }
 
-    pub fn reset_if_new_bank(&mut self, slot: Slot, stats: &mut CostTrackerStats) {
-        if slot != self.current_bank_slot {
-            stats.bank_slot = self.current_bank_slot;
+    pub fn reset_if_new_bank(&mut self, slot: Slot, stats: &mut CostTrackerStats) -> bool {
+        // report stats when slot changes
+        if slot != stats.bank_slot {
             stats.report();
-            *stats = CostTrackerStats::default();
+            *stats = CostTrackerStats::new(stats.id, slot);
+        }
 
+        if slot != self.current_bank_slot {
             self.current_bank_slot = slot;
             self.cost_by_writable_accounts.clear();
             self.block_cost = 0;
+
+            true
+        } else {
+            false
         }
     }
 

diff --git a/core/src/cost_tracker_stats.rs b/core/src/cost_tracker_stats.rs
@@ -0,0 +1,75 @@
+//! The Stats is not thread safe, each thread should have its own
+//! instance of stat with `id`; Stat reports and reset for each slot.
+#[derive(Debug, Default)]
+pub struct CostTrackerStats {
+    pub id: u32,
+    pub transaction_cost_histogram: histogram::Histogram,
+    pub writable_accounts_cost_histogram: histogram::Histogram,
+    pub transaction_count: u64,
+    pub block_cost: u64,
+    pub bank_slot: u64,
+}
+
+impl CostTrackerStats {
+    pub fn new(id: u32, bank_slot: u64) -> Self {
+        CostTrackerStats {
+            id,
+            bank_slot,
+            ..CostTrackerStats::default()
+        }
+    }
+
+    pub fn report(&self) {
+        datapoint_info!(
+            "cost_tracker_stats",
+            ("id", self.id as i64, i64),
+            (
+                "transaction_cost_unit_min",
+                self.transaction_cost_histogram.minimum().unwrap_or(0),
+                i64
+            ),
+            (
+                "transaction_cost_unit_max",
+                self.transaction_cost_histogram.maximum().unwrap_or(0),
+                i64
+            ),
+            (
+                "transaction_cost_unit_mean",
+                self.transaction_cost_histogram.mean().unwrap_or(0),
+                i64
+            ),
+            (
+                "transaction_cost_unit_2nd_std",
+                self.transaction_cost_histogram
+                    .percentile(95.0)
+                    .unwrap_or(0),
+                i64
+            ),
+            (
+                "writable_accounts_cost_min",
+                self.writable_accounts_cost_histogram.minimum().unwrap_or(0),
+                i64
+            ),
+            (
+                "writable_accounts_cost_max",
+                self.writable_accounts_cost_histogram.maximum().unwrap_or(0),
+                i64
+            ),
+            (
+                "writable_accounts_cost_mean",
+                self.writable_accounts_cost_histogram.mean().unwrap_or(0),
+                i64
+            ),
+            (
+                "writable_accounts_cost_2nd_std",
+                self.writable_accounts_cost_histogram
+                    .percentile(95.0)
+                    .unwrap_or(0),
+                i64
+            ),
+            ("transaction_count", self.transaction_count as i64, i64),
+            ("block_cost", self.block_cost as i64, i64),
+            ("bank_slot", self.bank_slot as i64, i64),
+        );
+    }
+}
diff --git a/core/src/lib.rs b/core/src/lib.rs
@@ -21,6 +21,7 @@ pub mod completed_data_sets_service;
 pub mod consensus;
 pub mod cost_model;
 pub mod cost_tracker;
+pub mod cost_tracker_stats;
 pub mod cost_update_service;
 pub mod execute_cost_table;
 pub mod fetch_stage;

diff --git a/ledger-tool/src/main.rs b/ledger-tool/src/main.rs
@@ -15,7 +15,8 @@ use solana_clap_utils::{
     },
 };
 use solana_core::cost_model::CostModel;
-use solana_core::cost_tracker::{CostTracker, CostTrackerStats};
+use solana_core::cost_tracker::CostTracker;
+use solana_core::cost_tracker_stats::CostTrackerStats;
 use solana_ledger::entry::Entry;
 use solana_ledger::{
     ancestor_iterator::AncestorIterator,