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

[Core Plugin] adopt more change from pr 3414 #3591

Merged
merged 8 commits into from
Nov 30, 2024
Merged
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
23 changes: 20 additions & 3 deletions src/Plugins/LevelDBStore/IO/Data/LevelDB/DB.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,13 @@
// Redistribution and use in source and binary forms with or without
// modifications are permitted.

#nullable enable

using System;
using System.Collections.Generic;
using System.IO;

namespace Neo.IO.Data.LevelDB
namespace Neo.IO.Storage.LevelDB
{
/// <summary>
/// A DB is a persistent ordered map from keys to values.
Expand Down Expand Up @@ -90,9 +93,9 @@ public static DB Open(string name)

public static DB Open(string name, Options options)
{
var Handle = Native.leveldb_open(options.Handle, Path.GetFullPath(name), out var error);
var handle = Native.leveldb_open(options.Handle, Path.GetFullPath(name), out var error);
NativeHelper.CheckError(error);
return new DB(Handle);
return new DB(handle);
}

/// <summary>
Expand Down Expand Up @@ -122,5 +125,19 @@ public void Write(WriteOptions options, WriteBatch write_batch)
Native.leveldb_write(Handle, options.Handle, write_batch.Handle, out var error);
NativeHelper.CheckError(error);
}

public IEnumerable<KeyValuePair<byte[], byte[]>> GetAll(Snapshot? snapshot = null)
{
using var options = new ReadOptions();
if (snapshot != null) options.Snapshot = snapshot;

using var iterator = NewIterator(options);
iterator.SeekToFirst();
shargon marked this conversation as resolved.
Show resolved Hide resolved
while (iterator.Valid())
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this while, and below is for. Might it be better to keep a consistent style?

Copy link
Member

Choose a reason for hiding this comment

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

He copied mine. but for would be better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will be fixed in another pr.

{
yield return new KeyValuePair<byte[], byte[]>(iterator.Key(), iterator.Value());
Copy link
Member

Choose a reason for hiding this comment

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

This behaviour maybe it's different than the previous one, because we are yield during a snapshot

Copy link
Member

@shargon shargon Nov 21, 2024

Choose a reason for hiding this comment

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

I prefer to receive the snapshot from an argument, maybe we don't wan't to use it, and avoid IEnumerable<KeyValuePair<byte[], byte[]>>

Copy link
Member

Choose a reason for hiding this comment

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

It's helpful for debugging. Better than being in the dark.

Copy link
Contributor Author

@Jim8y Jim8y Nov 22, 2024

Choose a reason for hiding this comment

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

It's helpful for debugging. Better than being in the dark.

Will this cause state difference or behavior difference? If the result is the same, i think it should be fine, this plugin can only be used here for the core.

Copy link
Member

Choose a reason for hiding this comment

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

We should receive the snapshot or null or we can have a big mistakes

Copy link
Member

@cschuchardt88 cschuchardt88 Nov 22, 2024

Choose a reason for hiding this comment

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

Will this cause state difference or behavior difference? If the result is the same, i think it should be fine, this plugin can only be used here for the core.

We should receive the snapshot or null or we can have a big mistakes

All its doing is displaying the whole database, has nothing to do with state or snapshots. The reason it takes a snapshot, so the data doesn't get changed (When viewing it).

Copy link
Member

Choose a reason for hiding this comment

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

But imagine that we use this method in a loop, when we change the data, we would not notice this error

Copy link
Member

Choose a reason for hiding this comment

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

than don't use snapshot just use

using var iterator = NewIterator(ReadOptions.Default);

Copy link
Member

Choose a reason for hiding this comment

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

Easy to forget

Copy link
Member

Choose a reason for hiding this comment

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

We can discuss in a different PR if we create the IEnumerable<KeyValuePair<byte[], byte[]>> , this one is ready to merge

iterator.Next();
}
}
}
}
19 changes: 4 additions & 15 deletions src/Plugins/LevelDBStore/IO/Data/LevelDB/Helper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,17 @@
using System.Collections.Generic;
using System.Runtime.InteropServices;

