LogRecord and SpanByte changes #1186

TedHartMS · 2025-05-06T20:05:22Z

This is an initial draft of a PR to convert Garnet to use a new ObjectAllocator with a revised on-disk format that will use only a single log file, change ISessionFunctions to be (Disk)LogRecord-based and propagate this to operations such as Compaction and Migration, remove TKey and TValue from TsavoriteKV, and many related refactors.

This PR will be long-lived as additional task PRs are merged into storage-v2 before it is ready for merge to main (see "Major Remaining Tasks" below). Currently this branch implements ONLY the in-memory portion; the IO work is not yet implemented.

Highlights:

ArgSlice and SpanByte have been merged to a PinnedSpanByte struct with much more limited use. SpanByte remains as a static class providing (ReadOnly)Span<byte> extensions.
All keys are now either PinnedSpanByte (mostly at and above the GarnetApi layer) or ReadOnlySpan<byte> at the StorageSession and below, including Tsavorite. There are no longer any byte[] keys. The TKey type argument is gone from Tsavorite and Garnet.
- Non-SpanByte KeyComparer implementations have been removed.
Values are either (ReadOnly)Span<byte> or objects implementing IHeapObject. TValue has been removed from TsavoriteKV.
"ref TKey", "ref TValue", and "ref RecordInfo" on ISessionFunctions methods are replaced by one of the LogRecord variants, where they are properties.
- See https://github.com/microsoft/garnet/tree/tedhar/object-allocator/website/docs/dev/tsavorite/logrecord.md for details of LogRecord and associated classes and structs.
ETag and Expiration are now properties on LogRecord rather than embedded within Value or IGarnetObject. Internally, they are after the Value and *LogRecord manages shifting as Value shrinks and grows; all "record extra length" management is done by LogRecord and is opaque to ISessionFunctions.
GenericAllocator has been replaced by ObjectAllocator; for details, see https://github.com/microsoft/garnet/tree/tedhar/object-allocator/website/docs/dev/tsavorite/logrecord.md. Currently only in-memory operations are implemented.
For ObjectAllocator, keys and string values that are past a configurable size can "overflow" to out-of-line byte[], keeping the record size manageable. Tsavorite implements this threshold automatically, and LogRecord manages it internally when returning the Key or ValueSpan.
SpanByteScanIterator and GenericScanIterator have been unified into RecordScanIterator
Some names have been changed for clarity:
- "Lockable*" and "Manual*" have become "Transactional*"
- "Transient" locking has become "Ephemeral"

Major remaining tasks (this will be updated as they are implemented):

Complete ObjectAllocator single-LogFile IO.
Change object-size tracking from page-level granularity to record-level.
Performance evaluation and tuning
Fix RENAME and any other remaining areas where the Expiration is not propagated as before, as it is no longer part of Value (including IGarnetObject)
Clean up error handling related to the VarLenInputMethods functions (make them consistent with returns from RMWMethods, etc.)
Implement JsonObject MemorySize/DiskSize
Finish converting and re-enabling all Tsavorite Unit Tests
Revise the current Garnet.common to be Garnet.core, then re-create Garnet.common as a common layer between Tsavorite and the Garnet processing layer, containing PinnedSpanByte, epochs, Utility functions (some of which are currently duplicated), and so on.
Investigate the performance impact of moving further along the (ReadOnly)Span<byte> path by replacing byte* usage (such as was done for Utility.HashBytes).

- Move CountdownWrapper to its own file - Rename Lockable* to Transactional* - Rename Transient* to Ephemeral*

… LogRecord

Other work to get CreateNewRecordRMW clean

…ns first pass done)

…ync APIs) from Tsavorite

kevin-montrose

I've looked through the first 150 files of this - up to libs/server/Transaction/TransactionManager.cs.

I'll continue with my review tomorrow.

libs/cluster/Session/RespClusterMigrateCommands.cs

libs/server/Resp/KeyAdminCommands.cs

libs/server/Resp/RespServerSession.cs

libs/server/Storage/Functions/MainStore/PrivateMethods.cs

libs/server/Storage/Functions/MainStore/RMWMethods.cs

kevin-montrose

Another 150 files reviewed, up through libs/storage/Tsavorite/cs/src/core/Index/Tsavorite/Implementation/UpsertValueSelector.cs. I'll wrap up tomorrow, time allowing.

libs/server/Transaction/TxnKeyEntry.cs

libs/server/Transaction/TxnWatchedKeysContainer.cs

libs/storage/Tsavorite/cs/src/core/Allocator/DiskLogRecord.cs

...torage/Tsavorite/cs/src/core/Index/Tsavorite/Implementation/Revivification/FreeRecordPool.cs

kevin-montrose

Finished review.

Mostly nits, some far this is looking very promising.

libs/storage/Tsavorite/cs/src/core/TsavoriteLog/TsavoriteLogSettings.cs

libs/storage/Tsavorite/cs/src/core/Utilities/MultiLevelPageArray.cs

