Remove accounts bloom by dvdplm · Pull Request #11589 · openethereum/parity-ethereum

dvdplm · 2020-03-29T21:09:27Z

Remove the accounts existence bloom filter.

The bloom has numerous problems:

does not scale as the number of accounts increase; resizing the bloom takes several hours
does not handle reorgs
is one-size-fits all, which means that small chains still need to run with a bloom filter sized for ETH, i.e. able to handle 100M accounts (and counting) at a non-negligible runtime cost (120+Mb ram, 7 hash functions for each lookup etc)

For this reason this PR removes the bloom filter entirely. See https://github.com/openethereum/openethereum/pull/11581 for more details and benchmarks.

Remove the accounts existence bloom filter. Used to compare performance of running with no bloom to running with an optimized bloom (see https://github.com/openethereum/openethereum/pull/11581).

* master: (25 commits) Update .gitmodules (#11628) ethcore/res: activate ecip-1088 phoenix on classic (#11598) Upgrade parity-common deps to latest (#11620) Fix Goerli syncing (#11604) deps: switch to upstream ctrlc (#11617) Deduplicating crate dependencies (part 3 of n) (#11614) Deduplicating crate dependencies (part 2 of n, `slab`) (#11613) Actually save ENR on creation and modification (#11602) Activate POSDAO on xDai chain and update bootnodes (#11610) Activate on-chain randomness in POA Core (#11609) Deduplicating crate dependencies (part 1 of n) (#11606) Update enodes for POA Sokol (#11611) Remove .git folder from dogerignore file so vergen library can get build date and commit hash in the binary generatio vergen library can get build date and commit hash in the binary generation (#11608) Reduced gas cost for static calls made to precompiles EIP2046/1352 (#11583) [easy] `ethcore-bloom-journal` was renamed to `accounts-bloom` (#11605) Use serde_json to export hardcoded sync (#11601) Node Discovery v4 ENR Extension (EIP-868) (#11540) Fix compile warnings (#11595) Update version to 3.0.0-alpha.1 (#11592) ethcore/res: bump canon fork hash for mordor and kotti testnets (#11584) ...

* master: Fix ecrecover builtin (#11623)

ordian · 2020-04-14T11:07:33Z

+	// Sometimes when importing a small number of blocks (~10) the report seems
+	// inaccurate, so we sleep a little before getting the report.
+	std::thread::sleep(Duration::from_secs(1));


yeah I have no idea wtf. If you try importing 5-10 blocks the report is all messed up, claiming wrong number of blocks were imported. I tossed in this in and moved on without investigating further.

I was hoping we could figure this out before the merge.

Firstly apologies for adding this change that has nothing to do with the rest of the PR. I'll remove it.

Secondly, I was not overly worried about it because the import is nondeterministic right? Some (user configurable) verifier threads are going to process the import queue and if the number is small maybe there's a race on the ClientReport so giving it a little more time, I thought, was "fine"?

ordian · 2020-04-14T11:09:05Z

+		for (n, (k,_)) in db.iter(COL_ACCOUNT_BLOOM).enumerate() {
+			batch.delete(COL_ACCOUNT_BLOOM, &k);
+			if n > 0 && n % 10_000 == 0 {
+				info!(target: "migration", "  Account Bloom entries queued for deletion: {}", n);
+			}
+		}
+		batch.delete(COL_ACCOUNT_BLOOM, ACCOUNT_BLOOM_HASHCOUNT_KEY);


we could use the new (not published yet) kvdb api to simplify this paritytech/parity-common#368

This migration is really quick to run. We can tweak this when we get around to updating the kvdb-*s.

Co-Authored-By: Andronik Ordian <write@reusable.software>

…h/parity-ethereum into dp/chore/remove-accounts-bloom * 'dp/chore/remove-accounts-bloom' of github.com:paritytech/parity-ethereum: Update ethcore/db/src/db.rs

dvdplm · 2020-04-16T21:59:27Z

One concern that was raised about removing the bloom filter was it might hurt performance in older parts of the chain severely. I tried this by syncing eth from genesis up to block 2.7M.

No blooms:

./openethereum-no-blooms --chain=eth --no-warp --no-jsonrpc --cache-size-db=10000 --pruning=archive
2020-04-14 13:26:42 UTC Syncing    #8386 0xee35…342c
…
2020-04-14 18:59:52 UTC Syncing #2708504 0x1804…c303     2.20 blk/s   12.6 tx/s    2.1 Mgas/s

Took 5h33m10s.

With bloom:

./parities/parity-master-april-10 --chain=eth --no-warp --no-jsonrpc --cache-size-db=10000 --pruning=archive
2020-04-16 14:06:52 UTC Syncing    #1905 0x44c0…dd4e
…
2020-04-16 20:33:58 UTC Syncing #2708504 0x1804…c303     2.00 blk/s   11.2 tx/s    1.7 Mgas/s

Took 06h27m06s

The big difference was something I did not expect.

ordian · 2020-04-20T13:09:44Z

+	// Sometimes when importing a small number of blocks (~10) the report seems
+	// inaccurate, so we sleep a little before getting the report.
+	std::thread::sleep(Duration::from_secs(1));


I was hoping we could figure this out before the merge.

This reverts commit 5f32878.

vorot93

LGTM

Remove accounts bloom

4c45f60

Remove the accounts existence bloom filter. Used to compare performance of running with no bloom to running with an optimized bloom (see https://github.com/openethereum/openethereum/pull/11581).

dvdplm self-assigned this Mar 29, 2020

dvdplm added the A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. label Mar 29, 2020

dvdplm mentioned this pull request Mar 31, 2020

Rebuild the accounts bloom #11581

Closed

dvdplm added 5 commits March 31, 2020 22:34

Measure the block import, not client start-up

5f32878

Tweak comment

1b87034

Migration that clears out the accounts existence bloom

3f5590d

Merge branch 'master' into dp/chore/remove-accounts-bloom

8f45332

* master: Fix ecrecover builtin (#11623)

ordian reviewed Apr 14, 2020

View reviewed changes

Comment thread ethcore/db/src/db.rs Outdated

ordian reviewed Apr 14, 2020

View reviewed changes

Comment thread ethcore/db/src/db.rs Outdated

ordian reviewed Apr 14, 2020

View reviewed changes

Update ethcore/db/src/db.rs

3e12a95

Co-Authored-By: Andronik Ordian <write@reusable.software>

dvdplm marked this pull request as ready for review April 14, 2020 11:32

dvdplm added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Apr 14, 2020

ordian reviewed Apr 14, 2020

View reviewed changes

Comment thread ethcore/snapshot/src/lib.rs Outdated

dvdplm added 2 commits April 14, 2020 13:37

Review feedback

f2bc761

Merge branch 'dp/chore/remove-accounts-bloom' of github.com:paritytec…

143012d

…h/parity-ethereum into dp/chore/remove-accounts-bloom * 'dp/chore/remove-accounts-bloom' of github.com:paritytech/parity-ethereum: Update ethcore/db/src/db.rs

dvdplm requested a review from ordian April 20, 2020 12:09

ordian approved these changes Apr 20, 2020

View reviewed changes

Revert "Measure the block import, not client start-up"

ee68764

This reverts commit 5f32878.

vorot93 approved these changes Apr 22, 2020

View reviewed changes

vorot93 added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Apr 22, 2020

vorot93 merged commit c85300c into master Apr 22, 2020

ordian deleted the dp/chore/remove-accounts-bloom branch April 22, 2020 08:32

dvdplm mentioned this pull request Apr 26, 2020

Drain the transaction overlay #11654

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove accounts bloom#11589

Remove accounts bloom#11589
vorot93 merged 10 commits into
masterfrom
dp/chore/remove-accounts-bloom

dvdplm commented Mar 29, 2020 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

ordian Apr 14, 2020

Uh oh!

dvdplm Apr 14, 2020 •

edited

Loading

Uh oh!

ordian Apr 20, 2020

Uh oh!

dvdplm Apr 20, 2020

Uh oh!

ordian Apr 14, 2020

Uh oh!

dvdplm Apr 14, 2020

Uh oh!

Uh oh!

dvdplm commented Apr 16, 2020

Uh oh!

ordian Apr 20, 2020

Uh oh!

vorot93 left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

dvdplm commented Mar 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ordian Apr 14, 2020

Choose a reason for hiding this comment

Uh oh!

dvdplm Apr 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ordian Apr 20, 2020

Choose a reason for hiding this comment

Uh oh!

dvdplm Apr 20, 2020

Choose a reason for hiding this comment

Uh oh!

ordian Apr 14, 2020

Choose a reason for hiding this comment

Uh oh!

dvdplm Apr 14, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dvdplm commented Apr 16, 2020

Uh oh!

ordian Apr 20, 2020

Choose a reason for hiding this comment

Uh oh!

vorot93 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dvdplm commented Mar 29, 2020 •

edited

Loading

dvdplm Apr 14, 2020 •

edited

Loading