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

Implement leader prioritizing packets by fee per CU #23207

Closed
tao-stones opened this issue Feb 17, 2022 · 6 comments
Closed

Implement leader prioritizing packets by fee per CU #23207

tao-stones opened this issue Feb 17, 2022 · 6 comments
Labels
stale [bot only] Added to stale content; results in auto-close after a week.

Comments

@tao-stones
Copy link
Contributor

tao-stones commented Feb 17, 2022

Problem

To implement the ideas of the 3rd point of #22820 , banking_stage needs to have a way to sort buffered packets by total fee / total CU per transaction. Banking_stage currently iterates batch-by-batch through buffer.

  1. Needs a different way to iterator buffer without introducing additional copy.
  2. Needs to identify how exactly "prioritizing by fee" works.

Proposed Solution

I am exploring following approach to refactor banking_stage, and prioritize packets before ending them down to bank for execution:

  1. refactor banking_stage buffer to partially deserialize packets into VersionedTransaction upon receiving, store it with packet. deserialize packets during receving and buffering #23145

  2. refactor banking_stage::consume_buffered_packets to following order: (WIP)

  • scan entire buffer for each packet info, eg its locator (=batch_index + packet_index), its sender stakes, its total fee/total CU.
  • total fee/total CU: total_fee = tx_addtional_fee + tx_base_fee; All the fees and CUs are from ComputeBudget instruction, hence are deterministic.
  • optional, stake weight shuffle packets, so all buffered packets are lined up with highest staked are likely on the top
  • then sort by total fee/total CU, so all buffered packets are lined up with highest fee paying TXs on the top; should the fee is same, the higher staked will likely to be ahead.
  • sending chunks (chunk size = 128) of SanitizedTransactions to process_transactions(....), where batched transactions are loaded and executed
  • update buffer with retryable transaction after execution (same logic as is)
@aeyakovenko
Copy link
Member

@taozhu-chicago at this stage do we need to worry about 1 writable account with a ton of high fee txs starving out all the other TXs? I want to make sure that when the banking stage starts forwarding that its able to forward TXs that are for other write accounts not just the hot one.

@carllin
Copy link
Contributor

carllin commented Feb 17, 2022

other threads that see that write account being locked should requeue that transaction and move on to the next transaction in the queue, so as long as the transactions are evenly distributed among the banking threads' independent queues, this should be taken care of?

@carllin
Copy link
Contributor

carllin commented Feb 17, 2022

what happens if a fee payer that can not afford to pay the fee continually marks a high additional fee to get themselves to the front of the queue? Seems like we should also ensure fee payer is sufficient?

@wkshare
Copy link

wkshare commented Feb 17, 2022

It's only <3000tps, so why start thinking about controlling transactions through fees?

@tao-stones
Copy link
Contributor Author

@taozhu-chicago at this stage do we need to worry about 1 writable account with a ton of high fee txs starving out all the other TXs? I want to make sure that when the banking stage starts forwarding that its able to forward TXs that are for other write accounts not just the hot one.

Yea, round robin forwarding transactions from different fee-paying buckets would be a separate issue.
On that topic, if current leader makes effort to forward mixed txs; The next leader would still re-prioritize them (logic in this issue) before sending them to bank. Not sure how chances for low-fee not-hot account txs will be improved.

@tao-stones
Copy link
Contributor Author

what happens if a fee payer that can not afford to pay the fee continually marks a high additional fee to get themselves to the front of the queue? Seems like we should also ensure fee payer is sufficient?

Checking fee payer might be necessary so no one makes promise that they can't keep. Thought I coded up something about it before, going to check it out.

@github-actions github-actions bot added the stale [bot only] Added to stale content; results in auto-close after a week. label Feb 20, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Feb 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
stale [bot only] Added to stale content; results in auto-close after a week.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants