Skip to content

Commit

Permalink
Fix double write during full pruning (#6415)
Browse files Browse the repository at this point in the history
* Fix double write during full pruning

* Missed a null condition
  • Loading branch information
asdacap authored Dec 24, 2023
1 parent 1c0ce55 commit 716a791
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ public CopyTreeVisitor(
}

public bool IsFullDbScan => true;

public ReadFlags ExtraReadFlag => ReadFlags.SkipDuplicateRead;

public bool ShouldVisit(Hash256 nextNode) => !_cancellationToken.IsCancellationRequested;

public void VisitTree(Hash256 rootHash, TrieVisitContext trieVisitContext)
Expand Down
3 changes: 3 additions & 0 deletions src/Nethermind/Nethermind.Core/IKeyValueStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
{
Expand Down
4 changes: 2 additions & 2 deletions src/Nethermind/Nethermind.Db/FullPruning/FullPruningDb.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public byte[]? this[ReadOnlySpan<byte> key]
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);
}
Expand All @@ -63,7 +63,7 @@ public byte[]? this[ReadOnlySpan<byte> key]
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);
}
Expand Down
3 changes: 3 additions & 0 deletions src/Nethermind/Nethermind.Trie/ITreeVisitor.cs
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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);
Expand Down
10 changes: 8 additions & 2 deletions src/Nethermind/Nethermind.Trie/PatriciaTree.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 716a791

Please sign in to comment.