Skip to content
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

[Neo Store Exception] Add exception to Use after Commit operation to Snapshots and Cache. #3405

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions src/Neo/Persistence/DataCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ namespace Neo.Persistence
/// </summary>
public abstract class DataCache
{
protected bool isCommitted = false;

/// <summary>
/// Represents an entry in the cache.
/// </summary>
Expand Down Expand Up @@ -146,6 +148,8 @@ public virtual void Commit()
dictionary.Remove(key);
}
changeSet.Clear();

isCommitted = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this flag must be changed to false when write or delete, isn't it?

}

/// <summary>
Expand All @@ -154,6 +158,7 @@ public virtual void Commit()
/// <returns>The snapshot of this instance.</returns>
public DataCache CreateSnapshot()
{
if (isCommitted) throw new InvalidOperationException("A committed data cache cannot be cloned.");
return new ClonedCache(this);
}

Expand All @@ -163,6 +168,7 @@ public DataCache CreateSnapshot()
/// <param name="key">The key of the entry.</param>
public void Delete(StorageKey key)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed data cache.");
lock (dictionary)
{
if (dictionary.TryGetValue(key, out Trackable trackable))
Expand Down Expand Up @@ -207,6 +213,8 @@ public void Delete(StorageKey key)
/// <returns>The entries found with the desired prefix.</returns>
public IEnumerable<(StorageKey Key, StorageItem Value)> Find(byte[] key_prefix = null, SeekDirection direction = SeekDirection.Forward)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed data cache.");

var seek_prefix = key_prefix;
if (direction == SeekDirection.Backward)
{
Expand Down Expand Up @@ -235,6 +243,8 @@ public void Delete(StorageKey key)

private IEnumerable<(StorageKey Key, StorageItem Value)> FindInternal(byte[] key_prefix, byte[] seek_prefix, SeekDirection direction)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed data cache.");

foreach (var (key, value) in Seek(seek_prefix, direction))
if (key.ToArray().AsSpan().StartsWith(key_prefix))
yield return (key, value);
Expand All @@ -251,6 +261,8 @@ public void Delete(StorageKey key)
/// <returns>The entries found with the desired range.</returns>
public IEnumerable<(StorageKey Key, StorageItem Value)> FindRange(byte[] start, byte[] end, SeekDirection direction = SeekDirection.Forward)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed data cache.");

ByteArrayComparer comparer = direction == SeekDirection.Forward
? ByteArrayComparer.Default
: ByteArrayComparer.Reverse;
Expand All @@ -267,6 +279,8 @@ public void Delete(StorageKey key)
/// <returns>The change set.</returns>
public IEnumerable<Trackable> GetChangeSet()
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed data cache.");

lock (dictionary)
{
foreach (StorageKey key in changeSet)
Expand All @@ -281,6 +295,8 @@ public IEnumerable<Trackable> GetChangeSet()
/// <returns><see langword="true"/> if the cache contains an entry with the specified key; otherwise, <see langword="false"/>.</returns>
public bool Contains(StorageKey key)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed data cache.");

lock (dictionary)
{
if (dictionary.TryGetValue(key, out Trackable trackable))
Expand Down Expand Up @@ -311,6 +327,8 @@ public bool Contains(StorageKey key)
/// <returns>The cached data. Or <see langword="null"/> if it doesn't exist and the <paramref name="factory"/> is not provided.</returns>
public StorageItem GetAndChange(StorageKey key, Func<StorageItem> factory = null)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed data cache.");

lock (dictionary)
{
if (dictionary.TryGetValue(key, out Trackable trackable))
Expand Down Expand Up @@ -367,6 +385,8 @@ public StorageItem GetAndChange(StorageKey key, Func<StorageItem> factory = null
/// <returns>The cached data.</returns>
public StorageItem GetOrAdd(StorageKey key, Func<StorageItem> factory)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed data cache.");

lock (dictionary)
{
if (dictionary.TryGetValue(key, out Trackable trackable))
Expand Down Expand Up @@ -416,6 +436,8 @@ public StorageItem GetOrAdd(StorageKey key, Func<StorageItem> factory)
/// <returns>An enumerator containing all the entries after seeking.</returns>
public IEnumerable<(StorageKey Key, StorageItem Value)> Seek(byte[] keyOrPrefix = null, SeekDirection direction = SeekDirection.Forward)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed data cache.");

IEnumerable<(byte[], StorageKey, StorageItem)> cached;
HashSet<StorageKey> cachedKeySet;
ByteArrayComparer comparer = direction == SeekDirection.Forward ? ByteArrayComparer.Default : ByteArrayComparer.Reverse;
Expand Down Expand Up @@ -480,6 +502,8 @@ public StorageItem GetOrAdd(StorageKey key, Func<StorageItem> factory)
/// <returns>The cached data. Or <see langword="null"/> if it is neither in the cache nor in the underlying storage.</returns>
public StorageItem TryGet(StorageKey key)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed data cache.");

lock (dictionary)
{
if (dictionary.TryGetValue(key, out Trackable trackable))
Expand Down
13 changes: 13 additions & 0 deletions src/Neo/Persistence/MemorySnapshot.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
// modifications are permitted.

using Neo.IO;
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Collections.Immutable;
Expand All @@ -19,6 +20,7 @@ namespace Neo.Persistence
{
internal class MemorySnapshot : ISnapshot
{
private bool isCommitted = false;
private readonly ConcurrentDictionary<byte[], byte[]> innerData;
private readonly ImmutableDictionary<byte[], byte[]> immutableData;
private readonly ConcurrentDictionary<byte[], byte[]> writeBatch;
Expand All @@ -37,10 +39,13 @@ public void Commit()
innerData.TryRemove(pair.Key, out _);
else
innerData[pair.Key] = pair.Value;
isCommitted = true;
}

public void Delete(byte[] key)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed snapshot.");

writeBatch[key] = null;
}

Expand All @@ -50,11 +55,15 @@ public void Dispose()

public void Put(byte[] key, byte[] value)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed snapshot.");

writeBatch[key[..]] = value[..];
}

public IEnumerable<(byte[] Key, byte[] Value)> Seek(byte[] keyOrPrefix, SeekDirection direction = SeekDirection.Forward)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed snapshot.");

ByteArrayComparer comparer = direction == SeekDirection.Forward ? ByteArrayComparer.Default : ByteArrayComparer.Reverse;
IEnumerable<KeyValuePair<byte[], byte[]>> records = immutableData;
if (keyOrPrefix?.Length > 0)
Expand All @@ -65,12 +74,16 @@ public void Put(byte[] key, byte[] value)

public byte[] TryGet(byte[] key)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed snapshot.");

immutableData.TryGetValue(key, out byte[] value);
return value?[..];
}

public bool Contains(byte[] key)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed snapshot.");

return immutableData.ContainsKey(key);
}
}
Expand Down
16 changes: 16 additions & 0 deletions src/Neo/Persistence/SnapshotCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,15 @@ public SnapshotCache(IReadOnlyStore store)

protected override void AddInternal(StorageKey key, StorageItem value)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed data cache.");

snapshot?.Put(key.ToArray(), value.ToArray());
}

protected override void DeleteInternal(StorageKey key)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed data cache.");

snapshot?.Delete(key.ToArray());
}

Expand All @@ -53,35 +57,47 @@ public override void Commit()

protected override bool ContainsInternal(StorageKey key)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed data cache.");

return store.Contains(key.ToArray());
}

public void Dispose()
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed data cache.");

snapshot?.Dispose();
}

protected override StorageItem GetInternal(StorageKey key)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed data cache.");

byte[] value = store.TryGet(key.ToArray());
if (value == null) throw new KeyNotFoundException();
return new(value);
}

protected override IEnumerable<(StorageKey, StorageItem)> SeekInternal(byte[] keyOrPrefix, SeekDirection direction)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed data cache.");

return store.Seek(keyOrPrefix, direction).Select(p => (new StorageKey(p.Key), new StorageItem(p.Value)));
}

protected override StorageItem TryGetInternal(StorageKey key)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed data cache.");

byte[] value = store.TryGet(key.ToArray());
if (value == null) return null;
return new(value);
}

protected override void UpdateInternal(StorageKey key, StorageItem value)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed data cache.");

snapshot?.Put(key.ToArray(), value.ToArray());
}
}
Expand Down
9 changes: 9 additions & 0 deletions src/Plugins/LevelDBStore/Plugins/Storage/Snapshot.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

using Neo.IO.Data.LevelDB;
using Neo.Persistence;
using System;
using System.Collections.Generic;
using LSnapshot = Neo.IO.Data.LevelDB.Snapshot;

Expand All @@ -22,6 +23,7 @@ internal class Snapshot : ISnapshot
private readonly LSnapshot snapshot;
private readonly ReadOptions options;
private readonly WriteBatch batch;
private bool isCommitted = false;

public Snapshot(DB db)
{
Expand All @@ -34,10 +36,13 @@ public Snapshot(DB db)
public void Commit()
{
db.Write(WriteOptions.Default, batch);
isCommitted = true;
}

public void Delete(byte[] key)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed snapshot.");

batch.Delete(key);
}

Expand All @@ -48,21 +53,25 @@ public void Dispose()

public IEnumerable<(byte[] Key, byte[] Value)> Seek(byte[] prefix, SeekDirection direction = SeekDirection.Forward)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed snapshot.");
return db.Seek(options, prefix, direction, (k, v) => (k, v));
}

public void Put(byte[] key, byte[] value)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed snapshot.");
batch.Put(key, value);
}

public bool Contains(byte[] key)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed snapshot.");
return db.Contains(options, key);
}

public byte[] TryGet(byte[] key)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed snapshot.");
return db.Get(options, key);
}
}
Expand Down
12 changes: 12 additions & 0 deletions src/Plugins/RocksDBStore/Plugins/Storage/Snapshot.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ internal class Snapshot : ISnapshot
private readonly RocksDbSharp.Snapshot snapshot;
private readonly WriteBatch batch;
private readonly ReadOptions options;
private bool isCommitted = false;

public Snapshot(RocksDb db)
{
Expand All @@ -37,20 +38,27 @@ public Snapshot(RocksDb db)
public void Commit()
{
db.Write(batch, Options.WriteDefault);
isCommitted = true;
}

public void Delete(byte[] key)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed snapshot.");

batch.Delete(key);
}

public void Put(byte[] key, byte[] value)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed snapshot.");

batch.Put(key, value);
}

public IEnumerable<(byte[] Key, byte[] Value)> Seek(byte[] keyOrPrefix, SeekDirection direction)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed snapshot.");

if (keyOrPrefix == null) keyOrPrefix = Array.Empty<byte>();

using var it = db.NewIterator(readOptions: options);
Expand All @@ -65,11 +73,15 @@ public void Put(byte[] key, byte[] value)

public bool Contains(byte[] key)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed snapshot.");

return db.Get(key, Array.Empty<byte>(), 0, 0, readOptions: options) >= 0;
}

public byte[] TryGet(byte[] key)
{
if (isCommitted) throw new InvalidOperationException("Can not read/write a committed snapshot.");

return db.Get(key, readOptions: options);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,9 +170,9 @@ public void TestDeleteRemainCanResolve()
var snapshot2 = store.GetSnapshot();
var mpt2 = new Trie(snapshot2, mpt1.Root.Hash);
Assert.IsTrue(mpt2.Delete("ac00".HexToBytes()));
Assert.IsTrue(mpt2.Delete("ac10".HexToBytes()));
mpt2.Commit();
snapshot2.Commit();
Assert.IsTrue(mpt2.Delete("ac10".HexToBytes()));
}

[TestMethod]
Expand Down
Loading