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

Support epoch 2 #2310

Merged
merged 9 commits into from
Sep 29, 2019
Merged

Support epoch 2 #2310

merged 9 commits into from
Sep 29, 2019

Conversation

wezrule
Copy link
Contributor

@wezrule wezrule commented Sep 23, 2019

This adds support for an epoch 2 upgrade as well as to further support adding new ones in the future. We currently have state_v1, pending_v1 & accounts_v1 databases which had all epoch 1 blocks. This makes it easy to decide what version the block/account is, however it does mean in some instances we have to check multiple tables, eg following the existing practice when trying to find a state block, it would end up looking like:

auto success = store.block_get (txn, state_v2, hash);  
if (!success)  
{
    success = store.block_get (txn, state_v1, hash);
    if (!success)
    {
        success = store.block_get (txn, state_v0, hash);
    }
}
// etc..

Instead of just:
auto success = store.block_get (txn, state, hash);
This is repeated for the pending blocks too. To do this the epoch is stored with each value.

rocksdb_merge_iterator can be removed as a result as well as various other code. lmdb still needs the merge iterator for upgrades.

Thanks to @cryptocode for pointing out MDB_APPEND. The upgrade process only took 9 minutes on my machine to merge 20M state blocks, 1.1M accounts and all the pending blocks to their respective singular table. This does require appending in the correct order. I am traversing the accounts and pending tables and putting them into memory, clearing the table and writing to it. Unfortunately the state tables is quite large now that I didn't think it was feasible to put them all in memory. I create a new one called state_blocks and put it there instead. I'm not aware of a way to change the database name afterwards.

(Bug) - nano::ledger::link () was hardcoded to use nano::epoch::epoch_1

The epoch 2 signer will not be the genesis account, but the account hasn't been decided yet, so genesis is used as a placeholder for now.

Resolving the failing upgrade tests seemed to take the most time when doing this. Various _v1 tables need to be recreated (as they are deleted during upgrade) and putting various entries back into those tables so that a proper upgrade test could be done.

Note on the essential copy pasta for the new *_v14 functions in lmdb.cpp. I've made a conscious to do this as these are only needed during the upgrade process. It all stems from having to read from the state_v1 database and returning if it was found there so that the correct epoch could be set. It would otherwise require turning these into templates/passing in callbacks so that we would know which functions to correctly call depending on whether we are currently upgrading or not. This didn't very seem nice for the general case. Especially as once we have a snapshot, this code can then just easily be deleted with no changes to existing code paths.

@wezrule wezrule added this to the V20.0 milestone Sep 23, 2019
@wezrule wezrule self-assigned this Sep 23, 2019
Copy link
Contributor

@cryptocode cryptocode left a comment

Choose a reason for hiding this comment

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

LGTM, ~6 min upgrade on release, ssd nvme

@SergiySW
Copy link
Contributor

~5 minutes to upgrade, Sata SSD Samsung 860 EVO

nano/node/lmdb/lmdb.cpp Outdated Show resolved Hide resolved
nano/secure/ledger.cpp Outdated Show resolved Hide resolved
nano/secure/ledger.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@SergiySW SergiySW left a comment

Choose a reason for hiding this comment

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

Looking good with question about sequential epochs

@SergiySW SergiySW mentioned this pull request Sep 27, 2019
@wezrule wezrule merged commit e68ae64 into nanocurrency:master Sep 29, 2019
@wezrule wezrule deleted the epoch_2 branch September 29, 2019 10:50
@zhyatt zhyatt added the major This item indicates the need for or supplies a major or notable change label Oct 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement major This item indicates the need for or supplies a major or notable change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants