From ca051c9a9e10732dea472456c8850fa08e698292 Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Mon, 18 Jul 2022 19:08:53 +0800 Subject: [PATCH 1/4] More pruning scenario --- .../Pruning/TestPruningStrategy.cs | 14 ++- .../PruningScenariosTests.cs | 94 ++++++++++++++++++- 2 files changed, 101 insertions(+), 7 deletions(-) diff --git a/src/Nethermind/Nethermind.Trie.Test/Pruning/TestPruningStrategy.cs b/src/Nethermind/Nethermind.Trie.Test/Pruning/TestPruningStrategy.cs index eb616eb7c3a..cf0f77171ee 100644 --- a/src/Nethermind/Nethermind.Trie.Test/Pruning/TestPruningStrategy.cs +++ b/src/Nethermind/Nethermind.Trie.Test/Pruning/TestPruningStrategy.cs @@ -21,16 +21,24 @@ namespace Nethermind.Trie.Test.Pruning public class TestPruningStrategy : IPruningStrategy { private readonly bool _pruningEnabled; - private readonly bool _shouldPrune; public TestPruningStrategy(bool pruningEnabled, bool shouldPrune = false) { _pruningEnabled = pruningEnabled; - _shouldPrune = shouldPrune; + ShouldPruneEnabled = shouldPrune; } public bool PruningEnabled => _pruningEnabled; public bool ShouldPruneEnabled { get; set; } - public bool ShouldPrune(in long currentMemory) => ShouldPruneEnabled || (_pruningEnabled && _shouldPrune); + public int? WithMemoryLimit { get; set; } + + public bool ShouldPrune(in long currentMemory) + { + if (!_pruningEnabled) return false; + if (ShouldPruneEnabled) return true; + if (WithMemoryLimit != null && currentMemory > WithMemoryLimit) return true; + + return false; + } } } diff --git a/src/Nethermind/Nethermind.Trie.Test/PruningScenariosTests.cs b/src/Nethermind/Nethermind.Trie.Test/PruningScenariosTests.cs index 57c80cf95b6..91e967d5075 100644 --- a/src/Nethermind/Nethermind.Trie.Test/PruningScenariosTests.cs +++ b/src/Nethermind/Nethermind.Trie.Test/PruningScenariosTests.cs @@ -111,6 +111,16 @@ public PruningContext SetAccountBalance(int accountIndex, UInt256 balance) return this; } + public PruningContext SetManyAccountWithSameBalance(int startNum, int numOfAccount, UInt256 balance) + { + for (int i = 0; i < numOfAccount; i++) + { + this.SetAccountBalance(startNum + i, balance); + } + return this; + } + + public PruningContext PruneOldBlock() { return this; @@ -122,6 +132,12 @@ public PruningContext TurnOnPrune() return this; } + public PruningContext SetPruningMemoryLimit(int? memoryLimit) + { + _pruningStrategy.WithMemoryLimit = memoryLimit; + return this; + } + public PruningContext SetStorage(int accountIndex, int storageKey, int storageValue = 1) { _storageProvider.Set( @@ -160,6 +176,13 @@ public PruningContext ReadAccountViaStateReader(int accountIndex) return this; } + public PruningContext CommitWithRandomChange() + { + SetAccountBalance(Random.Shared.Next(), (UInt256)Random.Shared.Next()); + Commit(); + return this; + } + public PruningContext Commit() { _storageProvider.Commit(); @@ -452,30 +475,56 @@ public void Persist_alternate_commitset() { Reorganization.MaxDepth = 3; - PruningContext.InMemoryAlwaysPrune + PruningContext.InMemory .SetAccountBalance(1, 100) .Commit() .SetAccountBalance(2, 10) .Commit() .SaveBranchingPoint("revert_main") - .SetAccountBalance(1, 103) .CreateAccount(3) .SetStorage(3, 1, 999) .Commit() + .Commit() .SaveBranchingPoint("main") + // We need this to get persisted // Storage is not set here, but commit set will commit this instead of block 3 in main as it appear later .RestoreBranchingPoint("revert_main") - .SetAccountBalance(1, 103) + .Commit() + .SetManyAccountWithSameBalance(100, 20, 1) .Commit() .RestoreBranchingPoint("main") + // But not this. Only prune cache for this + .Commit() .Commit() - .SetAccountBalance(4, 108) .Commit() + + .SetPruningMemoryLimit(10000) + // First commit it should prune and persist alternate block 3, memory usage should go down from 16k to 1.2k + .Commit() + + // After this, we need to slowly add node that can be cache-pruned, but does not require persist, so + // same account different balance. Each update of 2 account, add 1.5k of memory. But it need to keep + // at least 3 level of block reorg worth of nodes. + .SetManyAccountWithSameBalance(100, 2, 1) + .Commit() + .SetManyAccountWithSameBalance(100, 2, 2) .Commit() + .SetManyAccountWithSameBalance(100, 2, 3) .Commit() + .SetManyAccountWithSameBalance(100, 2, 4) + .Commit() + .SetManyAccountWithSameBalance(100, 2, 5) + .Commit() + .SetManyAccountWithSameBalance(100, 2, 6) + .Commit() + .SetManyAccountWithSameBalance(100, 2, 7) + .Commit() + .SetManyAccountWithSameBalance(100, 2, 8) + .Commit() + .SetManyAccountWithSameBalance(100, 2, 9) .Commit() // Although the committed set is different, later commit set still refer to the storage root hash. @@ -485,6 +534,43 @@ public void Persist_alternate_commitset() .VerifyStorageValue(3, 1, 999); } + [Test] + public void Persist_alternate_commitset_at_least_2_consecutive_sidechain() + { + Reorganization.MaxDepth = 3; + + PruningContext.InMemory + .SetAccountBalance(1, 100) + .Commit() + .SetAccountBalance(2, 10) + .Commit() + + .SaveBranchingPoint("revert_main") + .CreateAccount(3) + .SetStorage(3, 1, 999) + .Commit() + .Commit() + .SaveBranchingPoint("main") + + // We need this to get persisted + // Storage is not set here, but commit set will commit this instead of block 3 in main as it appear later + .RestoreBranchingPoint("revert_main") + .Commit() + .SetManyAccountWithSameBalance(100, 20, 1) + .Commit() + + .RestoreBranchingPoint("main") + // But not this. Only prune cache for this + .Commit() + .Commit() + .Commit() + + .SetPruningMemoryLimit(10000) + .Commit() + + .VerifyStorageValue(3, 1, 999); + } + [Test] public void StorageRoot_reset_at_lower_level() { From dceb67aa17b99a733b1d2cf4292792d66a36ff65 Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Tue, 19 Jul 2022 15:11:14 +0800 Subject: [PATCH 2/4] More failed pruning scenario --- .../PruningScenariosTests.cs | 82 +++++++++++++++---- 1 file changed, 67 insertions(+), 15 deletions(-) diff --git a/src/Nethermind/Nethermind.Trie.Test/PruningScenariosTests.cs b/src/Nethermind/Nethermind.Trie.Test/PruningScenariosTests.cs index 91e967d5075..caf88745f6b 100644 --- a/src/Nethermind/Nethermind.Trie.Test/PruningScenariosTests.cs +++ b/src/Nethermind/Nethermind.Trie.Test/PruningScenariosTests.cs @@ -485,18 +485,15 @@ public void Persist_alternate_commitset() .CreateAccount(3) .SetStorage(3, 1, 999) .Commit() - .Commit() .SaveBranchingPoint("main") // We need this to get persisted - // Storage is not set here, but commit set will commit this instead of block 3 in main as it appear later + // Storage is not set here, but commit set will commit this instead of previous block 3 .RestoreBranchingPoint("revert_main") - .Commit() .SetManyAccountWithSameBalance(100, 20, 1) .Commit() .RestoreBranchingPoint("main") - // But not this. Only prune cache for this .Commit() .Commit() .Commit() @@ -505,27 +502,33 @@ public void Persist_alternate_commitset() // First commit it should prune and persist alternate block 3, memory usage should go down from 16k to 1.2k .Commit() - // After this, we need to slowly add node that can be cache-pruned, but does not require persist, so - // same account different balance. Each update of 2 account, add 1.5k of memory. But it need to keep - // at least 3 level of block reorg worth of nodes. .SetManyAccountWithSameBalance(100, 2, 1) .Commit() + .VerifyStorageValue(3, 1, 999) .SetManyAccountWithSameBalance(100, 2, 2) .Commit() + .VerifyStorageValue(3, 1, 999) .SetManyAccountWithSameBalance(100, 2, 3) .Commit() + .VerifyStorageValue(3, 1, 999) .SetManyAccountWithSameBalance(100, 2, 4) .Commit() + .VerifyStorageValue(3, 1, 999) .SetManyAccountWithSameBalance(100, 2, 5) .Commit() + .VerifyStorageValue(3, 1, 999) .SetManyAccountWithSameBalance(100, 2, 6) .Commit() + .VerifyStorageValue(3, 1, 999) .SetManyAccountWithSameBalance(100, 2, 7) .Commit() + .VerifyStorageValue(3, 1, 999) .SetManyAccountWithSameBalance(100, 2, 8) .Commit() + .VerifyStorageValue(3, 1, 999) .SetManyAccountWithSameBalance(100, 2, 9) .Commit() + .VerifyStorageValue(3, 1, 999) // Although the committed set is different, later commit set still refer to the storage root hash. // When later set is committed, the storage root hash will be committed also, unless the alternate chain @@ -535,7 +538,7 @@ public void Persist_alternate_commitset() } [Test] - public void Persist_alternate_commitset_at_least_2_consecutive_sidechain() + public void Persist_alternate_commitset_of_length_2() { Reorganization.MaxDepth = 3; @@ -546,26 +549,75 @@ public void Persist_alternate_commitset_at_least_2_consecutive_sidechain() .Commit() .SaveBranchingPoint("revert_main") + + // Block 3 .CreateAccount(3) .SetStorage(3, 1, 999) .Commit() + + // Block 4 .Commit() .SaveBranchingPoint("main") - // We need this to get persisted - // Storage is not set here, but commit set will commit this instead of block 3 in main as it appear later - .RestoreBranchingPoint("revert_main") + // Block 3 - 2 + .RestoreBranchingPoint("revert_main") + .Commit() + + // Block 4 - 2 + .SetManyAccountWithSameBalance(100, 20, 1) + .Commit() + .RestoreBranchingPoint("main") + + .Commit() .Commit() - .SetManyAccountWithSameBalance(100, 20, 1) .Commit() - .RestoreBranchingPoint("main") - // But not this. Only prune cache for this + .TurnOnPrune() .Commit() + + .VerifyStorageValue(3, 1, 999); + } + + [Test] + public void Persist_with_2_alternate_branch_consecutive_of_each_other() + { + Reorganization.MaxDepth = 3; + + PruningContext.InMemory + .SetAccountBalance(1, 100) .Commit() + .SetAccountBalance(2, 10) .Commit() - .SetPruningMemoryLimit(10000) + .SaveBranchingPoint("revert_main") + + // Block 3 + .CreateAccount(3) + .SetStorage(3, 1, 999) + .Commit() + .SaveBranchingPoint("main") + + // Block 3 - 2 + .RestoreBranchingPoint("revert_main") + .Commit() + + .RestoreBranchingPoint("main") + + // Block 4 + .SaveBranchingPoint("revert_main") + .Commit() + .SaveBranchingPoint("main") + + .RestoreBranchingPoint("revert_main") // Go back to block 3 + // Block 4 - 2 + .SetStorage(3, 1, 1) + .Commit() + .RestoreBranchingPoint("main") // Go back to block 4 on main + + .TurnOnPrune() + .Commit() + .Commit() + .Commit() .Commit() .VerifyStorageValue(3, 1, 999); From ee66e686a2fa84bdb45572175df818f9e47bf686 Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Tue, 19 Jul 2022 15:19:51 +0800 Subject: [PATCH 3/4] Fix some test scenario --- .../PruningScenariosTests.cs | 21 +++++------- .../Nethermind.Trie/Pruning/TrieStore.cs | 34 +++++++++++++------ 2 files changed, 33 insertions(+), 22 deletions(-) diff --git a/src/Nethermind/Nethermind.Trie.Test/PruningScenariosTests.cs b/src/Nethermind/Nethermind.Trie.Test/PruningScenariosTests.cs index caf88745f6b..0b24f868cb3 100644 --- a/src/Nethermind/Nethermind.Trie.Test/PruningScenariosTests.cs +++ b/src/Nethermind/Nethermind.Trie.Test/PruningScenariosTests.cs @@ -487,13 +487,13 @@ public void Persist_alternate_commitset() .Commit() .SaveBranchingPoint("main") - // We need this to get persisted - // Storage is not set here, but commit set will commit this instead of previous block 3 - .RestoreBranchingPoint("revert_main") - .SetManyAccountWithSameBalance(100, 20, 1) - .Commit() + // We need this to get persisted + // Storage is not set here, but commit set will commit this instead of previous block 3 + .RestoreBranchingPoint("revert_main") + .SetManyAccountWithSameBalance(100, 20, 1) + .Commit() + .RestoreBranchingPoint("main") - .RestoreBranchingPoint("main") .Commit() .Commit() .Commit() @@ -528,17 +528,14 @@ public void Persist_alternate_commitset() .VerifyStorageValue(3, 1, 999) .SetManyAccountWithSameBalance(100, 2, 9) .Commit() - .VerifyStorageValue(3, 1, 999) - // Although the committed set is different, later commit set still refer to the storage root hash. - // When later set is committed, the storage root hash will be committed also, unless the alternate chain - // is longer than reorg depth, in which case, we have two parallel branch of length > reorg depth, which - // is not supported. + // Storage root actually never got pruned even-though another parallel branch get persisted. This + // is because the condition `LastSeen < LastPersistedBlock` never turn to true. .VerifyStorageValue(3, 1, 999); } [Test] - public void Persist_alternate_commitset_of_length_2() + public void Persist_alternate_branch_commitset_of_length_2() { Reorganization.MaxDepth = 3; diff --git a/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs b/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs index 7629e46b9c6..3efe02f89c0 100644 --- a/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs +++ b/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs @@ -412,28 +412,42 @@ private bool CanPruneCacheFurther() { if (_logger.IsDebug) _logger.Debug("Elevated pruning starting"); - BlockCommitSet? candidateSet = null; - while (_commitSetQueue.TryPeek(out BlockCommitSet? frontSet)) + List toAddBack = new(); + + List candidateSets = new(); + while (_commitSetQueue.TryDequeue(out BlockCommitSet? frontSet)) { if (frontSet!.BlockNumber >= LatestCommittedBlockNumber - Reorganization.MaxDepth) { - break; + toAddBack.Add(frontSet); + continue; } - if (_commitSetQueue.TryDequeue(out frontSet)) + if (candidateSets.Count > 0 && candidateSets[0].BlockNumber == frontSet.BlockNumber) { - candidateSet = frontSet; + candidateSets.Add(frontSet); } - else + else if (candidateSets.Count == 0 || candidateSets[0].BlockNumber > frontSet.BlockNumber) { - break; + candidateSets = new(); + candidateSets.Add(frontSet); } } - if (candidateSet is not null) + // TODO: Find a way to not have to re-add everything + foreach (BlockCommitSet blockCommitSet in toAddBack) + { + _commitSetQueue.Enqueue(blockCommitSet); + } + + foreach (BlockCommitSet blockCommitSet in candidateSets) + { + if (_logger.IsDebug) _logger.Debug($"Elevated pruning for candidate {blockCommitSet.BlockNumber}"); + Persist(blockCommitSet); + } + + if (candidateSets.Count > 0) { - if (_logger.IsDebug) _logger.Debug($"Elevated pruning for candidate {candidateSet.BlockNumber}"); - Persist(candidateSet); return true; } From 38d5aa589654c18338dcc8830591f284de8f6bb6 Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Tue, 19 Jul 2022 15:39:06 +0800 Subject: [PATCH 4/4] Fix if statement --- src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs b/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs index 3efe02f89c0..19d2aa0c8e8 100644 --- a/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs +++ b/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs @@ -427,7 +427,7 @@ private bool CanPruneCacheFurther() { candidateSets.Add(frontSet); } - else if (candidateSets.Count == 0 || candidateSets[0].BlockNumber > frontSet.BlockNumber) + else if (candidateSets.Count == 0 || frontSet.BlockNumber > candidateSets[0].BlockNumber) { candidateSets = new(); candidateSets.Add(frontSet);