From 3b7bb6be2f1b88f72a8790fe347f8162098ae3c0 Mon Sep 17 00:00:00 2001 From: Nikolay Kurtov Date: Thu, 28 Apr 2022 15:21:11 +0200 Subject: [PATCH 1/5] Init txpool metric really --- chain/chunks/src/lib.rs | 1 + chain/pool/src/lib.rs | 13 +++++++------ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/chain/chunks/src/lib.rs b/chain/chunks/src/lib.rs index e33852c5eae..46d0eb8ca8d 100644 --- a/chain/chunks/src/lib.rs +++ b/chain/chunks/src/lib.rs @@ -492,6 +492,7 @@ impl ShardsManager { network_adapter: Arc, rng_seed: RngSeed, ) -> Self { + TransactionPool::init_metric(); Self { me: me.clone(), tx_pools: HashMap::new(), diff --git a/chain/pool/src/lib.rs b/chain/pool/src/lib.rs index b0d3d9dfabc..42d7614f734 100644 --- a/chain/pool/src/lib.rs +++ b/chain/pool/src/lib.rs @@ -17,9 +17,9 @@ pub struct TransactionPool { /// Transactions are grouped by a pair of (account ID, signer public key). /// NOTE: It's more efficient on average to keep transactions unsorted and with potentially /// conflicting nonce than to create a BTreeMap for every transaction. - pub transactions: BTreeMap>, + transactions: BTreeMap>, /// Set of all hashes to quickly check if the given transaction is in the pool. - pub unique_transactions: HashSet, + unique_transactions: HashSet, /// A uniquely generated key seed to randomize PoolKey order. key_seed: RngSeed, /// The key after which the pool iterator starts. Doesn't have to be present in the pool. @@ -36,6 +36,11 @@ impl TransactionPool { } } + pub fn init_metric() { + // A `get()` call initializes a metric even if its value is zero. + metrics::TRANSACTION_POOL_TOTAL.get(); + } + fn key(&self, account_id: &AccountId, public_key: &PublicKey) -> PoolKey { let mut v = public_key.try_to_vec().unwrap(); v.extend_from_slice(&self.key_seed); @@ -108,10 +113,6 @@ impl TransactionPool { pub fn len(&self) -> usize { self.unique_transactions.len() } - - pub fn is_empty(&self) -> bool { - self.unique_transactions.is_empty() - } } /// PoolIterator is a structure to pull transactions from the pool. From 394de53c5c8bf388c7669cf68e3adee5de7146bd Mon Sep 17 00:00:00 2001 From: Nikolay Kurtov Date: Thu, 28 Apr 2022 17:39:06 +0200 Subject: [PATCH 2/5] Rename --- chain/chunks/src/lib.rs | 2 +- chain/pool/src/lib.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/chain/chunks/src/lib.rs b/chain/chunks/src/lib.rs index 46d0eb8ca8d..569999cdc8a 100644 --- a/chain/chunks/src/lib.rs +++ b/chain/chunks/src/lib.rs @@ -492,7 +492,7 @@ impl ShardsManager { network_adapter: Arc, rng_seed: RngSeed, ) -> Self { - TransactionPool::init_metric(); + TransactionPool::init_metrics(); Self { me: me.clone(), tx_pools: HashMap::new(), diff --git a/chain/pool/src/lib.rs b/chain/pool/src/lib.rs index 42d7614f734..57565ffcd4c 100644 --- a/chain/pool/src/lib.rs +++ b/chain/pool/src/lib.rs @@ -36,7 +36,7 @@ impl TransactionPool { } } - pub fn init_metric() { + pub fn init_metrics() { // A `get()` call initializes a metric even if its value is zero. metrics::TRANSACTION_POOL_TOTAL.get(); } From 877c63d415464b0ee4011c3ce3e6aaa919da0eba Mon Sep 17 00:00:00 2001 From: Nikolay Kurtov Date: Fri, 29 Apr 2022 11:22:08 +0200 Subject: [PATCH 3/5] Actually avoid inserting transactions in non-validator nodes. --- chain/client/src/client.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/chain/client/src/client.rs b/chain/client/src/client.rs index 71acca4efaa..0777a1d0edf 100644 --- a/chain/client/src/client.rs +++ b/chain/client/src/client.rs @@ -1673,7 +1673,6 @@ impl Client { shard_id, is_forwarded ); - self.shards_mgr.insert_transaction(shard_id, tx.clone()); // Active validator: // possibly forward to next epoch validators @@ -1681,6 +1680,10 @@ impl Client { // forward to current epoch validators, // possibly forward to next epoch validators if active_validator { + // Only add a transaction to a transaction pool if a node is a validator. + // Transactions may not be removed from a transaction pool otherwise. + self.shards_mgr.insert_transaction(shard_id, tx.clone()); + if !is_forwarded { self.possibly_forward_tx_to_next_epoch(tx)?; } From 051d94e5eeb145a41e2924eb2d0550c6e128bbd3 Mon Sep 17 00:00:00 2001 From: Nikolay Kurtov Date: Fri, 29 Apr 2022 11:25:21 +0200 Subject: [PATCH 4/5] Clarify comments by removing unclear comments. --- chain/client/src/client.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/chain/client/src/client.rs b/chain/client/src/client.rs index 0777a1d0edf..0111cc8af91 100644 --- a/chain/client/src/client.rs +++ b/chain/client/src/client.rs @@ -1680,8 +1680,6 @@ impl Client { // forward to current epoch validators, // possibly forward to next epoch validators if active_validator { - // Only add a transaction to a transaction pool if a node is a validator. - // Transactions may not be removed from a transaction pool otherwise. self.shards_mgr.insert_transaction(shard_id, tx.clone()); if !is_forwarded { From 225d7fd212a0565c145782d4f9ade9071fe5177b Mon Sep 17 00:00:00 2001 From: Nikolay Kurtov Date: Mon, 2 May 2022 17:09:33 +0200 Subject: [PATCH 5/5] Logging and metrics when receiving a transaction. --- chain/client/src/client.rs | 13 ++++++------- chain/client/src/metrics.rs | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/chain/client/src/client.rs b/chain/client/src/client.rs index 0111cc8af91..5d408708103 100644 --- a/chain/client/src/client.rs +++ b/chain/client/src/client.rs @@ -1666,13 +1666,6 @@ impl Client { let active_validator = self.active_validator(shard_id)?; // If I'm not an active validator I should forward tx to next validators. - debug!( - target: "client", - "Recording a transaction. I'm {:?}, {} is_forwarded: {}", - me, - shard_id, - is_forwarded - ); // Active validator: // possibly forward to next epoch validators @@ -1680,6 +1673,8 @@ impl Client { // forward to current epoch validators, // possibly forward to next epoch validators if active_validator { + debug!(target: "client", account=?me, shard_id, is_forwarded, "Recording a transaction."); + metrics::TRANSACTION_RECEIVED_VALIDATOR.inc(); self.shards_mgr.insert_transaction(shard_id, tx.clone()); if !is_forwarded { @@ -1687,9 +1682,13 @@ impl Client { } Ok(NetworkClientResponses::ValidTx) } else if !is_forwarded { + debug!(target: "client", shard_id, "Forwarding a transaction."); + metrics::TRANSACTION_RECEIVED_NON_VALIDATOR.inc(); self.forward_tx(&epoch_id, tx)?; Ok(NetworkClientResponses::RequestRouted) } else { + debug!(target: "client", shard_id, "Non-validator received a forwarded transaction, dropping it."); + metrics::TRANSACTION_RECEIVED_NON_VALIDATOR_FORWARDED.inc(); Ok(NetworkClientResponses::NoResponse) } } diff --git a/chain/client/src/metrics.rs b/chain/client/src/metrics.rs index b6323b8fad0..2b4bb0ef817 100644 --- a/chain/client/src/metrics.rs +++ b/chain/client/src/metrics.rs @@ -185,6 +185,25 @@ static NODE_BUILD_INFO: Lazy = Lazy::new(|| { .unwrap() }); +pub(crate) static TRANSACTION_RECEIVED_VALIDATOR: Lazy = Lazy::new(|| { + try_create_int_gauge("near_transaction_received_validator", "Validator received a transaction") + .unwrap() +}); +pub(crate) static TRANSACTION_RECEIVED_NON_VALIDATOR: Lazy = Lazy::new(|| { + try_create_int_gauge( + "near_transaction_received_non_validator", + "Non-validator received a transaction", + ) + .unwrap() +}); +pub(crate) static TRANSACTION_RECEIVED_NON_VALIDATOR_FORWARDED: Lazy = Lazy::new(|| { + try_create_int_gauge( + "near_transaction_received_non_validator_forwarded", + "Non-validator received a forwarded transaction", + ) + .unwrap() +}); + /// Exports neard, protocol and database versions via Prometheus metrics. /// /// Sets metrics which export node’s max supported protocol version, used