-
Notifications
You must be signed in to change notification settings - Fork 114
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
Archive states at runtime #498
Conversation
…present in the archive db
…od to check if open database added to data store interface
463e403
to
631da9e
Compare
rebased to include changes to #482 |
be9f60e
to
f8242db
Compare
5a0e502
to
843378f
Compare
843378f
to
b42dd50
Compare
d9027dc
to
188a483
Compare
492d4ad
to
8a1160b
Compare
After this PR the state pruning mode can be set from the config file before starting the kernel. <!--Data pruning behavior for the state database. Options: FULL, TOP, SPREAD.-->
<!--FULL: the state is not pruned-->
<!--TOP: the state is kept only for the top K blocks; limits sync to branching only within the stored blocks-->
<!--SPREAD: the state is kept for the top K blocks and at regular block intervals-->
<state-storage>FULL</state-storage> If the kernel is using a previous database, it's recommended to run the CLI state data reorganization before starting the kernel:
When switching from TOP mode to FULL or SPREAD mode one should run the CLI command to avoid issues if a backward sync occurs due to re-branching beyond stored block states. If this is not done the kernel may encounter errors at run time. A simple kernel restart will solve these issues however. |
} | ||
|
||
// checking if there are restrictions due to pruning | ||
if (batch.size() > 0 && chain.isPruneRestricted(batch.get(0).getNumber())) { |
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.
batch.size() > 0 -> !batch.empty()
@@ -94,39 +97,51 @@ public void run() { | |||
// decide the start block number | |||
long from = 0; | |||
int size = 24; | |||
|
|||
// depends on the number of blocks going BACKWARD | |||
state.setMaxRepeats(128 / size + 1); |
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.
can we make the backward blk# as a constant? there is a constant class for store the p2p constants.
ImportResult importResult = ImportResult.IMPORTED_NOT_BEST; | ||
|
||
// importing last block in batch to see if we can skip batch | ||
if (state != null && state.getMode() == PeerState.Mode.FORWARD && batch.size() > 0) { |
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.
use !batch.empty() instead of batch.size() > 0 is better
if (state.getRepeated() >= state.getMaxRepeats()) { | ||
state.setMode(PeerState.Mode.NORMAL); | ||
state.setBase(chain.getBestBlock().getNumber()); | ||
state.resetLastHeaderRequest(); |
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.
from line 114 to 270, a lot of code is duplicated, it increased the code readability also the maintenance difficulty.
switch (prune_option) { | ||
case TOP: | ||
// journal prune only | ||
this.prune = new CfgPrune(128); |
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.
same here, make 128 as a constant
break; | ||
case SPREAD: | ||
// journal prune with archived states | ||
this.prune = new CfgPrune(128, 10000); |
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.
Is the default 10K blocks?
this.archived = true; | ||
this.archive_rate = | ||
_archive_rate > MINIMUM_ARCHIVE_RATE ? _archive_rate : MINIMUM_ARCHIVE_RATE; | ||
} |
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.
do we need the upbound Value?
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.
I don't think an upper bound is necessary.
|
||
@Override | ||
public void doOnNode(byte[] hash, Value node) { | ||
db.put(hash, dummy_value); |
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.
what's the second arg for?
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.
Since only the keys are used at present the second argument is not needed. However, it cannot be null since that means deleting the key. In the future it may be used for more meaningful information, like possibly state snapshots to be used in the lightning sync PoC.
} | ||
} | ||
} | ||
} |
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.
can you do extract method for the scanTreeDiffLoop & scanTreeLoop ?
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.
it's difficult to reuse code meaningfully without unnecessary computations in one of the methods
59eca2f
to
e0ca459
Compare
Description
Please include a brief summary of the change that this pull request proposes. Include any relevant motivation and context. List any dependencies required for this change.
Fixes Issue #54.
Note: this PR builds on top of #482.
master-pre-merge
branch.Type of change
Insert x into the following checkboxes to confirm (eg. [x]):
Testing
Please describe the tests you used to validate this pull request. Provide any relevant details for test configurations as well as any instructions to reproduce these results.
./aion.sh --dump-state-size X
where X is the height of the blockchain.Verification
Insert x into the following checkboxes to confirm (eg. [x]):