namespace Neo.IO.Data.LevelDB
namespace Neo.IO.Storage.LevelDB
{
public static class Helper
{
public static IEnumerable<T> Seek<T>(this DB db, ReadOptions options, byte[] prefix, SeekDirection direction, Func<byte[], byte[], T> resultSelector)
public static IEnumerable<(byte[], byte[])> Seek(this DB db, ReadOptions options, byte[] prefix, SeekDirection direction)
{
using Iterator it = db.NewIterator(options);
if (direction == SeekDirection.Forward)
{
for (it.Seek(prefix); it.Valid(); it.Next())
yield return resultSelector(it.Key(), it.Value());
yield return new(it.Key(), it.Value());
}
else
{
Expand All @@ -37,18 +37,7 @@ public static IEnumerable<T> Seek<T>(this DB db, ReadOptions options, byte[] pre
it.Prev();

for (; it.Valid(); it.Prev())
yield return resultSelector(it.Key(), it.Value());
}
}

public static IEnumerable<T> FindRange<T>(this DB db, ReadOptions options, byte[] startKey, byte[] endKey, Func<byte[], byte[], T> resultSelector)
{
using Iterator it = db.NewIterator(options);
for (it.Seek(startKey); it.Valid(); it.Next())
{
byte[] key = it.Key();
if (key.AsSpan().SequenceCompareTo(endKey) > 0) break;
yield return resultSelector(key, it.Value());
yield return new(it.Key(), it.Value());
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/Plugins/LevelDBStore/IO/Data/LevelDB/Iterator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

using System;

namespace Neo.IO.Data.LevelDB
namespace Neo.IO.Storage.LevelDB
{
/// <summary>
/// An iterator yields a sequence of key/value pairs from a database.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

using System.Data.Common;

namespace Neo.IO.Data.LevelDB
namespace Neo.IO.Storage.LevelDB
{
public class LevelDBException : DbException
{
Expand Down
2 changes: 1 addition & 1 deletion src/Plugins/LevelDBStore/IO/Data/LevelDB/LevelDBHandle.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

using System;

namespace Neo.IO.Data.LevelDB
namespace Neo.IO.Storage.LevelDB
{
/// <summary>
/// Base class for all LevelDB objects
Expand Down
142 changes: 71 additions & 71 deletions src/Plugins/LevelDBStore/IO/Data/LevelDB/Native.cs

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/Plugins/LevelDBStore/IO/Data/LevelDB/Options.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

using System;

namespace Neo.IO.Data.LevelDB
namespace Neo.IO.Storage.LevelDB
{
/// <summary>
/// Options to control the behavior of a database (passed to Open)
Expand Down
2 changes: 1 addition & 1 deletion src/Plugins/LevelDBStore/IO/Data/LevelDB/ReadOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
// Redistribution and use in source and binary forms with or without
// modifications are permitted.

namespace Neo.IO.Data.LevelDB
namespace Neo.IO.Storage.LevelDB
{
/// <summary>
/// Options that control read operations.
Expand Down
2 changes: 1 addition & 1 deletion src/Plugins/LevelDBStore/IO/Data/LevelDB/Snapshot.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

using System;

namespace Neo.IO.Data.LevelDB
namespace Neo.IO.Storage.LevelDB
{
/// <summary>
/// A Snapshot is an immutable object and can therefore be safely
Expand Down
2 changes: 1 addition & 1 deletion src/Plugins/LevelDBStore/IO/Data/LevelDB/WriteBatch.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

using System;

namespace Neo.IO.Data.LevelDB
namespace Neo.IO.Storage.LevelDB
{
/// <summary>
/// WriteBatch holds a collection of updates to apply atomically to a DB.
Expand Down
2 changes: 1 addition & 1 deletion src/Plugins/LevelDBStore/IO/Data/LevelDB/WriteOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
// Redistribution and use in source and binary forms with or without
// modifications are permitted.

namespace Neo.IO.Data.LevelDB
namespace Neo.IO.Storage.LevelDB
{
/// <summary>
/// Options that control write operations.
Expand Down
2 changes: 1 addition & 1 deletion src/Plugins/LevelDBStore/Plugins/Storage/LevelDBStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
// Redistribution and use in source and binary forms with or without
// modifications are permitted.

using Neo.IO.Data.LevelDB;
using Neo.IO.Storage.LevelDB;
using Neo.Persistence;
using System;
using System.Linq;
Expand Down
7 changes: 4 additions & 3 deletions src/Plugins/LevelDBStore/Plugins/Storage/Snapshot.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@
// Redistribution and use in source and binary forms with or without
// modifications are permitted.

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

namespace Neo.Plugins.Storage
{
Expand Down Expand Up @@ -44,11 +44,12 @@ public void Delete(byte[] key)
public void Dispose()
{
_snapshot.Dispose();
_options.Dispose();
Copy link
Member

Choose a reason for hiding this comment

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

This memory leak is present in master

}

public IEnumerable<(byte[] Key, byte[] Value)> Seek(byte[] prefix, SeekDirection direction = SeekDirection.Forward)
{
return _db.Seek(_options, prefix, direction, (k, v) => (k, v));
return _db.Seek(_options, prefix, direction);
}

public void Put(byte[] key, byte[] value)
Expand Down
14 changes: 10 additions & 4 deletions src/Plugins/LevelDBStore/Plugins/Storage/Store.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
// Redistribution and use in source and binary forms with or without
// modifications are permitted.

using Neo.IO.Data.LevelDB;
using Neo.IO.Storage.LevelDB;
using Neo.Persistence;
using System.Collections.Generic;

Expand All @@ -18,18 +18,24 @@ namespace Neo.Plugins.Storage
internal class Store : IStore
{
private readonly DB _db;
private readonly Options _options;

public Store(string path)
{
_db = DB.Open(path, new Options { CreateIfMissing = true, FilterPolicy = Native.leveldb_filterpolicy_create_bloom(15) });
_options = new Options { CreateIfMissing = true, FilterPolicy = Native.leveldb_filterpolicy_create_bloom(15) };
_db = DB.Open(path, _options);
}

public void Delete(byte[] key)
{
_db.Delete(WriteOptions.Default, key);
}

public void Dispose() => _db.Dispose();
public void Dispose()
{
_db.Dispose();
_options.Dispose();
Copy link
Member

Choose a reason for hiding this comment

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

This one also, but one per node

}

public ISnapshot GetSnapshot() =>
new Snapshot(_db);
Expand All @@ -53,6 +59,6 @@ public bool TryGet(byte[] key, out byte[] value)
}

public IEnumerable<(byte[], byte[])> Seek(byte[] prefix, SeekDirection direction = SeekDirection.Forward) =>
_db.Seek(ReadOptions.Default, prefix, direction, (k, v) => (k, v));
_db.Seek(ReadOptions.Default, prefix, direction);
}
}
Loading