Skip to content
This repository has been archived by the owner on Oct 28, 2021. It is now read-only.

Write database minor version to disk and fix rebuild on minor version mismatch bug #5827

Merged
merged 2 commits into from
Nov 16, 2019

Conversation

halfalicious
Copy link
Contributor

@halfalicious halfalicious commented Nov 15, 2019

Fix #5822, #5813

Built ontop of #5826 so will wait to merge this until that has been merged.

This PR fixes two bugs:

  • It writes the database minor version to disk (if it's not already present such as when you launch Aleth with a new database path) - this is required to detect database minor version changes which occur during extras database upgrades (5822)
  • It doesn't remove the extras or state databases on minor version mismatch (5813) - removing these databases breaks the rebuild functionality because the rebuild functionality uses the last hash to determine chain tip (and therefore the range of blocks which must be reimported) - however, BlockChain::open populates this after renaming extras -> extras.old so since it can't find the "best" block in the extras DB it just uses the genesis hash.

@halfalicious halfalicious force-pushed the minorversion branch 2 times, most recently from bd53953 to 6472149 Compare November 15, 2019 06:55
@halfalicious halfalicious requested review from gumb0 and chfast November 15, 2019 06:55
@halfalicious halfalicious added the database Database-related work label Nov 15, 2019
@@ -268,7 +276,7 @@ unsigned BlockChain::open(fs::path const& _path, WithExisting _we)
throw;
}

if (_we != WithExisting::Verify && !details(m_genesisHash))
if (_we != WithExisting::Verify && !rebuildNeeded && !details(m_genesisHash))
Copy link
Member

Choose a reason for hiding this comment

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

If do this insertion here in case rebuildNeeded == true, maybe then we don't need to duplicate this code in rebuild ?

Copy link
Member

Choose a reason for hiding this comment

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

Ah I see, rebuild will first rename this database anyway

Rebuild is triggered on db minorversion mismatch, but BlockChain::open
removes the extras database so rebuild won't find any blocks to
reimport. Address this by not removing the extras database on
minorversion mismatch.
@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@dba2a7e). Click here to learn what that means.
The diff coverage is 22.22%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #5827   +/-   ##
=========================================
  Coverage          ?   64.08%           
=========================================
  Files             ?      360           
  Lines             ?    30898           
  Branches          ?     3427           
=========================================
  Hits              ?    19800           
  Misses            ?     9870           
  Partials          ?     1228

@halfalicious
Copy link
Contributor Author

Rebased

@halfalicious halfalicious merged commit 403dc90 into master Nov 16, 2019
@halfalicious halfalicious deleted the minorversion branch November 16, 2019 05:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
database Database-related work
Projects
None yet
3 participants