From e02542003d2c7290704ce155b0dee1d176a6ab27 Mon Sep 17 00:00:00 2001 From: Tao Zhu Date: Fri, 8 Apr 2022 16:20:12 -0500 Subject: [PATCH] fix merge conflicts --- core/src/banking_stage.rs | 3 +- core/src/qos_service.rs | 15 -- runtime/src/cost_model.rs | 12 -- runtime/src/cost_tracker.rs | 316 +----------------------------------- 4 files changed, 3 insertions(+), 343 deletions(-) diff --git a/core/src/banking_stage.rs b/core/src/banking_stage.rs index e28a38c145..adba861baa 100644 --- a/core/src/banking_stage.rs +++ b/core/src/banking_stage.rs @@ -2860,7 +2860,8 @@ mod tests { poh_recorder.lock().unwrap().set_bank(&bank); let (gossip_vote_sender, _gossip_vote_receiver) = unbounded(); - let qos_service = QosService::new(Arc::new(RwLock::new(CostModel::default())), 1); + let qos_service = + Arc::new(QosService::new(Arc::new(RwLock::new(CostModel::default())))); let get_block_cost = || bank.read_cost_tracker().unwrap().block_cost(); let get_tx_count = || bank.read_cost_tracker().unwrap().transaction_count(); diff --git a/core/src/qos_service.rs b/core/src/qos_service.rs index 72f1cda8a0..45a66ab52b 100644 --- a/core/src/qos_service.rs +++ b/core/src/qos_service.rs @@ -151,12 +151,6 @@ impl QosService { (select_results, num_included) } -<<<<<<< HEAD -<<<<<<< HEAD -======= - pub fn commit_transaction_cost( - &self, -======= /// Update the transaction cost in the cost_tracker with the real cost for /// transactions that were executed successfully; /// Otherwise remove the cost from the cost tracker, therefore preventing cost_tracker @@ -165,7 +159,6 @@ impl QosService { transaction_costs: impl Iterator, transaction_qos_results: impl Iterator>, retryable_transaction_indexes: &[usize], ->>>>>>> 924b8ea1e (Adjustments to cost_tracker updates) bank: &Arc, ) { let mut cost_tracker = bank.write_cost_tracker().unwrap(); @@ -187,14 +180,6 @@ impl QosService { }); } - // metrics are reported by bank slot - pub fn report_metrics(&self, bank: Arc) { - self.report_sender - .send(QosMetrics::BlockBatchUpdate { bank }) - .unwrap_or_else(|err| warn!("qos service report metrics failed: {:?}", err)); - } - ->>>>>>> 9e07272af (- Only commit successfully executed transactions' cost to cost_tracker;) pub fn accumulate_estimated_transaction_costs( &self, cost_details: &BatchedTransactionCostDetails, diff --git a/runtime/src/cost_model.rs b/runtime/src/cost_model.rs index f27f714483..1c9219dba3 100644 --- a/runtime/src/cost_model.rs +++ b/runtime/src/cost_model.rs @@ -21,11 +21,7 @@ pub struct TransactionCost { pub write_lock_cost: u64, pub data_bytes_cost: u64, pub execution_cost: u64, -<<<<<<< HEAD -======= - pub account_data_size: u64, pub is_simple_vote: bool, ->>>>>>> 9e07272af (- Only commit successfully executed transactions' cost to cost_tracker;) } impl Default for TransactionCost { @@ -36,11 +32,7 @@ impl Default for TransactionCost { write_lock_cost: 0u64, data_bytes_cost: 0u64, execution_cost: 0u64, -<<<<<<< HEAD -======= - account_data_size: 0u64, is_simple_vote: false, ->>>>>>> 9e07272af (- Only commit successfully executed transactions' cost to cost_tracker;) } } } @@ -116,11 +108,7 @@ impl CostModel { self.get_write_lock_cost(&mut tx_cost, transaction); tx_cost.data_bytes_cost = self.get_data_bytes_cost(transaction); tx_cost.execution_cost = self.get_transaction_cost(transaction); -<<<<<<< HEAD -======= - tx_cost.account_data_size = self.calculate_account_data_size(transaction); tx_cost.is_simple_vote = transaction.is_simple_vote_transaction(); ->>>>>>> 9e07272af (- Only commit successfully executed transactions' cost to cost_tracker;) debug!("transaction {:?} has cost {:?}", transaction, tx_cost); tx_cost diff --git a/runtime/src/cost_tracker.rs b/runtime/src/cost_tracker.rs index 12bce0b0ca..b621c3661a 100644 --- a/runtime/src/cost_tracker.rs +++ b/runtime/src/cost_tracker.rs @@ -32,30 +32,10 @@ pub struct CostTracker { block_cost: u64, vote_cost: u64, transaction_count: u64, -<<<<<<< HEAD -======= - account_data_size: u64, - - /// The amount of total account data size remaining. If `Some`, then do not add transactions - /// that would cause `account_data_size` to exceed this limit. - account_data_size_limit: Option, -<<<<<<< HEAD - - // Transactions have passed would_fit check, is being executed. - // If the execution is successful, it's actual Units can be committed - // to cost_tracker; otherwise, it should be removed without impacting - // cost_tracker. - pending_transactions: HashMap, ->>>>>>> 9e07272af (- Only commit successfully executed transactions' cost to cost_tracker;) -======= ->>>>>>> 924b8ea1e (Adjustments to cost_tracker updates) } impl Default for CostTracker { fn default() -> Self { -<<<<<<< HEAD - CostTracker::new(MAX_WRITABLE_ACCOUNT_UNITS, MAX_BLOCK_UNITS, MAX_VOTE_UNITS) -======= // Clippy doesn't like asserts in const contexts, so need to explicitly allow them. For // more info, see this issue: https://github.com/rust-lang/rust-clippy/issues/8159 #![allow(clippy::assertions_on_constants)] @@ -70,10 +50,7 @@ impl Default for CostTracker { block_cost: 0, vote_cost: 0, transaction_count: 0, - account_data_size: 0, - account_data_size_limit: None, } ->>>>>>> 9e07272af (- Only commit successfully executed transactions' cost to cost_tracker;) } } @@ -104,47 +81,9 @@ impl CostTracker { self.vote_cost_limit = vote_cost_limit; } -<<<<<<< HEAD -<<<<<<< HEAD - pub fn would_transaction_fit( - &self, - transaction: &SanitizedTransaction, - tx_cost: &TransactionCost, - ) -> Result<(), CostTrackerError> { - self.would_fit(&tx_cost.writable_accounts, tx_cost.sum(), transaction) - } - - pub fn add_transaction_cost( - &mut self, - transaction: &SanitizedTransaction, - tx_cost: &TransactionCost, - ) { - self.add_transaction(&tx_cost.writable_accounts, tx_cost.sum(), transaction); - } - -======= ->>>>>>> 9e07272af (- Only commit successfully executed transactions' cost to cost_tracker;) - pub fn try_add( - &mut self, - _transaction: &SanitizedTransaction, - tx_cost: &TransactionCost, - ) -> Result { -<<<<<<< HEAD - let cost = tx_cost.sum(); - self.would_fit(&tx_cost.writable_accounts, cost, transaction)?; - self.add_transaction(&tx_cost.writable_accounts, cost, transaction); -======= -======= pub fn try_add(&mut self, tx_cost: &TransactionCost) -> Result { ->>>>>>> a058f348a (Address review comments) self.would_fit(tx_cost)?; -<<<<<<< HEAD - self.pending_transactions - .insert(*transaction.signature(), tx_cost.clone()); ->>>>>>> 9e07272af (- Only commit successfully executed transactions' cost to cost_tracker;) -======= self.add_transaction_cost(tx_cost); ->>>>>>> 924b8ea1e (Adjustments to cost_tracker updates) Ok(self.block_cost) } @@ -210,35 +149,6 @@ impl CostTracker { let cost = tx_cost.sum(); let vote_cost = if tx_cost.is_simple_vote { cost } else { 0 }; -<<<<<<< HEAD - for tx_cost in self.pending_transactions.values() { - writable_account.extend(&tx_cost.writable_accounts); - cost = cost.saturating_add(tx_cost.sum()); - account_data_size = account_data_size.saturating_add(tx_cost.account_data_size); - vote_cost = vote_cost.saturating_add(if tx_cost.is_simple_vote { cost } else { 0 }); - } - - self.would_aggregated_transactions_fit( - &writable_account, - cost, - account_data_size, - vote_cost, - ) - } - - fn would_aggregated_transactions_fit( - &self, - keys: &[Pubkey], - cost: u64, -<<<<<<< HEAD - transaction: &SanitizedTransaction, -======= - account_data_len: u64, - vote_cost: u64, ->>>>>>> 9e07272af (- Only commit successfully executed transactions' cost to cost_tracker;) - ) -> Result<(), CostTrackerError> { -======= ->>>>>>> 924b8ea1e (Adjustments to cost_tracker updates) // check against the total package cost if self.block_cost.saturating_add(cost) > self.block_cost_limit { return Err(CostTrackerError::WouldExceedBlockMaxLimit); @@ -254,24 +164,6 @@ impl CostTracker { return Err(CostTrackerError::WouldExceedAccountMaxLimit); } -<<<<<<< HEAD -======= - // NOTE: Check if the total accounts data size is exceeded *before* the block accounts data - // size. This way, transactions are not unnecessarily retried. - let account_data_size = self - .account_data_size - .saturating_add(tx_cost.account_data_size); - if let Some(account_data_size_limit) = self.account_data_size_limit { - if account_data_size > account_data_size_limit { - return Err(CostTrackerError::WouldExceedAccountDataTotalLimit); - } - } - - if account_data_size > MAX_ACCOUNT_DATA_BLOCK_LEN { - return Err(CostTrackerError::WouldExceedAccountDataBlockLimit); - } - ->>>>>>> 924b8ea1e (Adjustments to cost_tracker updates) // check each account against account_cost_limit, for account_key in writable_accounts.iter() { match self.cost_by_writable_accounts.get(account_key) { @@ -289,18 +181,9 @@ impl CostTracker { Ok(()) } -<<<<<<< HEAD -<<<<<<< HEAD - fn add_transaction(&mut self, keys: &[Pubkey], cost: u64, transaction: &SanitizedTransaction) { - for account_key in keys.iter() { -======= - fn add_transaction(&mut self, tx_cost: &TransactionCost) { -======= fn add_transaction_cost(&mut self, tx_cost: &TransactionCost) { ->>>>>>> 924b8ea1e (Adjustments to cost_tracker updates) let cost = tx_cost.sum(); for account_key in tx_cost.writable_accounts.iter() { ->>>>>>> 9e07272af (- Only commit successfully executed transactions' cost to cost_tracker;) let account_cost = self .cost_by_writable_accounts .entry(*account_key) @@ -311,12 +194,6 @@ impl CostTracker { if tx_cost.is_simple_vote { self.vote_cost = self.vote_cost.saturating_add(cost); } -<<<<<<< HEAD -======= - self.account_data_size = self - .account_data_size - .saturating_add(tx_cost.account_data_size); ->>>>>>> 9e07272af (- Only commit successfully executed transactions' cost to cost_tracker;) self.transaction_count = self.transaction_count.saturating_add(1); } @@ -333,9 +210,6 @@ impl CostTracker { if tx_cost.is_simple_vote { self.vote_cost = self.vote_cost.saturating_sub(cost); } - self.account_data_size = self - .account_data_size - .saturating_sub(tx_cost.account_data_size); self.transaction_count = self.transaction_count.saturating_sub(1); } } @@ -352,13 +226,7 @@ mod tests { hash::Hash, signature::{Keypair, Signer}, system_transaction, -<<<<<<< HEAD - transaction::{TransactionError, VersionedTransaction}, -======= - transaction::{ - MessageHash, SanitizedTransaction, SimpleAddressLoader, VersionedTransaction, - }, ->>>>>>> a058f348a (Address review comments) + transaction::{SanitizedTransaction, TransactionError, VersionedTransaction}, }, solana_vote_program::vote_transaction, std::{cmp, sync::Arc}, @@ -438,19 +306,9 @@ mod tests { let cost = tx_cost.sum(); // build testee to have capacity for one simple transaction -<<<<<<< HEAD let mut testee = CostTracker::new(cost, cost, cost); - assert!(testee.would_fit(&keys, cost, &tx).is_ok()); - testee.add_transaction(&keys, cost, &tx); -======= - let mut testee = CostTracker::new(cost, cost, cost, None); assert!(testee.would_fit(&tx_cost).is_ok()); -<<<<<<< HEAD - testee.add_transaction(&tx_cost); ->>>>>>> 9e07272af (- Only commit successfully executed transactions' cost to cost_tracker;) -======= testee.add_transaction_cost(&tx_cost); ->>>>>>> a058f348a (Address review comments) assert_eq!(cost, testee.block_cost); assert_eq!(0, testee.vote_cost); let (_costliest_account, costliest_account_cost) = testee.find_costliest_account(); @@ -464,19 +322,9 @@ mod tests { let cost = tx_cost.sum(); // build testee to have capacity for one simple transaction -<<<<<<< HEAD let mut testee = CostTracker::new(cost, cost, cost); - assert!(testee.would_fit(&keys, cost, &tx).is_ok()); - testee.add_transaction(&keys, cost, &tx); -======= - let mut testee = CostTracker::new(cost, cost, cost, None); assert!(testee.would_fit(&tx_cost).is_ok()); -<<<<<<< HEAD - testee.add_transaction(&tx_cost); ->>>>>>> 9e07272af (- Only commit successfully executed transactions' cost to cost_tracker;) -======= testee.add_transaction_cost(&tx_cost); ->>>>>>> a058f348a (Address review comments) assert_eq!(cost, testee.block_cost); assert_eq!(cost, testee.vote_cost); let (_ccostliest_account, costliest_account_cost) = testee.find_costliest_account(); @@ -484,24 +332,6 @@ mod tests { } #[test] -<<<<<<< HEAD -======= - fn test_cost_tracker_add_data() { - let (mint_keypair, start_hash) = test_setup(); - let (_tx, mut tx_cost) = build_simple_transaction(&mint_keypair, &start_hash); - tx_cost.account_data_size = 1; - let cost = tx_cost.sum(); - - // build testee to have capacity for one simple transaction - let mut testee = CostTracker::new(cost, cost, cost, None); - assert!(testee.would_fit(&tx_cost).is_ok()); - let old = testee.account_data_size; - testee.add_transaction_cost(&tx_cost); - assert_eq!(old + 1, testee.account_data_size); - } - - #[test] ->>>>>>> 9e07272af (- Only commit successfully executed transactions' cost to cost_tracker;) fn test_cost_tracker_ok_add_two_same_accounts() { let (mint_keypair, start_hash) = test_setup(); // build two transactions with same signed account @@ -513,25 +343,12 @@ mod tests { // build testee to have capacity for two simple transactions, with same accounts let mut testee = CostTracker::new(cost1 + cost2, cost1 + cost2, cost1 + cost2); { -<<<<<<< HEAD - assert!(testee.would_fit(&keys1, cost1, &tx1).is_ok()); - testee.add_transaction(&keys1, cost1, &tx1); - } - { - assert!(testee.would_fit(&keys2, cost2, &tx2).is_ok()); - testee.add_transaction(&keys2, cost2, &tx2); -======= assert!(testee.would_fit(&tx_cost1).is_ok()); testee.add_transaction_cost(&tx_cost1); } { assert!(testee.would_fit(&tx_cost2).is_ok()); -<<<<<<< HEAD - testee.add_transaction(&tx_cost2); ->>>>>>> 9e07272af (- Only commit successfully executed transactions' cost to cost_tracker;) -======= testee.add_transaction_cost(&tx_cost2); ->>>>>>> a058f348a (Address review comments) } assert_eq!(cost1 + cost2, testee.block_cost); assert_eq!(1, testee.cost_by_writable_accounts.len()); @@ -552,25 +369,12 @@ mod tests { // build testee to have capacity for two simple transactions, with same accounts let mut testee = CostTracker::new(cmp::max(cost1, cost2), cost1 + cost2, cost1 + cost2); { -<<<<<<< HEAD - assert!(testee.would_fit(&keys1, cost1, &tx1).is_ok()); - testee.add_transaction(&keys1, cost1, &tx1); - } - { - assert!(testee.would_fit(&keys2, cost2, &tx2).is_ok()); - testee.add_transaction(&keys2, cost2, &tx2); -======= assert!(testee.would_fit(&tx_cost1).is_ok()); testee.add_transaction_cost(&tx_cost1); } { assert!(testee.would_fit(&tx_cost2).is_ok()); -<<<<<<< HEAD - testee.add_transaction(&tx_cost2); ->>>>>>> 9e07272af (- Only commit successfully executed transactions' cost to cost_tracker;) -======= testee.add_transaction_cost(&tx_cost2); ->>>>>>> a058f348a (Address review comments) } assert_eq!(cost1 + cost2, testee.block_cost); assert_eq!(2, testee.cost_by_writable_accounts.len()); @@ -591,21 +395,12 @@ mod tests { let mut testee = CostTracker::new(cmp::min(cost1, cost2), cost1 + cost2, cost1 + cost2); // should have room for first transaction { -<<<<<<< HEAD - assert!(testee.would_fit(&keys1, cost1, &tx1).is_ok()); - testee.add_transaction(&keys1, cost1, &tx1); - } - // but no more sapce on the same chain (same signer account) - { - assert!(testee.would_fit(&keys2, cost2, &tx2).is_err()); -======= assert!(testee.would_fit(&tx_cost1).is_ok()); testee.add_transaction_cost(&tx_cost1); } // but no more sapce on the same chain (same signer account) { assert!(testee.would_fit(&tx_cost2).is_err()); ->>>>>>> 9e07272af (- Only commit successfully executed transactions' cost to cost_tracker;) } } @@ -624,21 +419,12 @@ mod tests { CostTracker::new(cmp::max(cost1, cost2), cost1 + cost2 - 1, cost1 + cost2 - 1); // should have room for first transaction { -<<<<<<< HEAD - assert!(testee.would_fit(&keys1, cost1, &tx1).is_ok()); - testee.add_transaction(&keys1, cost1, &tx1); - } - // but no more room for package as whole - { - assert!(testee.would_fit(&keys2, cost2, &tx2).is_err()); -======= assert!(testee.would_fit(&tx_cost1).is_ok()); testee.add_transaction_cost(&tx_cost1); } // but no more room for package as whole { assert!(testee.would_fit(&tx_cost2).is_err()); ->>>>>>> 9e07272af (- Only commit successfully executed transactions' cost to cost_tracker;) } } @@ -656,122 +442,22 @@ mod tests { let mut testee = CostTracker::new(cmp::max(cost1, cost2), cost1 + cost2, cost1 + cost2 - 1); // should have room for first vote { -<<<<<<< HEAD - assert!(testee.would_fit(&keys1, cost1, &tx1).is_ok()); - testee.add_transaction(&keys1, cost1, &tx1); - } - // but no more room for package as whole - { - assert!(testee.would_fit(&keys2, cost2, &tx2).is_err()); -======= assert!(testee.would_fit(&tx_cost1).is_ok()); testee.add_transaction_cost(&tx_cost1); } // but no more room for package as whole { assert!(testee.would_fit(&tx_cost2).is_err()); ->>>>>>> 9e07272af (- Only commit successfully executed transactions' cost to cost_tracker;) } // however there is room for none-vote tx3 { let third_account = Keypair::new(); -<<<<<<< HEAD - let (tx3, keys3, cost3) = build_simple_transaction(&third_account, &start_hash); - assert!(testee.would_fit(&keys3, cost3, &tx3).is_ok()); -======= let (_tx3, tx_cost3) = build_simple_transaction(&third_account, &start_hash); assert!(testee.would_fit(&tx_cost3).is_ok()); ->>>>>>> 9e07272af (- Only commit successfully executed transactions' cost to cost_tracker;) } } #[test] -<<<<<<< HEAD -======= - fn test_cost_tracker_reach_data_block_limit() { - let (mint_keypair, start_hash) = test_setup(); - // build two transactions with diff accounts - let second_account = Keypair::new(); - let (_tx1, mut tx_cost1) = build_simple_transaction(&mint_keypair, &start_hash); - let (_tx2, mut tx_cost2) = build_simple_transaction(&second_account, &start_hash); - tx_cost1.account_data_size = MAX_ACCOUNT_DATA_BLOCK_LEN; - tx_cost2.account_data_size = MAX_ACCOUNT_DATA_BLOCK_LEN + 1; - let cost1 = tx_cost1.sum(); - let cost2 = tx_cost2.sum(); - - // build testee that passes - let testee = CostTracker::new( - cmp::max(cost1, cost2), - cost1 + cost2 - 1, - cost1 + cost2 - 1, - None, - ); - assert!(testee.would_fit(&tx_cost1).is_ok()); - // data is too big - assert_eq!( - testee.would_fit(&tx_cost2), - Err(CostTrackerError::WouldExceedAccountDataBlockLimit), - ); - } - - #[test] - fn test_cost_tracker_reach_data_total_limit() { - let (mint_keypair, start_hash) = test_setup(); - // build two transactions with diff accounts - let second_account = Keypair::new(); - let (_tx1, mut tx_cost1) = build_simple_transaction(&mint_keypair, &start_hash); - let (_tx2, mut tx_cost2) = build_simple_transaction(&second_account, &start_hash); - let remaining_account_data_size = 1234; - tx_cost1.account_data_size = remaining_account_data_size; - tx_cost2.account_data_size = remaining_account_data_size + 1; - let cost1 = tx_cost1.sum(); - let cost2 = tx_cost2.sum(); - - // build testee that passes - let testee = CostTracker::new( - cmp::max(cost1, cost2), - cost1 + cost2 - 1, - cost1 + cost2 - 1, - Some(remaining_account_data_size), - ); - assert!(testee.would_fit(&tx_cost1).is_ok()); - // data is too big - assert_eq!( - testee.would_fit(&tx_cost2), - Err(CostTrackerError::WouldExceedAccountDataTotalLimit), - ); - } - - #[test] - fn test_cost_tracker_remove() { - let (mint_keypair, start_hash) = test_setup(); - // build two transactions with diff accounts - let second_account = Keypair::new(); - let (_tx1, tx_cost1) = build_simple_transaction(&mint_keypair, &start_hash); - let (_tx2, tx_cost2) = build_simple_transaction(&second_account, &start_hash); - let cost1 = tx_cost1.sum(); - let cost2 = tx_cost2.sum(); - // build testee - let mut testee = CostTracker::new(cost1 + cost2, cost1 + cost2, cost1 + cost2, None); - - assert!(testee.try_add(&tx_cost1).is_ok()); - assert!(testee.try_add(&tx_cost2).is_ok()); - assert_eq!(cost1 + cost2, testee.block_cost); - - // removing a tx_cost affects block_cost - testee.remove(&tx_cost1); - assert_eq!(cost2, testee.block_cost); - - // add back tx1 - assert!(testee.try_add(&tx_cost1).is_ok()); - assert_eq!(cost1 + cost2, testee.block_cost); - - // cannot add tx1 again, cost limit would be exceeded - assert!(testee.try_add(&tx_cost1).is_err()); - } - - #[test] ->>>>>>> 9e07272af (- Only commit successfully executed transactions' cost to cost_tracker;) fn test_cost_tracker_try_add_is_atomic() { let (mint_keypair, start_hash) = test_setup(); // build two mocking vote transactions with diff accounts