[mempool] cherry-pick reduce default failover: 3 -> 1 and related PRs (…

…aptos-labs#8027)
sherry-x · May 3, 2023 · 5d087fe · 5d087fe
1 parent a343ab2
commit 5d087fe
Show file tree

Hide file tree

Showing 8 changed files with 26 additions and 9 deletions.
diff --git a/config/src/config/mempool_config.rs b/config/src/config/mempool_config.rs
@@ -49,7 +49,7 @@ impl Default for MempoolConfig {
             capacity: 2_000_000,
             capacity_bytes: 2 * 1024 * 1024 * 1024,
             capacity_per_user: 100,
-            default_failovers: 3,
+            default_failovers: 1,
             system_transaction_timeout_secs: 600,
             system_transaction_gc_interval_ms: 60_000,
             broadcast_buckets: DEFAULT_BUCKETS.to_vec(),

diff --git a/consensus/src/block_storage/block_store.rs b/consensus/src/block_storage/block_store.rs
@@ -64,8 +64,10 @@ pub fn update_counters_for_committed_blocks(blocks_to_commit: &[Arc<ExecutedBloc
             let commit_status = match status {
                 TransactionStatus::Keep(_) => counters::TXN_COMMIT_SUCCESS_LABEL,
                 TransactionStatus::Discard(reason) => {
-                    if reason == &DiscardedVMStatus::SEQUENCE_NUMBER_TOO_NEW {
+                    if *reason == DiscardedVMStatus::SEQUENCE_NUMBER_TOO_NEW {
                         counters::TXN_COMMIT_RETRY_LABEL
+                    } else if *reason == DiscardedVMStatus::SEQUENCE_NUMBER_TOO_OLD {
+                        counters::TXN_COMMIT_FAILED_DUPLICATE_LABEL
                     } else {
                         counters::TXN_COMMIT_FAILED_LABEL
                     }

diff --git a/consensus/src/counters.rs b/consensus/src/counters.rs
@@ -15,6 +15,8 @@ use once_cell::sync::Lazy;
 pub const TXN_COMMIT_SUCCESS_LABEL: &str = "success";
 /// Transaction commit failed (will not be retried)
 pub const TXN_COMMIT_FAILED_LABEL: &str = "failed";
+/// Transaction commit failed (will not be retried) because of a duplicate
+pub const TXN_COMMIT_FAILED_DUPLICATE_LABEL: &str = "failed_duplicate";
 /// Transaction commit was unsuccessful, but will be retried
 pub const TXN_COMMIT_RETRY_LABEL: &str = "retry";
 

diff --git a/docker/compose/aptos-node/fullnode.yaml b/docker/compose/aptos-node/fullnode.yaml
@@ -28,7 +28,7 @@ full_node_networks:
 mempool:
   shared_mempool_max_concurrent_inbound_syncs: 16 # default 4
   max_broadcasts_per_peer: 4 # default 1
-  default_failovers: 0 # default 3
+  default_failovers: 0 # default 1
   shared_mempool_batch_size: 200 # default 100
   shared_mempool_tick_interval_ms: 10 # default 50
 

diff --git a/execution/executor/src/components/chunk_output.rs b/execution/executor/src/components/chunk_output.rs
@@ -18,6 +18,7 @@ use aptos_types::{
 };
 use aptos_vm::{AptosVM, VMExecutor};
 use fail::fail_point;
+use move_core_types::vm_status::StatusCode;
 use std::time::Duration;
 
 pub struct ChunkOutput {
@@ -193,7 +194,12 @@ pub fn update_counters_for_processed_chunk(
                     )
                 );
                 (
-                    "discard",
+                    // Specialize duplicate txns for alerts
+                    if *discard_status_code == StatusCode::SEQUENCE_NUMBER_TOO_OLD {
+                        "discard_sequence_number_too_old"
+                    } else {
+                        "discard"
+                    },
                     "error_code",
                     if detailed_counters {
                         format!("{:?}", discard_status_code).to_lowercase()

diff --git a/mempool/src/core_mempool/mempool.rs b/mempool/src/core_mempool/mempool.rs
@@ -98,11 +98,17 @@ impl Mempool {
         if *reason == DiscardedVMStatus::SEQUENCE_NUMBER_TOO_NEW {
             self.log_reject_transaction(sender, sequence_number, counters::COMMIT_IGNORED_LABEL);
             // Do not remove the transaction from mempool
-        } else {
-            self.log_reject_transaction(sender, sequence_number, counters::COMMIT_REJECTED_LABEL);
-            self.transactions
-                .reject_transaction(sender, sequence_number, hash);
+            return;
         }
+
+        let label = if *reason == DiscardedVMStatus::SEQUENCE_NUMBER_TOO_OLD {
+            counters::COMMIT_REJECTED_DUPLICATE_LABEL
+        } else {
+            counters::COMMIT_REJECTED_LABEL
+        };
+        self.log_reject_transaction(sender, sequence_number, label);
+        self.transactions
+            .reject_transaction(sender, sequence_number, hash);
     }
 
     fn log_latency(&self, account: AccountAddress, sequence_number: u64, stage: &'static str) {

diff --git a/mempool/src/counters.rs b/mempool/src/counters.rs
@@ -24,6 +24,7 @@ pub const SIZE_BYTES_LABEL: &str = "size_bytes";
 // Core mempool stages labels
 pub const COMMIT_ACCEPTED_LABEL: &str = "commit_accepted";
 pub const COMMIT_REJECTED_LABEL: &str = "commit_rejected";
+pub const COMMIT_REJECTED_DUPLICATE_LABEL: &str = "commit_rejected_duplicate";
 pub const COMMIT_IGNORED_LABEL: &str = "commit_ignored";
 pub const CONSENSUS_READY_LABEL: &str = "consensus_ready";
 pub const CONSENSUS_PULLED_LABEL: &str = "consensus_pulled";

diff --git a/terraform/helm/aptos-node/values.yaml b/terraform/helm/aptos-node/values.yaml
@@ -131,7 +131,7 @@ fullnode:
     mempool:
       shared_mempool_max_concurrent_inbound_syncs: 16 # default 4
       max_broadcasts_per_peer: 4 # default 1
-      default_failovers: 0 # default 3
+      default_failovers: 0 # default 1
       shared_mempool_batch_size: 200 # default 100
       shared_mempool_tick_interval_ms: 10 # default 50