Skip to content

Wire scheduler up to adjust actual loaded accounts size cost for committed transactions#1547

Merged
tao-stones merged 3 commits intoanza-xyz:masterfrom
tao-stones:imp-consumer-adjust-for-actual-loaded-accounts-size-cost
Jun 7, 2024
Merged

Wire scheduler up to adjust actual loaded accounts size cost for committed transactions#1547
tao-stones merged 3 commits intoanza-xyz:masterfrom
tao-stones:imp-consumer-adjust-for-actual-loaded-accounts-size-cost

Conversation

@tao-stones
Copy link
Copy Markdown

Problem

Transaction may set loaded_accounts_data_size limit, or uses default limit, that is higher than it actually loads. When cost model starts to "charge" CU for loaded accounts data size, it is necessary to adjust block space transaction reserved with accounts size it actually loaded, alone with "executed CU" adjustment.

Summary of Changes

  • Add loaded_accounts_data_size to Committed enum variance.
  • Add loaded_accounts_data_size_cost to cost_tracker adjustment logic in update_execution_cost()

Note

Function of "charging CU for loaded accounts" logic is still behind include_loaded_accounts_data_size_in_fee_calculation gate, therefore this PR does not change Scheduler behavior until #1356

Fixes #

Copy link
Copy Markdown

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

I'm not familiar with this code, and I don't see anything obviously touching accounts-db. @tao-stones, is there a guidance on what you'd like me to look at?

Also, I see this PR does new stuff if a feature gate is active, but I don't see a new feature gate in this PR. That implies to me there's already an existing feature gate issue. Can you update the PR description to include the issue number and label (if you edit the PR description, you'll see them as comments at the bottom)? This will help me with the additional context. Thanks!

@tao-stones tao-stones requested a review from bw-solana June 5, 2024 16:42
@tao-stones
Copy link
Copy Markdown
Author

I'm not familiar with this code,

My apology - meant to add @bw-solana to this PR

Comment thread core/src/banking_stage/committer.rs
Comment thread core/src/banking_stage/qos_service.rs Outdated
@tao-stones tao-stones force-pushed the imp-consumer-adjust-for-actual-loaded-accounts-size-cost branch from ca3c4f7 to cdde7a6 Compare June 6, 2024 16:27
Comment thread cost-model/src/cost_tracker.rs
Copy link
Copy Markdown

@bw-solana bw-solana left a comment

Choose a reason for hiding this comment

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

LGTM

@tao-stones tao-stones merged commit d049a79 into anza-xyz:master Jun 7, 2024
@tao-stones tao-stones deleted the imp-consumer-adjust-for-actual-loaded-accounts-size-cost branch June 7, 2024 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants