gaspersic/solana
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

removes OrderedIterator and transaction batch iteration order (#16153)

Browse Source 
In TransactionBatch,
https://github.com/solana-labs/solana/blob/e50f59844/runtime/src/transaction_batch.rs#L4-L11
lock_results[i] is aligned with transactions[iteration_order[i]]:
https://github.com/solana-labs/solana/blob/e50f59844/runtime/src/bank.rs#L2414-L2424
https://github.com/solana-labs/solana/blob/e50f59844/runtime/src/accounts.rs#L788-L817

However load_and_execute_transactions is iterating over
  lock_results[iteration_order[i]]
https://github.com/solana-labs/solana/blob/e50f59844/runtime/src/bank.rs#L2878-L2889
and then returning i as for the index of the retryable transaction.

If iteratorion_order is [1, 2, 0], and i is 0, then:
  lock_results[iteration_order[i]] = lock_results[1]
which corresponds to
  transactions[iteration_order[1]] = transactions[2]
so neither i = 0, nor iteration_order[i] = 1 gives the correct index for the
corresponding transaction (which is 2).

This commit removes OrderedIterator and transaction batch iteration order
entirely. There is only one place in blockstore processor which the
iteration order is not ordinal:
https://github.com/solana-labs/solana/blob/e50f59844/ledger/src/blockstore_processor.rs#L269-L271
It seems like, instead of using an iteration order, that can shuffle entry
transactions in-place.
This commit is contained in:
behzad nouri
2021-03-31 23:59:19 +00:00
committed by GitHub
parent ad7f8e7f23
commit 3f63ed9a72
14 changed files with 161 additions and 352 deletions
Download Patch File Download Diff File Expand all files Collapse all files

109
ledger/src/blockstore_processor.rs
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>,
@ -228,9 +223,10 @@ pub fn process_entries(
result
}
// 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>,
@ -240,6 +236,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
@ -265,17 +267,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
@ -677,7 +670,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)
@ -738,10 +731,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,
@ -1113,7 +1107,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,
@ -1132,7 +1125,6 @@ impl TransactionStatusSender {
&self,
bank: Arc<Bank>,
transactions: &[Transaction],
iteration_order: Option<Vec<usize>>,
statuses: Vec<TransactionExecutionResult>,
balances: TransactionBalancesSet,
token_balances: TransactionTokenBalancesSet,
@ -1150,7 +1142,6 @@ impl TransactionStatusSender {
.send(TransactionStatusMessage::Batch(TransactionStatusBatch {
bank,
transactions: transactions.to_vec(),
iteration_order,
statuses,
balances,
token_balances,
@ -1897,7 +1888,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(),
@ -1912,7 +1904,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(()));
}
@ -2117,7 +2109,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);
}
@ -2150,7 +2145,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);
@ -2208,7 +2203,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,
@ -2280,7 +2275,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,
@ -2294,13 +2289,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());
}
@ -2388,7 +2383,7 @@ pub mod tests {
assert!(process_entries(
&bank,
&[
&mut [
entry_1_to_mint,
entry_2_to_3_and_1_to_mint,
entry_conflict_itself,
@ -2443,7 +2438,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);
@ -2477,7 +2472,7 @@ pub mod tests {
let present_account = AccountSharedData::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)
@ -2503,7 +2498,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]
@ -2563,7 +2561,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
@ -2621,7 +2622,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);
@ -2632,7 +2639,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)
);
}
@ -2712,7 +2719,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)
);
@ -2863,7 +2870,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, {
@ -2891,9 +2898,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(
@ -2914,12 +2921,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,
@ -2969,7 +2976,7 @@ pub mod tests {
process_entries_with_callback(
&bank0,
&entries,
&mut entries,
true,
None,
None,
@ -3049,12 +3056,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,
@ -3150,7 +3153,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)