removes OrderedIterator and transaction batch iteration order (backport #16153) (#16510)

Co-authored-by: behzad nouri <behzadnouri@gmail.com>
This commit is contained in:
mergify[bot]
2021-04-13 12:47:44 +00:00
committed by GitHub
parent 4276591eb9
commit a6b346d876
14 changed files with 162 additions and 363 deletions

View File

@@ -25,7 +25,6 @@ use solana_runtime::{
bank_utils,
commitment::VOTE_THRESHOLD_SIZE,
transaction_batch::TransactionBatch,
transaction_utils::OrderedIterator,
vote_account::ArcVoteAccount,
vote_sender_types::ReplayVoteSender,
};
@@ -76,10 +75,7 @@ fn get_first_error(
fee_collection_results: Vec<Result<()>>,
) -> Option<(Result<()>, Signature)> {
let mut first_err = None;
for (result, (_, transaction)) in fee_collection_results.iter().zip(OrderedIterator::new(
batch.transactions(),
batch.iteration_order(),
)) {
for (result, transaction) in fee_collection_results.iter().zip(batch.transactions()) {
if let Err(ref err) = result {
if first_err.is_none() {
first_err = Some((result.clone(), transaction.signatures[0]));
@@ -149,7 +145,6 @@ fn execute_batch(
transaction_status_sender.send_transaction_status_batch(
bank.clone(),
batch.transactions(),
batch.iteration_order_vec(),
execution_results,
balances,
token_balances,
@@ -208,7 +203,7 @@ fn execute_batches(
/// 4. Update the leader scheduler, goto 1
pub fn process_entries(
bank: &Arc<Bank>,
entries: &[Entry],
entries: &mut [Entry],
randomize: bool,
transaction_status_sender: Option<TransactionStatusSender>,
replay_vote_sender: Option<&ReplayVoteSender>,
@@ -224,9 +219,10 @@ pub fn process_entries(
)
}
// Note: If randomize is true this will shuffle entries' transactions in-place.
fn process_entries_with_callback(
bank: &Arc<Bank>,
entries: &[Entry],
entries: &mut [Entry],
randomize: bool,
entry_callback: Option<&ProcessCallback>,
transaction_status_sender: Option<TransactionStatusSender>,
@@ -236,6 +232,12 @@ fn process_entries_with_callback(
// accumulator for entries that can be processed in parallel
let mut batches = vec![];
let mut tick_hashes = vec![];
if randomize {
let mut rng = thread_rng();
for entry in entries.iter_mut() {
entry.transactions.shuffle(&mut rng);
}
}
for entry in entries {
if entry.is_tick() {
// If it's a tick, save it for later
@@ -261,17 +263,8 @@ fn process_entries_with_callback(
}
// else loop on processing the entry
loop {
let iteration_order = if randomize {
let mut iteration_order: Vec<usize> = (0..entry.transactions.len()).collect();
iteration_order.shuffle(&mut thread_rng());
Some(iteration_order)
} else {
None
};
// try to lock the accounts
let batch = bank.prepare_batch(&entry.transactions, iteration_order);
let batch = bank.prepare_batch(&entry.transactions);
let first_lock_err = first_err(batch.lock_results());
// if locking worked
@@ -667,7 +660,7 @@ pub fn confirm_slot(
) -> result::Result<(), BlockstoreProcessorError> {
let slot = bank.slot();
let (entries, num_shreds, slot_full) = {
let (mut entries, num_shreds, slot_full) = {
let mut load_elapsed = Measure::start("load_elapsed");
let load_result = blockstore
.get_slot_entries_with_shred_info(slot, progress.num_shreds, allow_dead_slots)
@@ -728,10 +721,11 @@ pub fn confirm_slot(
let mut replay_elapsed = Measure::start("replay_elapsed");
let mut execute_timings = ExecuteTimings::default();
// Note: This will shuffle entries' transactions in-place.
let process_result = process_entries_with_callback(
bank,
&entries,
true,
&mut entries,
true, // shuffle transactions.
entry_callback,
transaction_status_sender,
replay_vote_sender,
@@ -1099,7 +1093,6 @@ pub enum TransactionStatusMessage {
pub struct TransactionStatusBatch {
pub bank: Arc<Bank>,
pub transactions: Vec<Transaction>,
pub iteration_order: Option<Vec<usize>>,
pub statuses: Vec<TransactionExecutionResult>,
pub balances: TransactionBalancesSet,
pub token_balances: TransactionTokenBalancesSet,
@@ -1118,7 +1111,6 @@ impl TransactionStatusSender {
&self,
bank: Arc<Bank>,
transactions: &[Transaction],
iteration_order: Option<Vec<usize>>,
statuses: Vec<TransactionExecutionResult>,
balances: TransactionBalancesSet,
token_balances: TransactionTokenBalancesSet,
@@ -1136,7 +1128,6 @@ impl TransactionStatusSender {
.send(TransactionStatusMessage::Batch(TransactionStatusBatch {
bank,
transactions: transactions.to_vec(),
iteration_order,
statuses,
balances,
token_balances,
@@ -1883,7 +1874,8 @@ pub mod tests {
} = create_genesis_config(2);
let bank = Arc::new(Bank::new(&genesis_config));
let keypair = Keypair::new();
let slot_entries = create_ticks(genesis_config.ticks_per_slot, 1, genesis_config.hash());
let mut slot_entries =
create_ticks(genesis_config.ticks_per_slot, 1, genesis_config.hash());
let tx = system_transaction::transfer(
&mint_keypair,
&keypair.pubkey(),
@@ -1898,7 +1890,7 @@ pub mod tests {
);
// Now ensure the TX is accepted despite pointing to the ID of an empty entry.
process_entries(&bank, &slot_entries, true, None, None).unwrap();
process_entries(&bank, &mut slot_entries, true, None, None).unwrap();
assert_eq!(bank.process_transaction(&tx), Ok(()));
}
@@ -2103,7 +2095,10 @@ pub mod tests {
// ensure bank can process a tick
assert_eq!(bank.tick_height(), 0);
let tick = next_entry(&genesis_config.hash(), 1, vec![]);
assert_eq!(process_entries(&bank, &[tick], true, None, None), Ok(()));
assert_eq!(
process_entries(&bank, &mut [tick], true, None, None),
Ok(())
);
assert_eq!(bank.tick_height(), 1);
}
@@ -2136,7 +2131,7 @@ pub mod tests {
);
let entry_2 = next_entry(&entry_1.hash, 1, vec![tx]);
assert_eq!(
process_entries(&bank, &[entry_1, entry_2], true, None, None),
process_entries(&bank, &mut [entry_1, entry_2], true, None, None),
Ok(())
);
assert_eq!(bank.get_balance(&keypair1.pubkey()), 2);
@@ -2194,7 +2189,7 @@ pub mod tests {
assert_eq!(
process_entries(
&bank,
&[entry_1_to_mint, entry_2_to_3_mint_to_1],
&mut [entry_1_to_mint, entry_2_to_3_mint_to_1],
false,
None,
None,
@@ -2266,7 +2261,7 @@ pub mod tests {
assert!(process_entries(
&bank,
&[entry_1_to_mint.clone(), entry_2_to_3_mint_to_1.clone()],
&mut [entry_1_to_mint.clone(), entry_2_to_3_mint_to_1.clone()],
false,
None,
None,
@@ -2280,13 +2275,13 @@ pub mod tests {
// Check all accounts are unlocked
let txs1 = &entry_1_to_mint.transactions[..];
let txs2 = &entry_2_to_3_mint_to_1.transactions[..];
let batch1 = bank.prepare_batch(txs1, None);
let batch1 = bank.prepare_batch(txs1);
for result in batch1.lock_results() {
assert!(result.is_ok());
}
// txs1 and txs2 have accounts that conflict, so we must drop txs1 first
drop(batch1);
let batch2 = bank.prepare_batch(txs2, None);
let batch2 = bank.prepare_batch(txs2);
for result in batch2.lock_results() {
assert!(result.is_ok());
}
@@ -2374,7 +2369,7 @@ pub mod tests {
assert!(process_entries(
&bank,
&[
&mut [
entry_1_to_mint,
entry_2_to_3_and_1_to_mint,
entry_conflict_itself,
@@ -2429,7 +2424,7 @@ pub mod tests {
system_transaction::transfer(&keypair2, &keypair4.pubkey(), 1, bank.last_blockhash());
let entry_2 = next_entry(&entry_1.hash, 1, vec![tx]);
assert_eq!(
process_entries(&bank, &[entry_1, entry_2], true, None, None),
process_entries(&bank, &mut [entry_1, entry_2], true, None, None),
Ok(())
);
assert_eq!(bank.get_balance(&keypair3.pubkey()), 1);
@@ -2463,7 +2458,7 @@ pub mod tests {
let present_account = Account::new(1, 10, &Pubkey::default());
bank.store_account(&present_account_key.pubkey(), &present_account);
let entries: Vec<_> = (0..NUM_TRANSFERS)
let mut entries: Vec<_> = (0..NUM_TRANSFERS)
.step_by(NUM_TRANSFERS_PER_ENTRY)
.map(|i| {
let mut transactions = (0..NUM_TRANSFERS_PER_ENTRY)
@@ -2489,7 +2484,10 @@ pub mod tests {
next_entry_mut(&mut hash, 0, transactions)
})
.collect();
assert_eq!(process_entries(&bank, &entries, true, None, None), Ok(()));
assert_eq!(
process_entries(&bank, &mut entries, true, None, None),
Ok(())
);
}
#[test]
@@ -2549,7 +2547,10 @@ pub mod tests {
// Transfer lamports to each other
let entry = next_entry(&bank.last_blockhash(), 1, tx_vector);
assert_eq!(process_entries(&bank, &[entry], true, None, None), Ok(()));
assert_eq!(
process_entries(&bank, &mut [entry], true, None, None),
Ok(())
);
bank.squash();
// Even number keypair should have balance of 2 * initial_lamports and
@@ -2607,7 +2608,13 @@ pub mod tests {
system_transaction::transfer(&keypair1, &keypair4.pubkey(), 1, bank.last_blockhash());
let entry_2 = next_entry(&tick.hash, 1, vec![tx]);
assert_eq!(
process_entries(&bank, &[entry_1, tick, entry_2.clone()], true, None, None),
process_entries(
&bank,
&mut [entry_1, tick, entry_2.clone()],
true,
None,
None
),
Ok(())
);
assert_eq!(bank.get_balance(&keypair3.pubkey()), 1);
@@ -2618,7 +2625,7 @@ pub mod tests {
system_transaction::transfer(&keypair2, &keypair3.pubkey(), 1, bank.last_blockhash());
let entry_3 = next_entry(&entry_2.hash, 1, vec![tx]);
assert_eq!(
process_entries(&bank, &[entry_3], true, None, None),
process_entries(&bank, &mut [entry_3], true, None, None),
Err(TransactionError::AccountNotFound)
);
}
@@ -2698,7 +2705,7 @@ pub mod tests {
);
assert_eq!(
process_entries(&bank, &[entry_1_to_mint], false, None, None),
process_entries(&bank, &mut [entry_1_to_mint], false, None, None),
Err(TransactionError::AccountInUse)
);
@@ -2848,7 +2855,7 @@ pub mod tests {
let mut hash = bank.last_blockhash();
let mut root: Option<Arc<Bank>> = None;
loop {
let entries: Vec<_> = (0..NUM_TRANSFERS)
let mut entries: Vec<_> = (0..NUM_TRANSFERS)
.step_by(NUM_TRANSFERS_PER_ENTRY)
.map(|i| {
next_entry_mut(&mut hash, 0, {
@@ -2876,9 +2883,9 @@ pub mod tests {
})
.collect();
info!("paying iteration {}", i);
process_entries(&bank, &entries, true, None, None).expect("paying failed");
process_entries(&bank, &mut entries, true, None, None).expect("paying failed");
let entries: Vec<_> = (0..NUM_TRANSFERS)
let mut entries: Vec<_> = (0..NUM_TRANSFERS)
.step_by(NUM_TRANSFERS_PER_ENTRY)
.map(|i| {
next_entry_mut(
@@ -2899,12 +2906,12 @@ pub mod tests {
.collect();
info!("refunding iteration {}", i);
process_entries(&bank, &entries, true, None, None).expect("refunding failed");
process_entries(&bank, &mut entries, true, None, None).expect("refunding failed");
// advance to next block
process_entries(
&bank,
&(0..bank.ticks_per_slot())
&mut (0..bank.ticks_per_slot())
.map(|_| next_entry_mut(&mut hash, 1, vec![]))
.collect::<Vec<_>>(),
true,
@@ -2954,7 +2961,7 @@ pub mod tests {
process_entries_with_callback(
&bank0,
&entries,
&mut entries,
true,
None,
None,
@@ -3034,12 +3041,8 @@ pub mod tests {
bank.last_blockhash(),
);
account_loaded_twice.message.account_keys[1] = mint_keypair.pubkey();
let transactions = [account_loaded_twice, account_not_found_tx];
// Use inverted iteration_order
let iteration_order: Vec<usize> = vec![1, 0];
let batch = bank.prepare_batch(&transactions, Some(iteration_order));
let transactions = [account_not_found_tx, account_loaded_twice];
let batch = bank.prepare_batch(&transactions);
let (
TransactionResults {
fee_collection_results,
@@ -3135,7 +3138,7 @@ pub mod tests {
.collect();
let entry = next_entry(&bank_1_blockhash, 1, vote_txs);
let (replay_vote_sender, replay_vote_receiver) = unbounded();
let _ = process_entries(&bank1, &[entry], true, None, Some(&replay_vote_sender));
let _ = process_entries(&bank1, &mut [entry], true, None, Some(&replay_vote_sender));
let successes: BTreeSet<Pubkey> = replay_vote_receiver
.try_iter()
.map(|(vote_pubkey, _, _)| vote_pubkey)