-
Notifications
You must be signed in to change notification settings - Fork 790
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
Support epoch 2 #2310
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cryptocode
approved these changes
Sep 24, 2019
There was a problem hiding this 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
~5 minutes to upgrade, Sata SSD Samsung 860 EVO |
SergiySW
reviewed
Sep 25, 2019
SergiySW
reviewed
Sep 25, 2019
SergiySW
reviewed
Sep 25, 2019
SergiySW
reviewed
Sep 25, 2019
SergiySW
approved these changes
Sep 26, 2019
There was a problem hiding this 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
Merged
SergiySW
approved these changes
Sep 27, 2019
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: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 usenano::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 inlmdb.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 thestate_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.