From 0746b1b650a3c12e774290ce6fb2ec88a6eb6286 Mon Sep 17 00:00:00 2001
From: Balaji Arun <balaji@aptoslabs.com>
Date: Tue, 12 Nov 2024 16:32:49 -0800
Subject: [PATCH] [optqs] proposal status tracker metrics/logs (#15257)

---
 config/src/config/quorum_store_config.rs      |  2 ++
 consensus/src/counters.rs                     | 16 ++++++++++++
 consensus/src/epoch_manager.rs                |  1 +
 .../src/liveness/proposal_status_tracker.rs   | 26 +++++++++++++++++--
 4 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/config/src/config/quorum_store_config.rs b/config/src/config/quorum_store_config.rs
index c17ab0690ea41..f245e02f40b7d 100644
--- a/config/src/config/quorum_store_config.rs
+++ b/config/src/config/quorum_store_config.rs
@@ -97,6 +97,7 @@ pub struct QuorumStoreConfig {
     pub batch_buckets: Vec<u64>,
     pub allow_batches_without_pos_in_proposal: bool,
     pub enable_opt_quorum_store: bool,
+    pub opt_qs_minimum_batch_age_usecs: u64,
 }
 
 impl Default for QuorumStoreConfig {
@@ -136,6 +137,7 @@ impl Default for QuorumStoreConfig {
             batch_buckets: DEFAULT_BUCKETS.to_vec(),
             allow_batches_without_pos_in_proposal: true,
             enable_opt_quorum_store: false,
+            opt_qs_minimum_batch_age_usecs: Duration::from_millis(20).as_micros() as u64,
         }
     }
 }
diff --git a/consensus/src/counters.rs b/consensus/src/counters.rs
index f2dcb3797b794..f296aeac31517 100644
--- a/consensus/src/counters.rs
+++ b/consensus/src/counters.rs
@@ -1350,3 +1350,19 @@ pub static CONSENSUS_PROPOSAL_PAYLOAD_BATCH_AVAILABILITY_IN_QS: Lazy<IntCounterV
         .unwrap()
     },
 );
+
+pub static OPTQS_EXCLUDE_AUTHORS_COUNT: Lazy<IntCounterVec> = Lazy::new(|| {
+    register_int_counter_vec!(
+        "aptos_optqs_exclude_authors",
+        "The number of times a batch author appears on the exclude list",
+        &["author"]
+    )
+    .unwrap()
+});
+
+pub static OPTQS_LAST_CONSECUTIVE_SUCCESS_COUNT: Lazy<Histogram> = Lazy::new(|| {
+    register_avg_counter(
+        "aptos_optqs_last_consecutive_successes",
+        "The number of last consecutive successes capped at window length",
+    )
+});
diff --git a/consensus/src/epoch_manager.rs b/consensus/src/epoch_manager.rs
index d46d1bf2ca6ab..444ec3bd2f4ab 100644
--- a/consensus/src/epoch_manager.rs
+++ b/consensus/src/epoch_manager.rs
@@ -845,6 +845,7 @@ impl<P: OnChainConfigProvider> EpochManager<P> {
         )));
         let opt_qs_payload_param_provider = Arc::new(OptQSPullParamsProvider::new(
             self.config.quorum_store.enable_opt_quorum_store,
+            self.config.quorum_store.opt_qs_minimum_batch_age_usecs,
             failures_tracker.clone(),
         ));
 
diff --git a/consensus/src/liveness/proposal_status_tracker.rs b/consensus/src/liveness/proposal_status_tracker.rs
index 146cd89a5e9da..bea6c6c34a5f4 100644
--- a/consensus/src/liveness/proposal_status_tracker.rs
+++ b/consensus/src/liveness/proposal_status_tracker.rs
@@ -2,12 +2,15 @@
 // SPDX-License-Identifier: Apache-2.0
 
 use super::round_state::NewRoundReason;
+use crate::counters;
 use aptos_collections::BoundedVecDeque;
 use aptos_consensus_types::{
     common::Author, payload_pull_params::OptQSPayloadPullParams, round_timeout::RoundTimeoutReason,
 };
 use aptos_infallible::Mutex;
-use std::{collections::HashSet, sync::Arc, time::Duration};
+use aptos_logger::warn;
+use aptos_short_hex_str::AsShortHexStr;
+use std::{collections::HashSet, sync::Arc};
 
 pub trait TPastProposalStatusTracker: Send + Sync {
     fn push(&self, status: NewRoundReason);
@@ -103,16 +106,19 @@ impl TPastProposalStatusTracker for Mutex<ExponentialWindowFailureTracker> {
 
 pub struct OptQSPullParamsProvider {
     enable_opt_qs: bool,
+    minimum_batch_age_usecs: u64,
     failure_tracker: Arc<Mutex<ExponentialWindowFailureTracker>>,
 }
 
 impl OptQSPullParamsProvider {
     pub fn new(
         enable_opt_qs: bool,
+        minimum_batch_age_usecs: u64,
         failure_tracker: Arc<Mutex<ExponentialWindowFailureTracker>>,
     ) -> Self {
         Self {
             enable_opt_qs,
+            minimum_batch_age_usecs,
             failure_tracker,
         }
     }
@@ -126,14 +132,30 @@ impl TOptQSPullParamsProvider for OptQSPullParamsProvider {
 
         let tracker = self.failure_tracker.lock();
 
+        counters::OPTQS_LAST_CONSECUTIVE_SUCCESS_COUNT
+            .observe(tracker.last_consecutive_success_count as f64);
         if tracker.last_consecutive_success_count < tracker.window {
+            warn!(
+                "Skipping OptQS: (last_consecutive_successes) {} < {} (window)",
+                tracker.last_consecutive_success_count, tracker.window
+            );
             return None;
         }
 
         let exclude_authors = tracker.get_exclude_authors();
+        if !exclude_authors.is_empty() {
+            let exclude_authors_str: Vec<_> =
+                exclude_authors.iter().map(|a| a.short_str()).collect();
+            for author in &exclude_authors_str {
+                counters::OPTQS_EXCLUDE_AUTHORS_COUNT
+                    .with_label_values(&[author.as_str()])
+                    .inc();
+            }
+            warn!("OptQS exclude authors: {:?}", exclude_authors_str);
+        }
         Some(OptQSPayloadPullParams {
             exclude_authors,
-            minimum_batch_age_usecs: Duration::from_millis(10).as_micros() as u64,
+            minimum_batch_age_usecs: self.minimum_batch_age_usecs,
         })
     }
 }