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

cmd, core, eth: journal local transactions to disk #14784

Merged
merged 4 commits into from Jul 28, 2017

Conversation

Projects
None yet
2 participants
@karalabe
Member

karalabe commented Jul 10, 2017

This PR adds a disk journal to the transaction pool to back up local transactions. Upon startup, the contents of the journal are merged into the transaction pool, each transaction (and inherently sender address) marked as local. After that on startup, and every hour (configurable via cli) the current contents of the transaction pool are written out into a replacement journal and moved in place of the old one.

During the hour between the journal regeneration, the current journal is kept open append-only, and any arriving local transaction is injected into it.

@karalabe karalabe requested review from holiman, fjl and Arachnid Jul 10, 2017

@karalabe karalabe added this to the 1.6.7 milestone Jul 10, 2017

@fjl

Code looks OK, but I can imagine this breaking if many transactions are sent. Another thing to think about is how we could make this feature work with the light client.

Maybe the journal could be moved to its own object so it can send transactions into any pool, not just core.TxPool.

@karalabe

This comment has been minimized.

Show comment
Hide comment
@karalabe

karalabe Jul 11, 2017

Member

Why would this break anything if many transactions are sent? The journal is only ever reprocessed on startup, otherwise it's just overwritten every once in a while.

Member

karalabe commented Jul 11, 2017

Why would this break anything if many transactions are sent? The journal is only ever reprocessed on startup, otherwise it's just overwritten every once in a while.

@karalabe karalabe modified the milestones: 1.6.8, 1.6.7 Jul 11, 2017

karalabe added some commits Jul 10, 2017

@karalabe

This comment has been minimized.

Show comment
Hide comment
@karalabe

karalabe Jul 12, 2017

Member

@fjl @Arachnid PTAL. I've separated out the tx journal into its own type. Currently the tx pool contains the journal, but the journal itself is not aware of the pool:

  • The load methods which does the initial parsing of the dump and loading it into the pool accepts a callback 😱 to AddLocal to inject anything parsed directly into the pool. The reason it doesn't return all the transactions directly is to avoid loading them into memory, if most will be stale anyway and get discarded immediately. The reason for the callback vs. an interface is because currently the journal doesn't have a notion of local txs. It just operates on whatever it's given. It's cleaner imho not to mix locals in there too.
  • The rotate method requires a dump of transactions to push into the journal. This breaks the other link that the journal had to the transaction pool. This does require an extra method in inside the pool to collect the local transactions, but we'd need that either way if we want to seal the journal off from the pool.

I can look into the light pool in a followup PR. In all honesty I'd rather just use this pool, but if that's not possible, I can make any adjustments needed to get the journal in there too.

Member

karalabe commented Jul 12, 2017

@fjl @Arachnid PTAL. I've separated out the tx journal into its own type. Currently the tx pool contains the journal, but the journal itself is not aware of the pool:

  • The load methods which does the initial parsing of the dump and loading it into the pool accepts a callback 😱 to AddLocal to inject anything parsed directly into the pool. The reason it doesn't return all the transactions directly is to avoid loading them into memory, if most will be stale anyway and get discarded immediately. The reason for the callback vs. an interface is because currently the journal doesn't have a notion of local txs. It just operates on whatever it's given. It's cleaner imho not to mix locals in there too.
  • The rotate method requires a dump of transactions to push into the journal. This breaks the other link that the journal had to the transaction pool. This does require an extra method in inside the pool to collect the local transactions, but we'd need that either way if we want to seal the journal off from the pool.

I can look into the light pool in a followup PR. In all honesty I'd rather just use this pool, but if that's not possible, I can make any adjustments needed to get the journal in there too.

@karalabe karalabe modified the milestones: 1.7.0, 1.6.8 Jul 14, 2017

@karalabe karalabe merged commit 3d32690 into ethereum:master Jul 28, 2017

2 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
commit-message-check/gitcop All commit messages are valid
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

markya0616 added a commit to getamis/go-ethereum that referenced this pull request Aug 7, 2017

cmd, core, eth: journal local transactions to disk (ethereum#14784)
* core: reduce txpool event loop goroutines and sync structs

* cmd, core, eth: journal local transactions to disk

* core: journal replacement pending transactions too

* core: separate transaction journal from pool

markya0616 added a commit to markya0616/go-ethereum that referenced this pull request Aug 17, 2017

cmd, core, eth: journal local transactions to disk (ethereum#14784)
* core: reduce txpool event loop goroutines and sync structs

* cmd, core, eth: journal local transactions to disk

* core: journal replacement pending transactions too

* core: separate transaction journal from pool
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment