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

eth: don't accept transactions until we sync up with the network #2649

Merged
merged 1 commit into from
Jun 6, 2016

Conversation

karalabe
Copy link
Member

@karalabe karalabe commented Jun 2, 2016

When booting the node, we're most probably out of sync. There's no point in gathering and processing transactions at this point in time, as it could take a considerable amount of time to sync up, and shuffling transactions between every block is a wasted resource either way. This PR introduces a mechanism to discard inbound transactions until either the downloader reports a successful sync cycle, or until the fetcher schedules something for us. This should ensure that newly started nodes don't waste time processing transactions against a stale chain.

Further all the accumulates transactions are propagated to all newly joining peers, which additionally puts a large strain on the network and downloader, which needs to play nice with an already saturated connection.


Just for the reference, by the time a sync reaches 700K blocks, it accumulates currently about 7K pending transactions and 9K queued ones, each of which requires revalidation after every block and/or import batch.

@robotally
Copy link

robotally commented Jun 2, 2016

Vote Count Reviewers
👍 0
👎 0

Updated: Mon Jun 6 11:16:55 UTC 2016

@karalabe karalabe force-pushed the omit-startup-tx-processing branch from 43fc7ab to 170ee07 Compare June 2, 2016 13:11
@codecov-io
Copy link

Current coverage is 56.83%

Merging #2649 into develop will decrease coverage by <.01%

@@            develop      #2649   diff @@
==========================================
  Files           216        216          
  Lines         24539      24543     +4   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits          13953      13949     -4   
- Misses        10585      10593     +8   
  Partials          1          1          

Powered by Codecov. Last updated by 16a23ff...170ee07

@karalabe karalabe added this to the 1.4.6 milestone Jun 2, 2016
@JasonCoombs
Copy link

https://gitter.im/ethereum/go-ethereum?at=5750a31110f0fed86f4accbf
@karalabe #2649 is a winner. I was wondering why we were processing transactions at all before being sync'ed -- with so many moving parts, though, I wanted to see what the problem was as my blockchain got larger so it wasn't until now thanks to your PR that my sync is getting beyond 1,300,000 blocks. (attempting to sync for first time on high-latency satellite internet)

@karalabe
Copy link
Member Author

karalabe commented Jun 3, 2016

Glad to hear it. Btw, I've also identified an issue with the QoS PR. It works beautifully for fast sync but normal sync doesn't do enough requests fast enough to allow converging on to good peers. Maybe that's why it didn't produce the most stable results, I'm looking into fixing it.

// Transactions arrived, make sure we have a valid chain to handle them
if atomic.LoadUint32(&pm.fastSync) == 1 {
// Transactions arrived, make sure we have a valid and fresh chain to handle them
if atomic.LoadUint32(&pm.fastSync) == 1 || atomic.LoadUint32(&pm.synced) == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need the fast sync check? The fetcher doesn't run until fast sync is over, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

We agreed to remove the check in a call.

@fjl
Copy link
Contributor

fjl commented Jun 6, 2016

👍

@karalabe karalabe force-pushed the omit-startup-tx-processing branch from 45512e9 to 32559cc Compare June 6, 2016 11:45
@karalabe karalabe merged commit fdd61b8 into ethereum:develop Jun 6, 2016
@obscuren obscuren removed the review label Jun 6, 2016
maoueh pushed a commit to streamingfast/go-ethereum that referenced this pull request Aug 30, 2024
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