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

Improve speed of nonconsensus data removal #2717

Merged
merged 2 commits into from
Sep 23, 2019

Conversation

pasqu4le
Copy link
Contributor

Motivation

The recently introduction of non-consensus block's data removal is not efficient enough to keep the pace with block creation.

Changelog

Enhancements

This PR changes the logic to remove only data of blocks that lose consensus because they are being overridden (as opposed to also for blocks that are invalid neighbors) and changes the queries to perform.

Checklist for your PR

Problem: removal of nonconsensus data is too inefficient and as a result blocks are imported too slow.

Solution: reformulation of deletion logic for better performance
@pasqu4le pasqu4le self-assigned this Sep 20, 2019
@pasqu4le pasqu4le added the ready for review This PR is ready for reviews. label Sep 20, 2019
@coveralls
Copy link

Pull Request Test Coverage Report for Build 16282840-fb5b-4d95-9aba-e9af732f0d66

  • 18 of 22 (81.82%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.03%) to 78.506%

Changes Missing Coverage Covered Lines Changed/Added Lines %
apps/explorer/lib/explorer/chain/import/runner/blocks.ex 18 22 81.82%
Totals Coverage Status
Change from base Build d94c369a-8b26-40e5-96e1-4626be9542eb: -0.03%
Covered Lines: 5245
Relevant Lines: 6681

💛 - Coveralls

{_, result} =
acquire_query =
from(
block in where_invalid_neighbour(changes_list),
Copy link
Contributor

Choose a reason for hiding this comment

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

PR's description says

This PR changes the logic to remove only data of blocks that lose consensus because they are being overridden (as opposed to also for blocks that are invalid **neighbors**)

it looks like it also removes data from invalid neighbours because they are selected with where_invalid_neighbour(changes_list)

I see optimisation in using block_hash'es for selecting transactions instread of using block_number's because we have DB index for block_hash in the transactions table

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ayrat555 yes the invalid neighbors blocks are still removed, however if you look at each remove_nonconsensus_xxx step you'll find that now they take transaction hashes from the result of derive_transaction_forks, in other terms all the hashes of the transactions that have been forked.

What I figured was in fact that we do not fork transactions for invalid neighbors because those will be handled after anyway (neighbor loses consensus > get refetched > new block is inserted and old transactions get forked) and we can do the same for the rest of the nonconsensus data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review This PR is ready for reviews.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants