Add support for Merge operator #42

Havret · 2025-12-04T08:55:06Z

No description provided.

Copilot

Pull request overview

This PR adds comprehensive support for RocksDB merge operators, enabling atomic read-modify-write operations without requiring separate read-before-write operations. This feature is particularly useful for counters, list appends, and other accumulative data structures.

Key Changes

Introduces new abstractions for merge operations (IMergeOperator, IMergeAccessor, MergeableRocksDbStore)
Implements two concrete merge operators: ListAppendMergeOperator (for simple list appends) and ListMergeOperator (for add/remove operations)
Upgrades RocksDB package from version 9.10.0 to 10.4.2, which includes API changes

Reviewed changes

Copilot reviewed 17 out of 18 changed files in this pull request and generated 14 comments.

Show a summary per file

File	Description
`src/RocksDb.Extensions/IMergeOperator.cs`	Core interface defining merge operator contract with FullMerge and PartialMerge methods
`src/RocksDb.Extensions/IMergeAccessor.cs`	Accessor interface extending IRocksDbAccessor with Merge operation support
`src/RocksDb.Extensions/MergeableRocksDbStore.cs`	Base class for stores that support merge operations, providing common CRUD operations
`src/RocksDb.Extensions/MergeAccessor.cs`	Implementation of merge accessor with proper serialization for key and operand
`src/RocksDb.Extensions/MergeOperatorConfig.cs`	Internal configuration class for merge operators per column family
`src/RocksDb.Extensions/MergeOperators/ListAppendMergeOperator.cs`	Merge operator for appending items to lists
`src/RocksDb.Extensions/MergeOperators/ListMergeOperator.cs`	Advanced merge operator supporting both add and remove operations on lists
`src/RocksDb.Extensions/MergeOperators/ListOperation.cs`	Data class representing list add/remove operations
`src/RocksDb.Extensions/MergeOperators/OperationType.cs`	Enum for operation types (Add/Remove)
`src/RocksDb.Extensions/ListOperationSerializer.cs`	Serializer for ListOperation with size-prefixed format
`src/RocksDb.Extensions/IRocksDbBuilder.cs`	Added AddMergeableStore method to builder interface
`src/RocksDb.Extensions/RocksDbBuilder.cs`	Implementation of mergeable store registration with merge operator configuration
`src/RocksDb.Extensions/RocksDbContext.cs`	Updated column family creation to support merge operators; changed API method name for RocksDB 10.x
`src/RocksDb.Extensions/RocksDbOptions.cs`	Added dictionary to track merge operators per column family
`src/RocksDb.Extensions/RocksDbAccessor.cs`	Changed field visibility modifiers and added Merge method implementation
`src/RocksDb.Extensions/RocksDbStore.cs`	New base class file providing standard store operations
`src/RocksDb.Extensions/RocksDb.Extensions.csproj`	Upgraded RocksDB package to version 10.4.2
`test/RocksDb.Extensions.Tests/MergeOperatorTests.cs`	Comprehensive test suite covering list append, add/remove operations, and edge cases

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-04T23:15:45Z

src/RocksDb.Extensions/RocksDbAccessor.cs

+    protected private readonly ISerializer<TValue> _valueSerializer;
+    protected private readonly RocksDbContext _rocksDbContext;


Invalid access modifier combination: protected private is not a valid C# modifier combination. This should be either protected, private, or private protected.

Suggested change

protected private readonly ISerializer<TValue> _valueSerializer;

protected private readonly RocksDbContext _rocksDbContext;

private protected readonly ISerializer<TValue> _valueSerializer;

private protected readonly RocksDbContext _rocksDbContext;

Copilot · 2025-12-04T23:15:45Z

src/RocksDb.Extensions/RocksDbAccessor.cs

+    protected private readonly ISerializer<TValue> _valueSerializer;
+    protected private readonly RocksDbContext _rocksDbContext;


Invalid access modifier combination: private protected should be private protected (in that order). The correct syntax is private protected, not protected private.

Suggested change

protected private readonly ISerializer<TValue> _valueSerializer;

protected private readonly RocksDbContext _rocksDbContext;

private protected readonly ISerializer<TValue> _valueSerializer;

private protected readonly RocksDbContext _rocksDbContext;

Copilot · 2025-12-04T23:15:46Z

src/RocksDb.Extensions/ListOperationSerializer.cs

+
+    public void WriteTo(ref ListOperation<T> value, IBufferWriter<byte> buffer)
+    {
+        throw new NotImplementedException();


The WriteTo(ref ListOperation<T> value, IBufferWriter<byte> buffer) method throws NotImplementedException, but this is a required method for the serializer interface. This will cause runtime failures when the buffer writer path is used instead of the span-based path. This method should be implemented to support variable-sized serialization.

Suggested change

throw new NotImplementedException();

// Write operation type (1 byte)

var opSpan = buffer.GetSpan(sizeof(byte));

opSpan[0] = (byte)value.Type;

buffer.Advance(sizeof(byte));

// Write count (4 bytes)

var countSpan = buffer.GetSpan(sizeof(int));

BitConverter.TryWriteBytes(countSpan, value.Items.Count);

buffer.Advance(sizeof(int));

// Write each item with size prefix and data

for (int i = 0; i < value.Items.Count; i++)

{

var item = value.Items[i];

if (_itemSerializer.TryCalculateSize(ref item, out var itemSize))

{

// Write size prefix (4 bytes)

var sizeSpan = buffer.GetSpan(sizeof(int));

BitConverter.TryWriteBytes(sizeSpan, itemSize);

buffer.Advance(sizeof(int));

// Write item data

var itemSpan = buffer.GetSpan(itemSize);

var tmpSpan = itemSpan.Slice(0, itemSize);

_itemSerializer.WriteTo(ref item, ref tmpSpan);

buffer.Advance(itemSize);

}

}

Copilot · 2025-12-04T23:15:46Z

src/RocksDb.Extensions/ListOperationSerializer.cs

+        // Write each item with size prefix
+        for (int i = 0; i < value.Items.Count; i++)
+        {
+            var item = value.Items[i];
+            if (_itemSerializer.TryCalculateSize(ref item, out var itemSize))
+            {
+                slice = span.Slice(offset, sizeof(int));
+                BitConverter.TryWriteBytes(slice, itemSize);
+                offset += sizeof(int);
+
+                slice = span.Slice(offset, itemSize);
+                _itemSerializer.WriteTo(ref item, ref slice);
+                offset += itemSize;
+            }
+        }


The WriteTo method doesn't handle the case where _itemSerializer.TryCalculateSize returns false. When this happens, the item cannot be serialized using the span-based approach, but the method silently skips these items, leading to data loss. The method should fall back to using a buffer writer for items that don't support fixed-size serialization.

src/RocksDb.Extensions/RocksDbContext.cs

Copilot · 2025-12-04T23:15:48Z

src/RocksDb.Extensions/RocksDbAccessor.cs

+    protected private readonly ISerializer<TValue> _valueSerializer;
+    protected private readonly RocksDbContext _rocksDbContext;


Invalid access modifier combination: protected private is not a valid C# modifier combination. This should be either protected, private, or private protected.

Suggested change

protected private readonly ISerializer<TValue> _valueSerializer;

protected private readonly RocksDbContext _rocksDbContext;

private protected readonly ISerializer<TValue> _valueSerializer;

private protected readonly RocksDbContext _rocksDbContext;

src/RocksDb.Extensions/MergeAccessor.cs

Copilot · 2025-12-04T23:15:49Z

src/RocksDb.Extensions/RocksDbAccessor.cs

+            : Span<byte>.Empty;
+
+        ReadOnlySpan<byte> keySpan = keyBuffer;
+        ArrayPoolBufferWriter<byte>? keyBufferWriter = null;


This variable is manually disposed in a finally block - consider a C# using statement as a preferable resource management technique.

Copilot · 2025-12-04T23:15:49Z

src/RocksDb.Extensions/RocksDbAccessor.cs

+
+
+        ReadOnlySpan<byte> valueSpan = valueBuffer;
+        ArrayPoolBufferWriter<byte>? valueBufferWriter = null;


This variable is manually disposed in a finally block - consider a C# using statement as a preferable resource management technique.

Havret · 2025-12-05T11:44:15Z

@copilot try to fix the build. it's passing on MacOS

Copilot · 2025-12-05T11:44:24Z

@Havret I've opened a new pull request, #43, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated 9 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/RocksDb.Extensions/RocksDbContext.cs

src/RocksDb.Extensions/RocksDbBuilder.cs

src/RocksDb.Extensions/MergeOperators/ListMergeOperator.cs

Copilot · 2025-12-11T16:03:12Z

src/RocksDb.Extensions/ListOperationSerializer.cs

+    public void WriteTo(ref ListOperation<T> value, ref Span<byte> span)
+    {
+        int offset = 0;
+
+        // Write operation type (1 byte)
+        span[offset] = (byte)value.Type;
+        offset += sizeof(byte);
+
+        // Write count
+        var slice = span.Slice(offset, sizeof(int));
+        BitConverter.TryWriteBytes(slice, value.Items.Count);
+        offset += sizeof(int);
+
+        // Write each item with size prefix
+        for (int i = 0; i < value.Items.Count; i++)
+        {
+            var item = value.Items[i];
+            if (_itemSerializer.TryCalculateSize(ref item, out var itemSize))
+            {
+                slice = span.Slice(offset, sizeof(int));
+                BitConverter.TryWriteBytes(slice, itemSize);
+                offset += sizeof(int);
+
+                slice = span.Slice(offset, itemSize);
+                _itemSerializer.WriteTo(ref item, ref slice);
+                offset += itemSize;
+            }
+        }
+    }


The WriteTo method only writes items when TryCalculateSize succeeds for each item, but there's no handling for the case when TryCalculateSize returns false. This creates an inconsistency with the TryCalculateSize method (which also has this issue) and could result in incomplete serialization where the count written doesn't match the actual number of items serialized.

Copilot · 2025-12-11T16:03:13Z

src/RocksDb.Extensions/MergeableRocksDbStore.cs

+/// public class CounterStore : MergeableRocksDbStore&lt;string, long, long&gt;
+/// {
+///     public CounterStore(IRocksDbAccessor&lt;string, long&gt; accessor, IMergeAccessor&lt;string, long&gt; mergeAccessor) 
+///         : base(accessor, mergeAccessor) { }
+///     
+///     public void Increment(string key, long delta = 1) => Merge(key, delta);
+/// }


The example in the documentation shows a CounterStore constructor that accepts two parameters (accessor and mergeAccessor), but based on the actual implementation pattern in the codebase (see test file), stores should only accept the mergeAccessor parameter. This inconsistency could confuse developers trying to implement their own mergeable stores.

src/RocksDb.Extensions/MergeOperators/ListMergeOperator.cs

src/RocksDb.Extensions/RocksDbBuilder.cs

src/RocksDb.Extensions/RocksDbOptions.cs

Copilot · 2025-12-11T16:03:14Z

src/RocksDb.Extensions/RocksDbAccessor.cs

            }

