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.

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

Evict transaction from transaction pool #2797

Merged
merged 2 commits into from
May 13, 2019

Conversation

quentinlesceller
Copy link
Member

Replace #2706 and fix #1417.

Thanks to #2758 we can drastically simplify the eviction process when the transaction pool is full:

  1. Whenever we receive a stem transaction, we check if adding it would overflow the current pool capacity.
  2. Add the transaction to the transaction pool.
  3. If overflow then bucket_transactions and remove the last one (No other tx depends on it, it has low fee_to_weight and is unlikely to participate in any cut-through).

Copy link
Member

@antiochp antiochp left a comment

Choose a reason for hiding this comment

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

Looks good - one comment about the actual remove step.

// Get last transaction and remove it
match bucket_transactions.last() {
Some(evictable_transaction) => {
// Remove transaction
Copy link
Member

Choose a reason for hiding this comment

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

We should be able use entries.remove() here I think.

Copy link
Member Author

@quentinlesceller quentinlesceller May 3, 2019

Choose a reason for hiding this comment

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

I'm not sure this is doable since we are comparing a PoolEntry with a Transaction.
If both were PoolEntry a remove_item(...) would be possible
(or there is a cheap way to get the index of the PoolEntry that I am missing).

Copy link
Member

Choose a reason for hiding this comment

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

Ah good point.

@ignopeverell ignopeverell merged commit 59db5e3 into mimblewimble:master May 13, 2019
@quentinlesceller quentinlesceller deleted the evict2 branch May 14, 2019 02:29
@antiochp antiochp added this to the 1.1.0 milestone Jun 5, 2019
@antiochp antiochp added the release notes To be included in release notes (of relevant milestone). label Jun 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement release notes To be included in release notes (of relevant milestone).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Evict transactions from the pool by order of fee/weight
3 participants