libs/storage/Tsavorite/cs/src/core/Utilities/SimpleConcurrentStack.cs

libs/storage/Tsavorite/cs/src/core/Utilities/Utility.cs

website/docs/dev/tsavorite/logrecord.md

website/docs/dev/tsavorite/object-allocator.md

… for SpanByteAllocator. The AddressType change is a breaking on-disk format change: it shuffles bits around in RecordInfo to add an additional bit adjacent to the old ReadCache bit to mark an address as: - 00: Reserved - 11: ReadCache - 10: InMemory portion of the main log - 01: On-Disk

…hem "in" rather than "ref"

PaulusParssinen · 2025-06-02T14:11:49Z

I'll be able to have a look tomorrow at this beast! I have been glancing through it and in general I like the direction 👍

update: this is gonna take a while 😅 Half-way through reading the changes, I'll leave comments/questions for later once I understand the changes a bit better.

PaulusParssinen

Review marathon mostly over (I skipped test diffs) and I like the changes a lot! A lot of nice consolidation and reducing the complexity of certain parts. Thanks for the useful documentation too, but I won't pretend that I understand all of the new Log changes thoroughly yet.

This PR also seems to do a lot of nice work that is useful if we want to switch (de)serialization logic from Stream (BinaryReader/BinaryWriter) to span based approach in future (I have some local attempts at this and it looks more approachable after this PRs changes).

PaulusParssinen · 2025-06-11T20:44:57Z

libs/storage/Tsavorite/cs/src/core/Allocator/DiskLogRecord.cs

+            var result = 0;
+            do
+            {
+                num >>= 8;
+                result++;
+            } while (num > 0);
+            return result;


I think we can turn this branchless :) (given we don't use this for negative values here)

Suggested change

var result = 0;

do

{

num >>= 8;

result++;

} while (num > 0);

return result;

var bitLength = (sizeof(ulong) * 8) - BitOperations.LeadingZeroCount((ulong)(x | 1));

return (bitLength + 7) / 8;

PaulusParssinen · 2025-06-11T21:05:22Z

libs/storage/Tsavorite/cs/src/core/Allocator/ObjectAllocatorImpl.cs

-                throw new TsavoriteException("SpanByte Keys cannot be mixed with object Values");
-            if (typeof(TValue) == typeof(SpanByte))
-                throw new TsavoriteException("SpanByte Values cannot be mixed with object Keys");
+            freePagePool = new OverflowPool<PageUnit<ObjectPage>>(4, static p => { });


If I'm reading this right (that it is same logic as in my experiment PR), we need to free these overflowing pages.

We could also report these unmanaged allocations for GC if we want to be nice.

PaulusParssinen · 2025-06-11T21:10:29Z

libs/storage/Tsavorite/cs/src/core/Allocator/SpanByteAllocatorImpl.cs


        public SpanByteAllocatorImpl(AllocatorSettings settings, TStoreFunctions storeFunctions, Func<object, SpanByteAllocator<TStoreFunctions>> wrapperCreator)
            : base(settings.LogSettings, storeFunctions, wrapperCreator, settings.evictCallback, settings.epoch, settings.flushCallback, settings.logger)
        {
-            overflowPagePool = new OverflowPool<PageUnit>(4, p => { });
+            freePagePool = new OverflowPool<PageUnit<Empty>>(4, p => { });


ditto, we should free the unmanaged pool pages.

PaulusParssinen · 2025-06-11T21:18:02Z

libs/storage/Tsavorite/cs/src/core/Index/Common/KVSettings.cs

+        public int MaxInlineKeySize = 1 << LogSettings.kDefaultMaxInlineKeySizeBits;
+
+        /// <summary>
+        /// Maximum size of a valuie stored inline in the in-memory portion of the main log for <see cref="SpanByteAllocator{TStoreFunctions}"/>.


Suggested change

/// Maximum size of a valuie stored inline in the in-memory portion of the main log for <see cref="SpanByteAllocator{TStoreFunctions}"/>.

/// Maximum size of a value stored inline in the in-memory portion of the main log for <see cref="SpanByteAllocator{TStoreFunctions}"/>.

PaulusParssinen · 2025-06-11T21:48:33Z

libs/storage/Tsavorite/cs/src/core/VarLen/PinnedSpanByte.cs

+namespace Tsavorite.core
+{
+    /// <summary>
+    /// Represents contiguous region of arbitrary _pinned_ memory.


This emphasis that I added some time ago with the original comment is now much less useful when the type name contains the information 😅

Suggested change

/// Represents contiguous region of arbitrary _pinned_ memory.

/// Represents contiguous region of arbitrary pinned memory.

TedHartMS added 30 commits November 26, 2024 15:39

Refactors prior to storage-v2:

0ca8493

- Move CountdownWrapper to its own file - Rename Lockable* to Transactional* - Rename Transient* to Ephemeral*

WIP; first cut at LogRecord etc. implementations

421d7a0

WIP: more LogRecord

7542d8a

More *LogRecord, and DiskRecord

0797412

WIP: Convert allocators to new LogRecord and DiskRecord

f94a769

WIP: incorporate LogRecord into ISessionFunctions

53f4941

More logRecord WIP

ca4b399

More WIP on MainSessionFunctions RMWMethods (and PrivateMethods) with…

3b754e6

… LogRecord

WIP LogRecord (MainStore RMWMethods)

49e9769

WIP: Consolidate to a single LogRecord; convert InternalRMW

a92c2bb

TKey removal

1ce5ee6

A few more TKey removals

95205ce

WIP more logRecord

493302d

Remove pendingConterxt.recordInfo as it is no longer in RecordMetadata

a470009

Other work to get CreateNewRecordRMW clean

WIP; Internal(RUMD) converted.

60cba5a

added PendingContext implementation of ISourceLogRecord

0c587fc

Finish ContinuePending

81bd194

Iterator changes - consolidate to RecordScanIterator

1de5c11

Mostly IO work

cf491bc

WIP: Reintroduce TValue through ObjectAllocator, LogRecord, etc.

5d925ff

WIP Convert Garnet to LogRecord and ObjectAllocator (MainStoreFunctio…

06464cb

…ns first pass done)

340c1a7

WIP converting Garnet to non-ref SpanByte-only keys

2063e47

More WIP on Garnet migration to LogRecord

7e44fa7

WIP Tsavorite LogRecord odds and ends

d2d32c5

Rename TsavoriteLog to TsavoriteAof

5f0639d

More processing-layer LogRecord etc. fixes

575fd02

fix iterator as ISourceLogRecord

1f878ab

More Expiration etc. cleanup in processing layer

785cc31

WIP BlockAllocate work

42dd7ab

TedHartMS added 17 commits April 27, 2025 21:00

SImple Migrate tests now run

12c797c

Simple diskless replica runs

77b6105

Fixes for in-memory tests

90793d2

Add UpsertValueSelector

0dd2039

Remove PendingContext.IsAsync (a remnant of earlier, since-removed as…

c43401c

…ync APIs) from Tsavorite

merge 'main'

3eb46a0

Merge 'tedhar/storage-v2'

eb20108

Cleanup Read initialization of PendingContext

b5205ec

Fix record alignment in YCSB test

c415865

tune HashBytes

00b050d

Fix initial inserting in YCSB

4e043c1

Update LogRecord.md and some cleanup

cad3254

Restore TsavoriteAof naming back to TsavoriteLog

b4a942f

Merge remote-tracking branch 'origin/main' into tedhar/storage-v2

c7666db

merge 'tedhar/storage-v2'

48a31d4

merge 'main'

83781a1

merge 'main'

3f06a7a

badrishc mentioned this pull request May 19, 2025

Add more test coverage for Tsavorite primitives #1121

Draft

kevin-montrose reviewed May 21, 2025

View reviewed changes

kevin-montrose reviewed May 22, 2025

View reviewed changes

kevin-montrose reviewed May 23, 2025

View reviewed changes

TedHartMS added 7 commits May 27, 2025 09:20

Merge 'main'; also fix a couple DbScan tests I missed with AddressType

539db55

Fix a few more cursor tests (int => long)

36633c7

Fixes for review comments (except for "ref RecordSizeInfo")

f31ce8a

TSourceLogRecord and RecordSizeInfo parameters are readonly so make t…

11fdf3d

…hem "in" rather than "ref"

Fix some missing ref => in for TSourceLogRecord

ba3eed2

Merge remote-tracking branch 'origin/main' into tedhar/storage-v2

62f4a73

PaulusParssinen reviewed Jun 11, 2025

View reviewed changes

	/// Maximum size of a valuie stored inline in the in-memory portion of the main log for <see cref="SpanByteAllocator{TStoreFunctions}"/>.
	/// Maximum size of a value stored inline in the in-memory portion of the main log for <see cref="SpanByteAllocator{TStoreFunctions}"/>.

	/// Represents contiguous region of arbitrary _pinned_ memory.
	/// Represents contiguous region of arbitrary pinned memory.

LogRecord and SpanByte changes #1186

Are you sure you want to change the base?

LogRecord and SpanByte changes #1186

Uh oh!

Conversation

TedHartMS commented May 6, 2025

Uh oh!

kevin-montrose left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kevin-montrose left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kevin-montrose left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PaulusParssinen commented Jun 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PaulusParssinen left a comment

Choose a reason for hiding this comment

Uh oh!

PaulusParssinen Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PaulusParssinen Jun 11, 2025

Choose a reason for hiding this comment

Uh oh!

PaulusParssinen Jun 11, 2025

Choose a reason for hiding this comment

Uh oh!

PaulusParssinen Jun 11, 2025

Choose a reason for hiding this comment

Uh oh!

PaulusParssinen Jun 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

PaulusParssinen commented Jun 2, 2025 •

edited

Loading

PaulusParssinen Jun 11, 2025 •

edited

Loading