-            if (_checkIfExists && _rocksDbContext.Db.HasKey(keySpan, _columnFamily.Handle) == false)
+            if (_checkIfExists && RocksDbContext.Db.HasKey(keySpan, ColumnFamily.Handle) == false)


The expression 'A == false' can be simplified to '!A'.

Suggested change

if (_checkIfExists && RocksDbContext.Db.HasKey(keySpan, ColumnFamily.Handle) == false)

if (_checkIfExists && !RocksDbContext.Db.HasKey(keySpan, ColumnFamily.Handle))

Havret added 2 commits December 4, 2025 10:22

WIP

6f77e2b

wip

81f3746

Havret force-pushed the merge_support_wip branch from 08a2eb2 to 81f3746 Compare December 4, 2025 09:35

Havret added 7 commits December 4, 2025 13:07

wip

8563757

WIP

0ba1404

wip

36a7381

wip

1143ec4

wip

da93637

wip

95dbd3a

wip

3b1398f

Havret requested a review from Copilot December 4, 2025 23:10

Copilot started reviewing on behalf of Havret December 4, 2025 23:11 View session

Copilot finished reviewing on behalf of Havret December 4, 2025 23:13

Copilot AI reviewed Dec 4, 2025

View reviewed changes

Copilot AI mentioned this pull request Dec 5, 2025

Skip merge operator tests on Linux due to RocksDbSharp bug #43

Closed

Havret mentioned this pull request Dec 10, 2025

Fix: Linux Interop Issues for Custom Merge Operators curiosity-ai/rocksdb-sharp#66

Merged

wip

c985414

Havret force-pushed the merge_support_wip branch from 460c597 to c985414 Compare December 10, 2025 23:19

wip

63b97fc

Havret requested a review from Copilot December 11, 2025 15:55

Copilot started reviewing on behalf of Havret December 11, 2025 15:55 View session

Copilot AI reviewed Dec 11, 2025

View reviewed changes

Havret added 6 commits December 11, 2025 23:18

wip

bd32b5a

wip

d2670f5

wip

ee73d9f

wip

ae6036c

wip

4cc4e27

wip

0d6565c

Havret added 2 commits December 17, 2025 17:53

wip

47f24e1

wip

00fb780

Havret closed this Dec 17, 2025

Havret deleted the merge_support_wip branch December 17, 2025 23:31

		protected private readonly ISerializer<TValue> _valueSerializer;
		protected private readonly RocksDbContext _rocksDbContext;

-        throw new NotImplementedException();
+        // Write operation type (1 byte)
+        var opSpan = buffer.GetSpan(sizeof(byte));
+        opSpan[0] = (byte)value.Type;
+        buffer.Advance(sizeof(byte));
+        // Write count (4 bytes)
+        var countSpan = buffer.GetSpan(sizeof(int));
+        BitConverter.TryWriteBytes(countSpan, value.Items.Count);
+        buffer.Advance(sizeof(int));
+        // Write each item with size prefix and data
+        for (int i = 0; i < value.Items.Count; i++)
+        {
+            var item = value.Items[i];
+            if (_itemSerializer.TryCalculateSize(ref item, out var itemSize))
+            {
+                // Write size prefix (4 bytes)
+                var sizeSpan = buffer.GetSpan(sizeof(int));
+                BitConverter.TryWriteBytes(sizeSpan, itemSize);
+                buffer.Advance(sizeof(int));
+                // Write item data
+                var itemSpan = buffer.GetSpan(itemSize);
+                var tmpSpan = itemSpan.Slice(0, itemSize);
+                _itemSerializer.WriteTo(ref item, ref tmpSpan);
+                buffer.Advance(itemSize);
+            }
+        }



		ReadOnlySpan<byte> valueSpan = valueBuffer;
		ArrayPoolBufferWriter<byte>? valueBufferWriter = null;

	if (_checkIfExists && RocksDbContext.Db.HasKey(keySpan, ColumnFamily.Handle) == false)
	if (_checkIfExists && !RocksDbContext.Db.HasKey(keySpan, ColumnFamily.Handle))

Add support for Merge operator #42

Add support for Merge operator #42

Uh oh!

Conversation

Havret commented Dec 4, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Havret commented Dec 5, 2025

Uh oh!

Copilot AI commented Dec 5, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants