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

Rename types and metrics from executed to processed #2435

Merged
merged 4 commits into from
Aug 9, 2024
Merged

Rename types and metrics from executed to processed #2435

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2c6f847
Rename types and metrics from executed to processed
jstarry Aug 5, 2024
edc5214
feedback
jstarry Aug 8, 2024
6ee6d40
feedback
jstarry Aug 9, 2024
b94019a
more fixes
jstarry Aug 9, 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
22 changes: 12 additions & 10 deletions core/src/banking_stage/committer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use {
},
solana_measure::measure_us,
solana_runtime::{
bank::{Bank, ExecutedTransactionCounts, TransactionBalancesSet},
bank::{Bank, ProcessedTransactionCounts, TransactionBalancesSet},
bank_utils,
prioritization_fee_cache::PrioritizationFeeCache,
transaction_batch::TransactionBatch,
Expand All @@ -15,7 +15,9 @@ use {
solana_sdk::{hash::Hash, pubkey::Pubkey, saturating_add_assign},
solana_svm::{
transaction_commit_result::{TransactionCommitResult, TransactionCommitResultExtensions},
transaction_execution_result::TransactionExecutionResult,
transaction_processing_result::{
TransactionProcessingResult, TransactionProcessingResultExtensions,
},
},
solana_transaction_status::{
token_balances::TransactionTokenBalancesSet, TransactionTokenBalance,
Expand Down Expand Up @@ -67,27 +69,27 @@ impl Committer {
pub(super) fn commit_transactions(
&self,
batch: &TransactionBatch,
execution_results: Vec<TransactionExecutionResult>,
processing_results: Vec<TransactionProcessingResult>,
last_blockhash: Hash,
lamports_per_signature: u64,
starting_transaction_index: Option<usize>,
bank: &Arc<Bank>,
pre_balance_info: &mut PreBalanceInfo,
execute_and_commit_timings: &mut LeaderExecuteAndCommitTimings,
execution_counts: &ExecutedTransactionCounts,
processed_counts: &ProcessedTransactionCounts,
) -> (u64, Vec<CommitTransactionDetails>) {
let executed_transactions = execution_results
let processed_transactions = processing_results
.iter()
.zip(batch.sanitized_transactions())
.filter_map(|(execution_result, tx)| execution_result.was_executed().then_some(tx))
.filter_map(|(processing_result, tx)| processing_result.was_processed().then_some(tx))
.collect_vec();

let (commit_results, commit_time_us) = measure_us!(bank.commit_transactions(
batch.sanitized_transactions(),
execution_results,
processing_results,
last_blockhash,
lamports_per_signature,
execution_counts,
processed_counts,
&mut execute_and_commit_timings.execute_timings,
));
execute_and_commit_timings.commit_us = commit_time_us;
Expand Down Expand Up @@ -122,7 +124,7 @@ impl Committer {
starting_transaction_index,
);
self.prioritization_fee_cache
.update(bank, executed_transactions.into_iter());
.update(bank, processed_transactions.into_iter());
});
execute_and_commit_timings.find_and_send_votes_us = find_and_send_votes_us;
(commit_time_us, commit_transaction_statuses)
Expand All @@ -145,7 +147,7 @@ impl Committer {
let batch_transaction_indexes: Vec<_> = commit_results
.iter()
.map(|commit_result| {
if commit_result.was_executed() {
if commit_result.was_committed() {
Copy link

Choose a reason for hiding this comment

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

Should this be was_processed?

Copy link
Author

Choose a reason for hiding this comment

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

Committed implies processed. I think committed is more appropriate here

Copy link

Choose a reason for hiding this comment

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

Yeah, think that committed is probably more appropriate here. This is meant to gather and communicate state changes, which will only occur if the tx is committed.

let this_transaction_index = transaction_index;
saturating_add_assign!(transaction_index, 1);
this_transaction_index
Expand Down
36 changes: 18 additions & 18 deletions core/src/banking_stage/consume_worker.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,18 +234,18 @@ impl ConsumeWorkerMetrics {
}: &ExecuteAndCommitTransactionsOutput,
) {
self.count_metrics
.transactions_attempted_execution_count
.transactions_attempted_processing_count
.fetch_add(
transaction_counts.attempted_execution_count,
transaction_counts.attempted_processing_count,
Ordering::Relaxed,
);
self.count_metrics
.executed_transactions_count
.fetch_add(transaction_counts.executed_count, Ordering::Relaxed);
.processed_transactions_count
.fetch_add(transaction_counts.processed_count, Ordering::Relaxed);
self.count_metrics
.executed_with_successful_result_count
.processed_with_successful_result_count
.fetch_add(
transaction_counts.executed_with_successful_result_count,
transaction_counts.processed_with_successful_result_count,
Ordering::Relaxed,
);
self.count_metrics
Expand Down Expand Up @@ -410,9 +410,9 @@ impl ConsumeWorkerMetrics {
}

struct ConsumeWorkerCountMetrics {
transactions_attempted_execution_count: AtomicU64,
executed_transactions_count: AtomicU64,
executed_with_successful_result_count: AtomicU64,
transactions_attempted_processing_count: AtomicU64,
processed_transactions_count: AtomicU64,
processed_with_successful_result_count: AtomicU64,
retryable_transaction_count: AtomicUsize,
retryable_expired_bank_count: AtomicUsize,
cost_model_throttled_transactions_count: AtomicU64,
Expand All @@ -423,9 +423,9 @@ struct ConsumeWorkerCountMetrics {
impl Default for ConsumeWorkerCountMetrics {
fn default() -> Self {
Self {
transactions_attempted_execution_count: AtomicU64::default(),
executed_transactions_count: AtomicU64::default(),
executed_with_successful_result_count: AtomicU64::default(),
transactions_attempted_processing_count: AtomicU64::default(),
processed_transactions_count: AtomicU64::default(),
processed_with_successful_result_count: AtomicU64::default(),
retryable_transaction_count: AtomicUsize::default(),
retryable_expired_bank_count: AtomicUsize::default(),
cost_model_throttled_transactions_count: AtomicU64::default(),
Expand All @@ -441,19 +441,19 @@ impl ConsumeWorkerCountMetrics {
"banking_stage_worker_counts",
"id" => id,
(
"transactions_attempted_execution_count",
self.transactions_attempted_execution_count
"transactions_attempted_processing_count",
self.transactions_attempted_processing_count
.swap(0, Ordering::Relaxed),
i64
),
(
"executed_transactions_count",
self.executed_transactions_count.swap(0, Ordering::Relaxed),
"processed_transactions_count",
self.processed_transactions_count.swap(0, Ordering::Relaxed),
i64
),
(
"executed_with_successful_result_count",
self.executed_with_successful_result_count
"processed_with_successful_result_count",
self.processed_with_successful_result_count
.swap(0, Ordering::Relaxed),
i64
),
Expand Down
Loading