Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

TransactionState: add TransactionCost #34881

Merged
merged 2 commits into from
Jan 23, 2024
Merged

TransactionState: add TransactionCost #34881

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
43b427b
TransactionState: add TransactionCost
apfitzge Jan 22, 2024
62d5ed2
InFlightTracker - track TransactionCost CUs instead of requested
apfitzge Jan 22, 2024
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
14 changes: 8 additions & 6 deletions core/src/banking_stage/transaction_scheduler/prio_graph_scheduler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,9 +191,7 @@ impl PrioGraphScheduler {
saturating_add_assign!(num_scheduled, 1);

let sanitized_transaction_ttl = transaction_state.transition_to_pending();
let cu_limit = transaction_state
.transaction_priority_details()
.compute_unit_limit;
let cost = transaction_state.transaction_cost().sum();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a quick check: for Vote, its cu_limit is always 0, but has value of 3428 for cost. By using different CUs, will it impact how vote's scheduled?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vote's don't go through the scheduler - they have their own independent threads still.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this PR realy has no impact on its' own. The CUs are tracked, but not used for load-balancing currently because they are so unreliable. This is intended to be used later on, but right now its' just tracked, so changing from cu_limit to cost shouldn't impact anything.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, good to know it is for the purpose of load-balancing, not counting towards the limits.

Also worth noting that transaction_cost includes requested_cu_limit, if used. So that unreliability also in transaction_cost atm.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, a major difference here is that for simple txs which we know the cost of, it will not include "requested_cus". For example a simple transfer. So for purposes of prioritizing by reward/cu we want to use this number

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that would de-prioritize txs with large requested_cu but actually not-so-largge_actual-cu, if signature fee is included in reward/cu calculation, would it?


let SanitizedTransactionTTL {
transaction,
Expand All @@ -203,7 +201,7 @@ impl PrioGraphScheduler {
batches.transactions[thread_id].push(transaction);
batches.ids[thread_id].push(id.id);
batches.max_age_slots[thread_id].push(max_age_slot);
saturating_add_assign!(batches.total_cus[thread_id], cu_limit);
saturating_add_assign!(batches.total_cus[thread_id], cost);

// If target batch size is reached, send only this batch.
if batches.ids[thread_id].len() >= TARGET_NUM_TRANSACTIONS_PER_BATCH {
Expand Down Expand Up @@ -492,10 +490,12 @@ mod tests {
crate::banking_stage::consumer::TARGET_NUM_TRANSACTIONS_PER_BATCH,
crossbeam_channel::{unbounded, Receiver},
itertools::Itertools,
solana_cost_model::cost_model::CostModel,
solana_runtime::transaction_priority_details::TransactionPriorityDetails,
solana_sdk::{
compute_budget::ComputeBudgetInstruction, hash::Hash, message::Message, pubkey::Pubkey,
signature::Keypair, signer::Signer, system_instruction, transaction::Transaction,
compute_budget::ComputeBudgetInstruction, feature_set::FeatureSet, hash::Hash,
message::Message, pubkey::Pubkey, signature::Keypair, signer::Signer,
system_instruction, transaction::Transaction,
},
std::borrow::Borrow,
};
Expand Down Expand Up @@ -568,6 +568,7 @@ mod tests {
let id = TransactionId::new(index as u64);
let transaction =
prioritized_tranfers(from_keypair.borrow(), to_pubkeys, lamports, priority);
let transaction_cost = CostModel::calculate_cost(&transaction, &FeatureSet::default());
let transaction_ttl = SanitizedTransactionTTL {
transaction,
max_age_slot: Slot::MAX,
Expand All @@ -579,6 +580,7 @@ mod tests {
priority,
compute_unit_limit: 1,
},
transaction_cost,
);
}

Expand Down
4 changes: 4 additions & 0 deletions core/src/banking_stage/transaction_scheduler/scheduler_controller.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use {
},
crossbeam_channel::RecvTimeoutError,
solana_accounts_db::transaction_error_metrics::TransactionErrorMetrics,
solana_cost_model::cost_model::CostModel,
solana_measure::measure_us,
solana_runtime::{bank::Bank, bank_forks::BankForks},
solana_sdk::{
Expand Down Expand Up @@ -342,6 +343,8 @@ impl SchedulerController {
{
saturating_add_assign!(post_transaction_check_count, 1);
let transaction_id = self.transaction_id_generator.next();

let transaction_cost = CostModel::calculate_cost(&transaction, &bank.feature_set);
let transaction_ttl = SanitizedTransactionTTL {
transaction,
max_age_slot: last_slot_in_epoch,
Expand All @@ -351,6 +354,7 @@ impl SchedulerController {
transaction_id,
transaction_ttl,
priority_details,
transaction_cost,
) {
saturating_add_assign!(self.count_metrics.num_dropped_on_capacity, 1);
}
Expand Down
30 changes: 30 additions & 0 deletions core/src/banking_stage/transaction_scheduler/transaction_state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use {
solana_cost_model::transaction_cost::TransactionCost,
solana_runtime::transaction_priority_details::TransactionPriorityDetails,
solana_sdk::{slot_history::Slot, transaction::SanitizedTransaction},
};
Expand Down Expand Up @@ -34,11 +35,13 @@ pub(crate) enum TransactionState {
Unprocessed {
transaction_ttl: SanitizedTransactionTTL,
transaction_priority_details: TransactionPriorityDetails,
transaction_cost: TransactionCost,
forwarded: bool,
},
/// The transaction is currently scheduled or being processed.
Pending {
transaction_priority_details: TransactionPriorityDetails,
transaction_cost: TransactionCost,
forwarded: bool,
},
}
Expand All @@ -48,10 +51,12 @@ impl TransactionState {
pub(crate) fn new(
transaction_ttl: SanitizedTransactionTTL,
transaction_priority_details: TransactionPriorityDetails,
transaction_cost: TransactionCost,
) -> Self {
Self::Unprocessed {
transaction_ttl,
transaction_priority_details,
transaction_cost,
forwarded: false,
}
}
Expand All @@ -70,6 +75,18 @@ impl TransactionState {
}
}

/// Returns a reference to the transaction cost of the transaction.
pub(crate) fn transaction_cost(&self) -> &TransactionCost {
match self {
Self::Unprocessed {
transaction_cost, ..
} => transaction_cost,
Self::Pending {
transaction_cost, ..
} => transaction_cost,
}
}

/// Returns the priority of the transaction.
pub(crate) fn priority(&self) -> u64 {
self.transaction_priority_details().priority
Expand Down Expand Up @@ -103,10 +120,12 @@ impl TransactionState {
TransactionState::Unprocessed {
transaction_ttl,
transaction_priority_details,
transaction_cost,
forwarded,
} => {
*self = TransactionState::Pending {
transaction_priority_details,
transaction_cost,
forwarded,
};
transaction_ttl
Expand All @@ -128,11 +147,13 @@ impl TransactionState {
TransactionState::Unprocessed { .. } => panic!("already unprocessed"),
TransactionState::Pending {
transaction_priority_details,
transaction_cost,
forwarded,
} => {
*self = Self::Unprocessed {
transaction_ttl,
transaction_priority_details,
transaction_cost,
forwarded,
}
}
Expand Down Expand Up @@ -162,6 +183,9 @@ impl TransactionState {
priority: 0,
compute_unit_limit: 0,
},
transaction_cost: TransactionCost::SimpleVote {
writable_accounts: vec![],
},
forwarded: false,
},
)
Expand All @@ -172,6 +196,7 @@ impl TransactionState {
mod tests {
use {
super::*,
solana_cost_model::transaction_cost::UsageCostDetails,
solana_sdk::{
compute_budget::ComputeBudgetInstruction, hash::Hash, message::Message,
signature::Keypair, signer::Signer, system_instruction, transaction::Transaction,
Expand All @@ -190,6 +215,10 @@ mod tests {
];
let message = Message::new(&ixs, Some(&from_keypair.pubkey()));
let tx = Transaction::new(&[&from_keypair], message, Hash::default());
let transaction_cost = TransactionCost::Transaction(UsageCostDetails {
signature_cost: 5000,
..UsageCostDetails::default()
});

let transaction_ttl = SanitizedTransactionTTL {
transaction: SanitizedTransaction::from_transaction_for_tests(tx),
Expand All @@ -202,6 +231,7 @@ mod tests {
priority,
compute_unit_limit: 0,
},
transaction_cost,
)
}

Expand Down
33 changes: 27 additions & 6 deletions core/src/banking_stage/transaction_scheduler/transaction_state_container.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use {
},
crate::banking_stage::scheduler_messages::TransactionId,
min_max_heap::MinMaxHeap,
solana_cost_model::transaction_cost::TransactionCost,
solana_runtime::transaction_priority_details::TransactionPriorityDetails,
std::collections::HashMap,
};
Expand Down Expand Up @@ -125,12 +126,17 @@ impl TransactionStateContainer {
transaction_id: TransactionId,
transaction_ttl: SanitizedTransactionTTL,
transaction_priority_details: TransactionPriorityDetails,
transaction_cost: TransactionCost,
) -> bool {
let priority_id =
TransactionPriorityId::new(transaction_priority_details.priority, transaction_id);
self.id_to_transaction_state.insert(
transaction_id,
TransactionState::new(transaction_ttl, transaction_priority_details),
TransactionState::new(
transaction_ttl,
transaction_priority_details,
transaction_cost,
),
);
self.push_id_into_queue(priority_id)
}
Expand Down Expand Up @@ -176,8 +182,10 @@ impl TransactionStateContainer {
mod tests {
use {
super::*,
solana_cost_model::cost_model::CostModel,
solana_sdk::{
compute_budget::ComputeBudgetInstruction,
feature_set::FeatureSet,
hash::Hash,
message::Message,
signature::Keypair,
Expand All @@ -188,7 +196,13 @@ mod tests {
},
};

fn test_transaction(priority: u64) -> (SanitizedTransactionTTL, TransactionPriorityDetails) {
fn test_transaction(
priority: u64,
) -> (
SanitizedTransactionTTL,
TransactionPriorityDetails,
TransactionCost,
) {
let from_keypair = Keypair::new();
let ixs = vec![
system_instruction::transfer(
Expand All @@ -199,10 +213,14 @@ mod tests {
ComputeBudgetInstruction::set_compute_unit_price(priority),
];
let message = Message::new(&ixs, Some(&from_keypair.pubkey()));
let tx = Transaction::new(&[&from_keypair], message, Hash::default());

let tx = SanitizedTransaction::from_transaction_for_tests(Transaction::new(
&[&from_keypair],
message,
Hash::default(),
));
let transaction_cost = CostModel::calculate_cost(&tx, &FeatureSet::default());
let transaction_ttl = SanitizedTransactionTTL {
transaction: SanitizedTransaction::from_transaction_for_tests(tx),
transaction: tx,
max_age_slot: Slot::MAX,
};
(
Expand All @@ -211,17 +229,20 @@ mod tests {
priority,
compute_unit_limit: 0,
},
transaction_cost,
)
}

fn push_to_container(container: &mut TransactionStateContainer, num: usize) {
for id in 0..num as u64 {
let priority = id;
let (transaction_ttl, transaction_priority_details) = test_transaction(priority);
let (transaction_ttl, transaction_priority_details, transaction_cost) =
test_transaction(priority);
container.insert_new_transaction(
TransactionId::new(id),
transaction_ttl,
transaction_priority_details,
transaction_cost,
);
}
}
Expand Down