Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Revert "v2.0: Refactor and additional metrics for cost tracking (backport of #1888) (#1900) #1937

Merged
merged 1 commit into from
Jul 1, 2024
Merged

Revert "v2.0: Refactor and additional metrics for cost tracking (backport of #1888) (#1900) #1937

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

36 changes: 30 additions & 6 deletions core/benches/consumer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use {
solana_runtime::bank::Bank,
solana_sdk::{
account::{Account, ReadableAccount},
feature_set::apply_cost_tracker_during_replay,
signature::Keypair,
signer::Signer,
stake_history::Epoch,
Expand Down Expand Up @@ -96,7 +97,7 @@ struct BenchFrame {
signal_receiver: Receiver<(Arc<Bank>, (Entry, u64))>,
}

fn setup() -> BenchFrame {
fn setup(apply_cost_tracker_during_replay: bool) -> BenchFrame {
let mint_total = u64::MAX;
let GenesisConfigInfo {
mut genesis_config, ..
Expand All @@ -108,6 +109,10 @@ fn setup() -> BenchFrame {

let mut bank = Bank::new_for_benches(&genesis_config);

if !apply_cost_tracker_during_replay {
bank.deactivate_feature(&apply_cost_tracker_during_replay::id());
}

// Allow arbitrary transaction processing time for the purposes of this bench
bank.ns_per_slot = u128::MAX;

Expand All @@ -134,7 +139,11 @@ fn setup() -> BenchFrame {
}
}

fn bench_process_and_record_transactions(bencher: &mut Bencher, batch_size: usize) {
fn bench_process_and_record_transactions(
bencher: &mut Bencher,
batch_size: usize,
apply_cost_tracker_during_replay: bool,
) {
const TRANSACTIONS_PER_ITERATION: usize = 64;
assert_eq!(
TRANSACTIONS_PER_ITERATION % batch_size,
Expand All @@ -152,7 +161,7 @@ fn bench_process_and_record_transactions(bencher: &mut Bencher, batch_size: usiz
poh_recorder,
poh_service,
signal_receiver: _signal_receiver,
} = setup();
} = setup(apply_cost_tracker_during_replay);
let consumer = create_consumer(&poh_recorder);
let transactions = create_transactions(&bank, 2_usize.pow(20));
let mut transaction_iter = transactions.chunks(batch_size);
Expand All @@ -177,15 +186,30 @@ fn bench_process_and_record_transactions(bencher: &mut Bencher, batch_size: usiz

#[bench]
fn bench_process_and_record_transactions_unbatched(bencher: &mut Bencher) {
bench_process_and_record_transactions(bencher, 1);
bench_process_and_record_transactions(bencher, 1, true);
}

#[bench]
fn bench_process_and_record_transactions_half_batch(bencher: &mut Bencher) {
bench_process_and_record_transactions(bencher, 32);
bench_process_and_record_transactions(bencher, 32, true);
}

#[bench]
fn bench_process_and_record_transactions_full_batch(bencher: &mut Bencher) {
bench_process_and_record_transactions(bencher, 64);
bench_process_and_record_transactions(bencher, 64, true);
}

#[bench]
fn bench_process_and_record_transactions_unbatched_disable_tx_cost_update(bencher: &mut Bencher) {
bench_process_and_record_transactions(bencher, 1, false);
}

#[bench]
fn bench_process_and_record_transactions_half_batch_disable_tx_cost_update(bencher: &mut Bencher) {
bench_process_and_record_transactions(bencher, 32, false);
}

#[bench]
fn bench_process_and_record_transactions_full_batch_disable_tx_cost_update(bencher: &mut Bencher) {
bench_process_and_record_transactions(bencher, 64, false);
}
39 changes: 34 additions & 5 deletions core/src/banking_stage/consumer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,14 +518,27 @@ impl Consumer {

// Costs of all transactions are added to the cost_tracker before processing.
// To ensure accurate tracking of compute units, transactions that ultimately
// were not included in the block should have their cost removed, the rest
// should update with their actually consumed units.
QosService::remove_or_update_costs(
// were not included in the block should have their cost removed.
QosService::remove_costs(
transaction_qos_cost_results.iter(),
commit_transactions_result.as_ref().ok(),
bank,
);

// once feature `apply_cost_tracker_during_replay` is activated, leader shall no longer
// adjust block with executed cost (a behavior more inline with bankless leader), it
// should use requested, or default `compute_unit_limit` as transaction's execution cost.
if !bank
.feature_set
.is_active(&feature_set::apply_cost_tracker_during_replay::id())
{
QosService::update_costs(
transaction_qos_cost_results.iter(),
commit_transactions_result.as_ref().ok(),
bank,
);
}

retryable_transaction_indexes
.iter_mut()
.for_each(|x| *x += chunk_offset);
Expand Down Expand Up @@ -1419,6 +1432,16 @@ mod tests {

#[test]
fn test_bank_process_and_record_transactions_cost_tracker() {
for apply_cost_tracker_during_replay_enabled in [true, false] {
bank_process_and_record_transactions_cost_tracker(
apply_cost_tracker_during_replay_enabled,
);
}
}

fn bank_process_and_record_transactions_cost_tracker(
apply_cost_tracker_during_replay_enabled: bool,
) {
solana_logger::setup();
let GenesisConfigInfo {
genesis_config,
Expand All @@ -1427,6 +1450,9 @@ mod tests {
} = create_slow_genesis_config(10_000);
let mut bank = Bank::new_for_tests(&genesis_config);
bank.ns_per_slot = u128::MAX;
if !apply_cost_tracker_during_replay_enabled {
bank.deactivate_feature(&feature_set::apply_cost_tracker_during_replay::id());
}
let bank = bank.wrap_with_bank_forks_for_tests().0;
let pubkey = solana_sdk::pubkey::new_rand();

Expand Down Expand Up @@ -1495,7 +1521,8 @@ mod tests {

// TEST: it's expected that the allocation will execute but the transfer will not
// because of a shared write-lock between mint_keypair. Ensure only the first transaction
// takes compute units in the block
// takes compute units in the block AND the apply_cost_tracker_during_replay_enabled feature
// is applied correctly
let allocate_keypair = Keypair::new();
let transactions = sanitize_transactions(vec![
system_transaction::allocate(
Expand Down Expand Up @@ -1534,7 +1561,7 @@ mod tests {
);
assert_eq!(retryable_transaction_indexes, vec![1]);

let expected_block_cost = {
let expected_block_cost = if !apply_cost_tracker_during_replay_enabled {
let (actual_programs_execution_cost, actual_loaded_accounts_data_size_cost) =
match commit_transactions_result.first().unwrap() {
CommitTransactionDetails::Committed {
Expand All @@ -1560,6 +1587,8 @@ mod tests {
}

block_cost + cost.sum()
} else {
block_cost + CostModel::calculate_cost(&transactions[0], &bank.feature_set).sum()
};

assert_eq!(get_block_cost(), expected_block_cost);
Expand Down
126 changes: 83 additions & 43 deletions core/src/banking_stage/qos_service.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,28 +132,39 @@ impl QosService {
(select_results, num_included)
}

/// Removes transaction costs from the cost tracker if not committed or recorded, or
/// updates the transaction costs for committed transactions.
pub fn remove_or_update_costs<'a>(
/// Updates the transaction costs for committed transactions. Does not handle removing costs
/// for transactions that didn't get recorded or committed
pub fn update_costs<'a>(
transaction_cost_results: impl Iterator<Item = &'a transaction::Result<TransactionCost>>,
transaction_committed_status: Option<&Vec<CommitTransactionDetails>>,
bank: &Bank,
) {
if let Some(transaction_committed_status) = transaction_committed_status {
Self::update_committed_transaction_costs(
transaction_cost_results,
transaction_committed_status,
bank,
)
}
}

/// Removes transaction costs from the cost tracker if not committed or recorded
pub fn remove_costs<'a>(
transaction_cost_results: impl Iterator<Item = &'a transaction::Result<TransactionCost>>,
transaction_committed_status: Option<&Vec<CommitTransactionDetails>>,
bank: &Bank,
) {
match transaction_committed_status {
Some(transaction_committed_status) => {
Self::remove_or_update_recorded_transaction_costs(
transaction_cost_results,
transaction_committed_status,
bank,
)
}
None => Self::remove_unrecorded_transaction_costs(transaction_cost_results, bank),
Some(transaction_committed_status) => Self::remove_uncommitted_transaction_costs(
transaction_cost_results,
transaction_committed_status,
bank,
),
None => Self::remove_transaction_costs(transaction_cost_results, bank),
}
}

/// For recorded transactions, remove units reserved by uncommitted transaction, or update
/// units for committed transactions.
fn remove_or_update_recorded_transaction_costs<'a>(
fn remove_uncommitted_transaction_costs<'a>(
transaction_cost_results: impl Iterator<Item = &'a transaction::Result<TransactionCost>>,
transaction_committed_status: &Vec<CommitTransactionDetails>,
bank: &Bank,
Expand All @@ -167,31 +178,45 @@ impl QosService {
// checked for update
if let Ok(tx_cost) = tx_cost {
num_included += 1;
match transaction_committed_details {
CommitTransactionDetails::Committed {
compute_units,
loaded_accounts_data_size,
} => {
cost_tracker.update_execution_cost(
tx_cost,
*compute_units,
CostModel::calculate_loaded_accounts_data_size_cost(
*loaded_accounts_data_size,
&bank.feature_set,
),
);
}
CommitTransactionDetails::NotCommitted => {
cost_tracker.remove(tx_cost);
}
if *transaction_committed_details == CommitTransactionDetails::NotCommitted {
cost_tracker.remove(tx_cost)
}
}
});
cost_tracker.sub_transactions_in_flight(num_included);
}

/// Remove reserved units for transaction batch that unsuccessfully recorded.
fn remove_unrecorded_transaction_costs<'a>(
fn update_committed_transaction_costs<'a>(
transaction_cost_results: impl Iterator<Item = &'a transaction::Result<TransactionCost>>,
transaction_committed_status: &Vec<CommitTransactionDetails>,
bank: &Bank,
) {
let mut cost_tracker = bank.write_cost_tracker().unwrap();
transaction_cost_results
.zip(transaction_committed_status)
.for_each(|(estimated_tx_cost, transaction_committed_details)| {
// Only transactions that the qos service included have to be
// checked for update
if let Ok(estimated_tx_cost) = estimated_tx_cost {
if let CommitTransactionDetails::Committed {
compute_units,
loaded_accounts_data_size,
} = transaction_committed_details
{
cost_tracker.update_execution_cost(
estimated_tx_cost,
*compute_units,
CostModel::calculate_loaded_accounts_data_size_cost(
*loaded_accounts_data_size,
&bank.feature_set,
),
)
}
}
});
}

fn remove_transaction_costs<'a>(
transaction_cost_results: impl Iterator<Item = &'a transaction::Result<TransactionCost>>,
bank: &Bank,
) {
Expand Down Expand Up @@ -759,11 +784,18 @@ mod tests {
+ (execute_units_adjustment + loaded_accounts_data_size_cost_adjustment)
* transaction_count;

QosService::remove_or_update_costs(
qos_cost_results.iter(),
Some(&committed_status),
&bank,
// All transactions are committed, no costs should be removed
QosService::remove_costs(qos_cost_results.iter(), Some(&committed_status), &bank);
assert_eq!(
total_txs_cost,
bank.read_cost_tracker().unwrap().block_cost()
);
assert_eq!(
transaction_count,
bank.read_cost_tracker().unwrap().transaction_count()
);

QosService::update_costs(qos_cost_results.iter(), Some(&committed_status), &bank);
assert_eq!(
final_txs_cost,
bank.read_cost_tracker().unwrap().block_cost()
Expand Down Expand Up @@ -811,7 +843,18 @@ mod tests {
bank.read_cost_tracker().unwrap().block_cost()
);

QosService::remove_or_update_costs(qos_cost_results.iter(), None, &bank);
// update costs doesn't impact non-committed
QosService::update_costs(qos_cost_results.iter(), None, &bank);
assert_eq!(
total_txs_cost,
bank.read_cost_tracker().unwrap().block_cost()
);
assert_eq!(
transaction_count,
bank.read_cost_tracker().unwrap().transaction_count()
);

QosService::remove_costs(qos_cost_results.iter(), None, &bank);
assert_eq!(0, bank.read_cost_tracker().unwrap().block_cost());
assert_eq!(0, bank.read_cost_tracker().unwrap().transaction_count());
}
Expand Down Expand Up @@ -883,11 +926,8 @@ mod tests {
})
.collect();

QosService::remove_or_update_costs(
qos_cost_results.iter(),
Some(&committed_status),
&bank,
);
QosService::remove_costs(qos_cost_results.iter(), Some(&committed_status), &bank);
QosService::update_costs(qos_cost_results.iter(), Some(&committed_status), &bank);

// assert the final block cost
let mut expected_final_txs_count = 0u64;
Expand Down
Loading