Enabling JournalPruneDataSource #478

AlexandraRoatis · 2018-05-11T21:00:55Z

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.

minor cleanup in BlockchainDataRecoveryTest.java
re-enabling the use of JournalPruneDataSource.java for Trie pruning
configuration setting for the number of recent blocks to be maintained in the database (with a minimum value set to 128)
utils for exploring the state trie
functionality for rebuilding the genesis block when recovery is triggered by a missing state at kernel startup
unit tests for the JournalPruneDataSource.java data storage and retrieval operations

Fixes Issue #298.

Type of change

Insert x into the following checkboxes to confirm (eg. [x]):

Bug fix.
New feature.
Breaking change (a fix or feature that causes existing functionality to not work as expected).
Requires documentation update.

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.

In addition to the unit tests above:

the kernel was run with different settings of pruneBlockCount (10, 100, 1000, 10000) for over 24 hours, only the version keeping only 10 blocks resulted in issues, which are now avoided due to the minimum value setting
ran the kernel with negative and zero pruneBlockCount and can confirm pruning disabled
unit tests show that values are not cached when the pruning is disabled (note: the class was initially disabled due to memory usage concerns)
ran the kernel after reverting to blocks without stored state and confirmed that the genesis was rebuilt and full state recovery was successful

Verification

Insert x into the following checkboxes to confirm (eg. [x]):

I have self-reviewed my own code and conformed to the style guidelines of this project.
New and existing tests pass locally with my changes.
I have added tests for my fix or feature.
I have made appropriate changes to the corresponding documentation.
My code generates no new warnings.
Any dependent changes have been made.

…lock printing utility

…for the pruneBlockCount variable

AionJayT

few performance enhancement comments.

AionJayT · 2018-05-16T15:36:35Z

modAionImpl/src/org/aion/zero/impl/AionHub.java

+                this.repository.getBlockStore().saveBlock(genesis, genesis.getCumulativeDifficulty(), true);
+                LOG.info("Rebuilding genesis block SUCCEEDED.");
+            }
+


how about extract this code snippet to a mathod?

AionJayT · 2018-05-16T19:20:14Z

modMcf/src/org/aion/mcf/trie/JournalPruneDataSource.java

                } else if (ref != null) {
                    ref.dbRef = false;
                }
            }
-            src.putBatch(batchRemove);
+            src.deleteBatch(batchRemove);


how many keys need to be removed during the prune? if it's a lot, use parallel-stream to shorten the processing time instead of the for loop?

AionJayT · 2018-05-16T19:35:49Z

modMcf/src/org/aion/mcf/trie/JournalPruneDataSource.java

+            return;
+        }
+        // deletes are delayed
+        keys.forEach(key -> currentUpdates.deletedKeys.add(new ByteArrayWrapper(key)));


same here. can we do parallel?

AlexandraRoatis · 2018-05-17T14:33:39Z

I think the batch sizes are too small to need parallelism here at present.

aion-sridevi · 2018-05-17T15:45:36Z

modAionImpl/src/org/aion/zero/impl/AionHub.java

+
+                LOG.info("Rebuilding genesis block SUCCEEDED.");
+            }
+


What if buildGenesis fails? Do we need any log message here?

if there is anything wrong with the genesis file an error message will be printed before recovery is attempted

qoire · 2018-05-15T14:34:59Z

modAionImpl/src/org/aion/zero/impl/db/AionRepositoryImpl.java

@@ -76,7 +76,12 @@ protected AionRepositoryImpl(IRepositoryConfig repoConfig) {
        // repository singleton instance
        private final static AionRepositoryImpl inst = new AionRepositoryImpl(
                new RepositoryConfig(new File(config.getBasePath(), config.getDb().getPath()).getAbsolutePath(),
-                                     -1,
+                                     config.getDb().getPrune() > 0 ?
+                                             // if the value is smaller than backward step


Pruning value should dictate parameters for our sync. So if we set a value of 16 as our pruning parameter, sync should respect that value.

the interaction between pruning and sync is tuned in #498

iamyulong · 2018-05-22T21:28:13Z

modAionImpl/src/org/aion/zero/impl/AionHub.java

+                        "Corrupt world state for genesis block hash: " + genesis.getShortHash() + ", number: " + genesis
+                                .getNumber() + ".");
+
+                buildGenesis(genesis);


buildGenesis() seems to be building the genesis state based on the current repository. It's correct for the latter usage, when the database is empty, but is it still valid here?

it's valid, i.e. it always results in the same state root, because when calling createAccount(Address) the stateRoot = EMPTY_TRIE_HASH;, so it does not build on the state root of the tracked repository

AlexandraRoatis added 8 commits May 9, 2018 16:03

cleaning up the data recovery unit tests

024681a

reenabling the journal prune DS #298

a19a604

adding the pune block count to the configuration

349c930

command line utils for exploring the state trie; corrections to the b…

4041344

…lock printing utility

rebuilding the genesis block if missing when attempting state recovery

f8152db

disabled pruning when pruneBlockCount is negative; set a lower bound …

9377138

…for the pruneBlockCount variable

unit tests for JournalPruneDataSource.java

493f112

correcting test case

7e8dfbd

AlexandraRoatis requested review from qoire and AionJayT May 11, 2018 21:00

AlexandraRoatis added this to the v0.2.7 - Island Peak milestone May 11, 2018

AlexandraRoatis added the enhancement New feature or request label May 11, 2018

AlexandraRoatis requested review from iamyulong and aion-sridevi May 14, 2018 18:50

AlexandraRoatis mentioned this pull request May 14, 2018

Refactor and test journal pruning class #482

Merged

10 tasks

AionJayT reviewed May 16, 2018

View reviewed changes

extracting common code into a method

ab40ed4

aion-sridevi reviewed May 17, 2018

View reviewed changes

qoire reviewed May 17, 2018

View reviewed changes

AionJayT modified the milestones: v0.2.7 - Island Peak, v0.2.8 - University Peak May 22, 2018

iamyulong reviewed May 22, 2018

View reviewed changes

This was referenced May 24, 2018

Reenable JournalPruningDataSource after thorough testing #298

Closed

database pruning #54

Closed

AionJayT approved these changes May 24, 2018

View reviewed changes

iamyulong approved these changes May 30, 2018

View reviewed changes

Merge branch 'master-pre-merge' into db-journal-prune

46fccd6

AionJayT merged commit 8ac445d into master-pre-merge Jun 1, 2018

AionJayT mentioned this pull request Jun 14, 2018

merge v0.2.8 features into master #525

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enabling JournalPruneDataSource #478

Enabling JournalPruneDataSource #478

AlexandraRoatis commented May 11, 2018

AionJayT left a comment

AionJayT May 16, 2018

AionJayT May 16, 2018

AionJayT May 16, 2018

AlexandraRoatis commented May 17, 2018

aion-sridevi May 17, 2018

AlexandraRoatis May 24, 2018

qoire May 15, 2018

AlexandraRoatis May 22, 2018

iamyulong May 22, 2018

AlexandraRoatis May 22, 2018

Enabling JournalPruneDataSource #478

Enabling JournalPruneDataSource #478

Conversation

AlexandraRoatis commented May 11, 2018

Description

Type of change

Testing

Verification

AionJayT left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlexandraRoatis commented May 17, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment