Skip to content

Transaction pool optimizations#2742

Merged
obscuren merged 5 commits intoethereum:developfrom
karalabe:tx-spam-protection
Sep 2, 2016
Merged

Transaction pool optimizations#2742
obscuren merged 5 commits intoethereum:developfrom
karalabe:tx-spam-protection

Conversation

@karalabe
Copy link
Copy Markdown
Member

@karalabe karalabe commented Jun 27, 2016

Commit core, eth, miner: only retain 1 tx/nonce, remove bad ones

Our transaction pool currently maintains arbitrarily many transactions with a given nonce. This isn't the best because only the highest paying transaction will be executed either way, so everything else is just wasting resources. This PR reworks the txpool internals so that instead of using a set of transactions (that we cannot do specialized checks on) it uses per-account nonce-indexed transaction list maintenance. This allows any newly added transaction to be immediately compared to existing ones and only the best retained.

The PR also fixes another bug in the worker whereby invalid transactions were not removed from the pool.

Commit core, eth, internal, miner: optimize txpool for quick ops

Even though the previous commit changed the internal representation of pending/queued transactions from flat sets to per-account nonce-sorted lists, most of the transaction pool algorithms still operated on them as if they were unsorted, loosing precious performance.

This commit reworks the actual operational implementation of the transaction pool so that it takes advantage of the already sorted representation of the transactions. It further enhances the nonce-gapped queued transactions with a heap-based nonce index to allow quickly locating processable transactions instead of needing to always verify every transaction individually.

The PR also caches sorted versions of the pending/future queues so that multiple consecutive queries without actual data mutations are fast. Similarly, to avoid the costly balance filtering (dropping txs which cannot be funded by the sender) during every cycle, each accounts' transaction list also caches the cost of the highest transaction. This ensures that if the account balance is higher that the cost of the most expensive transaction, there's no need to individually check all of them.

Commit core/types, miner: switch over to the grouped tx sets

The miner currently first sorted the entire list of pending transactions, then iterated it and incorporated them into the blocks one by one. If however a transaction from a specific account could not be executed (failed, out of gas, low price), all subsequent transactions needed to be checked against various ignore lists and dropped.

Since this PR already maintains and delivers transactions on a per-account-nonce-sorted basis, its much more optimal to do the sorting on the fly, pulling the best transaction for incorporation but only inserting the next from the account if all went well. In the case where a transaction fails for some reason, it is simply dropped from the current queue, preventing any other txs from the same account ever entering in its place and wasting time (nonce error).

Commit core: add upper bound on the queued transactions

Currently there is no upper limit on the total number of queued (non executable) transaction in the transaction pool, only a per account limit. This could lead to nasty stuff if someone were to create thousands of accounts to spam transactions with.

This commit places an upper limit of 64K transactions that's queueable, after which transactions from the account with the least recent activity (i.e. least recently executed transaction) are starting to be dropped. Further there's also a periodic (1 minute) check that iterates over all queued transactions and drops the stale ones (older than 3 hours) to ensure that we don't accumulate junk over time.

@robotally
Copy link
Copy Markdown

robotally commented Jun 27, 2016

Vote Count Reviewers
👍 1 @Arachnid
👎 0

Updated: Fri Sep 2 11:16:22 UTC 2016

Comment thread core/tx_pool.go Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comments are inaccurate - no sorting!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

*group :)

@Arachnid
Copy link
Copy Markdown
Contributor

👍

@karalabe
Copy link
Copy Markdown
Member Author

I'll pull this back from review and work on it a bit more to update the sort algos and other things too since there's no urgency to push it out now.

@karalabe karalabe changed the title core, eth, miner: only retain 1 tx/nonce, remove bad ones Transaction pool optimizations Jul 1, 2016
@fjl
Copy link
Copy Markdown
Contributor

fjl commented Jul 25, 2016

Do you want this in 1.5?

@karalabe
Copy link
Copy Markdown
Member Author

Yes. I'll try and finish it up tomorrow.

@fjl fjl added this to the 1.5.0 milestone Jul 25, 2016
@fjl fjl self-assigned this Aug 9, 2016
Comment thread core/tx_pool.go Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What if this is the last tx in the queue? Also, is modifying the map while iterating over it safe?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In Go, deleting from a map safe is safe during iteration.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Okay. Shouldn't you delete txs from pool.queue if it's now empty, though? This is why I think some generic add/delete methods would make this easier to follow.

Comment thread core/tx_list.go Outdated
Copy link
Copy Markdown

@awfm9 awfm9 Aug 22, 2016

Choose a reason for hiding this comment

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

teh -> the :trollface:

Comment thread core/tx_list.go Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

blow -> below

@karalabe
Copy link
Copy Markdown
Member Author

@Arachnid PTAL

@Arachnid
Copy link
Copy Markdown
Contributor

I thought about making you implement heapsort, since the heap's already part sorted, but decided I'm not that evil. ;)

👍

@karalabe
Copy link
Copy Markdown
Member Author

I think I'll start keeping a review debt ledger with all the suggestions I owe to others ;)

Comment thread core/tx_list.go Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You could update *m.index outside of the loop before heap.Init once with: *m.index = (*m.index)[:threshold].

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Indeed, thx!

@bas-vk
Copy link
Copy Markdown
Member

bas-vk commented Sep 1, 2016

I tried it and send numerous raw transactions, including transactions out of "nonce order" or with higher gas prices and it seems to work 👍.

@karalabe
Copy link
Copy Markdown
Member Author

karalabe commented Sep 2, 2016

@bas-vk Fixed the array index optimization, rebased and force pushed.

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.

7 participants