Fix double write during full pruning (#6415)

* Fix double write during full pruning * Missed a null condition
NethermindEth · Dec 24, 2023 · 716a791 · 716a791
1 parent 1c0ce55
commit 716a791
Show file tree

Hide file tree

Showing 7 changed files with 49 additions and 4 deletions.
diff --git a/src/Nethermind/Nethermind.Blockchain.Test/FullPruning/CopyTreeVisitorTests.cs b/src/Nethermind/Nethermind.Blockchain.Test/FullPruning/CopyTreeVisitorTests.cs
@@ -53,6 +53,7 @@ public void copies_state_between_dbs(int fullPruningMemoryBudgetMb, int maxDegre
             clonedDb.Values.Should().BeEquivalentTo(values);
 
             clonedDb.KeyWasWrittenWithFlags(keys[0], WriteFlags.LowPriority);
+            trieDb.KeyWasReadWithFlags(keys[0], ReadFlags.SkipDuplicateRead | ReadFlags.HintCacheMiss);
         }
 
         [Test, Timeout(Timeout.MaxTestTime)]

diff --git a/src/Nethermind/Nethermind.Blockchain/FullPruning/CopyTreeVisitor.cs b/src/Nethermind/Nethermind.Blockchain/FullPruning/CopyTreeVisitor.cs
@@ -43,6 +43,9 @@ public class CopyTreeVisitor : ITreeVisitor, IDisposable
         }
 
         public bool IsFullDbScan => true;
+
+        public ReadFlags ExtraReadFlag => ReadFlags.SkipDuplicateRead;
+
         public bool ShouldVisit(Hash256 nextNode) => !_cancellationToken.IsCancellationRequested;
 
         public void VisitTree(Hash256 rootHash, TrieVisitContext trieVisitContext)

diff --git a/src/Nethermind/Nethermind.Core/IKeyValueStore.cs b/src/Nethermind/Nethermind.Core/IKeyValueStore.cs
@@ -70,6 +70,9 @@ public enum ReadFlags
 
         // Hint that the workload is likely to need the next value in the sequence and should prefetch it.
         HintReadAhead = 2,
+
+        // Used for full pruning db to skip duplicate read
+        SkipDuplicateRead = 4,
     }
 
     [Flags]

diff --git a/src/Nethermind/Nethermind.Db.Test/FullPruning/FullPruningDbTests.cs b/src/Nethermind/Nethermind.Db.Test/FullPruning/FullPruningDbTests.cs
@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: LGPL-3.0-only
 
 using FluentAssertions;
+using Nethermind.Core;
 using Nethermind.Db.FullPruning;
 using NSubstitute;
 using NUnit.Framework;
@@ -65,6 +66,34 @@ public void during_pruning_writes_to_both_dbs()
             test.CurrentMirrorDb[key].Should().BeEquivalentTo(value);
         }
 
+        [Test]
+        public void during_pruning_duplicate_on_read()
+        {
+            TestContext test = new();
+            byte[] key = { 1, 2 };
+            byte[] value = { 5, 6 };
+            test.FullPruningDb[key] = value;
+
+            test.FullPruningDb.TryStartPruning(out IPruningContext _);
+
+            test.FullPruningDb.Get(key);
+            test.CurrentMirrorDb[key].Should().BeEquivalentTo(value);
+        }
+
+        [Test]
+        public void during_pruning_dont_duplicate_read_with_skip_duplicate_read()
+        {
+            TestContext test = new();
+            byte[] key = { 1, 2 };
+            byte[] value = { 5, 6 };
+            test.FullPruningDb[key] = value;
+
+            test.FullPruningDb.TryStartPruning(out IPruningContext _);
+
+            test.FullPruningDb.Get(key, ReadFlags.SkipDuplicateRead);
+            test.CurrentMirrorDb[key].Should().BeNull();
+        }
+
         [Test]
         public void increments_metrics_on_write_to_mirrored_db()
         {

diff --git a/src/Nethermind/Nethermind.Db/FullPruning/FullPruningDb.cs b/src/Nethermind/Nethermind.Db/FullPruning/FullPruningDb.cs
@@ -52,7 +52,7 @@ public FullPruningDb(RocksDbSettings settings, IRocksDbFactory dbFactory, Action
         public byte[]? Get(ReadOnlySpan<byte> key, ReadFlags flags = ReadFlags.None)
         {
             byte[]? value = _currentDb.Get(key, flags); // we are reading from the main DB
-            if (_pruningContext?.DuplicateReads == true)
+            if (value != null && _pruningContext?.DuplicateReads == true && (flags & ReadFlags.SkipDuplicateRead) == 0)
             {
                 Duplicate(_pruningContext.CloningDb, key, value, WriteFlags.None);
             }
@@ -63,7 +63,7 @@ public FullPruningDb(RocksDbSettings settings, IRocksDbFactory dbFactory, Action
         public Span<byte> GetSpan(ReadOnlySpan<byte> key, ReadFlags flags = ReadFlags.None)
         {
             Span<byte> value = _currentDb.GetSpan(key, flags); // we are reading from the main DB
-            if (!value.IsNull() && _pruningContext?.DuplicateReads == true)
+            if (!value.IsNull() && _pruningContext?.DuplicateReads == true && (flags & ReadFlags.SkipDuplicateRead) == 0)
             {
                 Duplicate(_pruningContext.CloningDb, key, value, WriteFlags.None);
             }

diff --git a/src/Nethermind/Nethermind.Trie/ITreeVisitor.cs b/src/Nethermind/Nethermind.Trie/ITreeVisitor.cs
@@ -1,6 +1,7 @@
 // SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited
 // SPDX-License-Identifier: LGPL-3.0-only
 
+using Nethermind.Core;
 using Nethermind.Core.Crypto;
 
 namespace Nethermind.Trie
@@ -12,6 +13,8 @@ public interface ITreeVisitor
         /// </summary>
         public bool IsFullDbScan { get; }
 
+        ReadFlags ExtraReadFlag => ReadFlags.None;
+
         bool ShouldVisit(Hash256 nextNode);
 
         void VisitTree(Hash256 rootHash, TrieVisitContext trieVisitContext);

diff --git a/src/Nethermind/Nethermind.Trie/PatriciaTree.cs b/src/Nethermind/Nethermind.Trie/PatriciaTree.cs
@@ -1088,10 +1088,16 @@ public void Accept(ITreeVisitor visitor, Hash256 rootHash, VisitingOptions? visi
                 }
             }
 
-            ITrieNodeResolver resolver = TrieStore;
+            ReadFlags flags = visitor.ExtraReadFlag;
             if (visitor.IsFullDbScan)
             {
-                resolver = new TrieNodeResolverWithReadFlags(TrieStore, ReadFlags.HintCacheMiss);
+                flags |= ReadFlags.HintCacheMiss;
+            }
+
+            ITrieNodeResolver resolver = TrieStore;
+            if (flags != ReadFlags.None)
+            {
+                resolver = new TrieNodeResolverWithReadFlags(TrieStore, flags);
             }
 
             visitor.VisitTree(rootHash, trieVisitContext);