Skip to content

Add object-log chunking and no-copy flushing - #2081

Draft
Ted Hart (TedHartMS) wants to merge 118 commits into
mainfrom
tedhar/aof-chunk
Draft

Ted Hart (TedHartMS) wants to merge 118 commits into
mainfrom
tedhar/aof-chunk

Conversation

@TedHartMS

@TedHartMS Ted Hart (TedHartMS) commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds chunked object-log serialization for large objects and overflow values/keys, including bounded read-ahead, DMA, and segment-boundary handling.
  • Improves flush efficiency with no-copy Snapshot, ReadOnly, and current-format recovery writes, shorter epoch holds, and coordinated Snapshot/ReadOnly page ownership.
  • Hardens recovery with exact Snapshot/main-log boundary merging, low-memory eviction support, fuzzy-record rejection, durable-tail bounds, and write-error propagation.
  • Adds an offline --upgrade mode that up-converts a downlevel (v7) object log to the current chunk-framed format, for FoldOver and Snapshot checkpoints, with or without the append-only file, standalone or on a cluster node. Conversion writes to a separate upgrade object-log device and swaps it in only after a checkpoint has made every converted position durable.
  • Carries pageSize, segmentSize and objectLogSegmentSize in HybridLogRecoveryInfo and verifies them against the store's configuration on recovery.
  • Expands byte-exact boundary, recovery, failure, and second-recovery coverage.

To be done

  • Code review: Complete review of existing code changes.
  • Complete OnDispose copy-off/deferred-layout work required by the no-copy flush contract. The live flush-thread defect is fixed (object-log flush captures a record's out-of-line components before consulting its state, and rechecks the live RecordInfo afterwards); what remains is deferring physical cleanup for records inside an in-flight flush window.
  • Add test-only generation of downlevel main-log, Snapshot, object-log, and metadata files.
  • Complete backward compatibility: replace snapshotFinalLogicalAddress with pageSize, append segmentSize to HybridLogRecoveryInfo, preserve downlevel metadata, and complete guarded v2.1 recovery.
  • Fix Snapshot flushes that zero the remainder of a sector when the checkpoint boundary falls mid-sector, affecting inline and object allocators.

Known limitations

Ted Hart (TedHartMS) and others added 30 commits July 8, 2026 16:40
…very guard

Wires the FLUSH object-log format for values whose length exceeds the 24-bit
RDH ValueLength field, fixes an intermittent reader-ring race, and guards the
recovery verbatim-copy path against headered records. The no-copy flush
optimization was attempted and reverted (see below).

VALUE chunked-object encoding (RecordDataHeader/LogRecord/ObjectLogReader/Writer):
- Object value <4MB: headerless exact length (bit 23 clear). >=4MB: chunked
  (bit 23 + 12-bit full-buffer count + 10-bit final-4KB-page count); reader
  sizes read-ahead from the extent, deserializer self-terminates; >16GB throws.
- Overflow value <16MB: exact 24-bit. >=16MB: ValueLength sentinel + leading
  ChunkHeader (symmetric with the >=sentinel overflow key path).
- SetObjectLogLengthHints / EncodeFlushObjectValue / DecodeFlushValueExtent.
  DiskLogRecord.SetChunkedFlushOverflowLengths (network/migration) unchanged.

Reader-ring race fixes (CircularDiskReadBuffer/DiskReadBuffer):
- ExtendUnreadLengthRemaining primes empty ring buffers using a reader-thread
  readIssued flag (not HasData/HasInFlightRead, which reads endPosition before
  the acquiring countdownEvent.IsSet and can re-read a primed buffer at the
  wrong offset -> short read); absorb sector-alignment over-read via
  readAheadSlack; DiskReadBuffer.Dispose drains in-flight reads before returning
  pooled memory (single-record read path disposes via using without OnEndReadRecords).

Recovery snapshot-copy guard (ObjectAllocatorImpl): fail fast when copying a
record whose overflow key/value is at/above the RDH sentinel (leading
ChunkHeader) -- the keyHint+valueHint sizing would under-copy and truncate it.
Chunked object values over-copy harmlessly (deserializer self-terminates).

No-copy flush: attempted (useLivePage for ReadOnly full-page aligned flushes)
and reverted. It passed the previously-failing SnapshotRecoveryDeferredObjectLoad
(10/10) and full recovery (201) + recordops (6x256), but review found a real
crash-recovery race: read-only-region records are not content-immutable
(Upsert/RMW/Delete seal + dispose the superseded source in place down to
HeadAddress), and a ReadOnly flush holds no epoch during the async device write,
so writing the live page can persist torn/half-cleared bytes and mark the page
durable while the superseding tail record is not. The srcBuffer copy isolates
the write. A safe no-copy needs a per-page flush-in-progress freeze on
seal/dispose; deferred. Doc section 7 records the full analysis.

Tests: LargeObjectDiskWriteReadChunkedValue, LargeOverflowValueChunkedTest,
RecordDataHeaderFlushEncodingTests. Validated: recordops 6x (256/4-skip),
recovery suite (201), SnapshotRecoveryDeferredObjectLoad 10/10, format clean.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 69a17cc0-e986-4422-bd1d-fd0d0b8a2c37
…y flush safe

A no-copy flush attempt that made every mutator flip a superseded read-only
record to Invalid atomically before clearing it (SealAndInvalidate then
OnDispose, matching CreateNewRecordUpsert/RMW, incl. changing CreateNewRecordDelete)
was implemented and reverted: it still doesn't close the race. The async device
write (LocalStorageDevice.WriteAsync) reads the live page over the whole I/O
duration, so it can read the old Valid RecordInfo at one instant and the
concurrently-cleared body at a later instant -> a torn Valid record on disk.
Invalidate-before-clear only helps an in-memory reader (one atomic word), not a
byte-by-byte device read of a mutating buffer. A safe no-copy needs a per-page
flush-in-progress freeze; documented in section 7.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 69a17cc0-e986-4422-bd1d-fd0d0b8a2c37
…lush contract

A plain ReadOnly, full-page, sector-aligned flush now writes the live main-log
page directly to the device (useLivePage) instead of copying it into a
srcBuffer; Snapshot/Recovery/partial/unaligned flushes still copy. The live
records are stamped in place with the length hints + ObjectLogPosition, which is
non-destructive to in-memory readers (the ValueLength property masks the raw
field to ObjectIdSize; the objectId slot is untouched) and the page stays
resident throughout the flush (HeadAddress <= FlushedUntilAddress).

Correctness relies on the contract that OnDispose keeps a record byte-consistent
(readable) throughout a flush -- it copies off what it needs for cleanup rather
than tearing the record's flush-critical bytes -- so the async device write always
observes a consistent record even if a concurrent Upsert/RMW/Delete supersedes it
(the only other in-place mutation, Seal(), is a single atomic RecordInfo word).
Output is byte-identical to the copy path (perf only). Doc section 7 records the
contract and a caveat that ClearHeapFields must be made flush-safe to honor it for
every store.

Validated: recordops 256/4-skip; main Tsavorite.test 301/26-skip; recovery
201/9-skip (incl. SnapshotRecoveryDeferredObjectLoad 10/10); Garnet SeSaveRecover
17. Format clean.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 69a17cc0-e986-4422-bd1d-fd0d0b8a2c37
…f copy

The snapshot-region recovery verbatim copy sized each record by its RDH
KeyLength/ValueLength hints, which cap at the sentinel and omit the leading
8-byte ChunkHeader + the length beyond the sentinel for an overflow key/value
at/above its sentinel (>= 16 MB value, or a sentinel key) -- so such records were
guarded with a fail-fast throw to avoid silent truncation.

Now size such a headered record by the successor object record's snapshot
position minus this record's: exactly this record's raw key+value+header+padding
extent, copied verbatim (the reader re-frames from the record's own ChunkHeader
and ignores any trailing over-copy, so this can never under-copy/truncate). This
removes the guard for a headered record that has a successor object record on its
page (the common case). A headered record that is the LAST object record on its
page has no successor to bound it (its exact extent would need a ChunkHeader read
up to the full overflow length away), so that narrow case stays guarded.
Headerless records and chunked object values are unchanged (their hints are exact
/ safely over-read).

Adds RecoverSnapshotHeaderedOverflowValue (verified to exercise the headered
snapshot-copy path). Full recovery suite 202/9-skip; recordops 256/4-skip; no
regression to the non-headered paths.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 69a17cc0-e986-4422-bd1d-fd0d0b8a2c37
…on-diff

Section 9 now documents that a >=-sentinel overflow key/value record is copied by
the successor object record's snapshot-position difference (exact raw extent,
never under-copies); only a headered record that is the last object record on its
page remains guarded.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 69a17cc0-e986-4422-bd1d-fd0d0b8a2c37
Introduce the new FLUSH out-of-line VALUE length encoding that will replace the bit-23-chunked/bit-22-overflow-header/24-bit-exact scheme: low 12 bits of the ValueLength field hold bits 0-9 payload (exact byte size if isExactSize bit 11, else 4KB-page count with 1023 sentinel) and bit 10 hasHeader. Values <=1023 bytes are headerless (isExactSize); longer values carry a leading ChunkHeader and a page-count encoding for a precise (no 4MB over-read) initial read. Pure-additive: new encoders/decoders/predicates and unit tests only; the old scheme and all callers are untouched, rewired in the next increment.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 69a17cc0-e986-4422-bd1d-fd0d0b8a2c37
Switch the object-log overflow VALUE path from the old 24-bit-exact/16MB-sentinel encoding to the v2.2 12-bit encoding: values >1023 bytes now carry a leading ChunkHeader (was >=16MB) and the RDH holds a 4KB-page-count/sentinel read hint plus the has-header bit, giving precise reads (no 4MB over-read) and a header signal independent of the sentinel (prerequisite for DMA-padded overflow).

Object values keep the current hint encoding (deferred unification); the reader value path is type-selected so the two coexist cleanly. Reader uses the flush-specific FlushValueHasHeader (not the shared ValueLengthIsSentinel, which the network/DiskLogRecord path still uses). ReadOverflowHeaderAndExtend now clamps the read-ahead extend to the shortfall (below-sentinel page-count hints cover the header, so no negative extend) and skips DMA alignment padding. Recovery successor-diff predicate keyed off FlushValuePageCountIsSentinel (only the sentinel under-counts; below-sentinel headered values over-count safely via the hint). Updated RecoverSnapshotHeaderedOverflowValue (trailing record now headerless at 1000B).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 69a17cc0-e986-4422-bd1d-fd0d0b8a2c37
Implement the zero-copy direct-DMA object-log write for overflow spans > MaxCopySpanLen (128KB), replacing the retained/disabled buggy WriteDirect. On-disk layout is [ChunkHeader][alignmentPadding][data]: the header + zero alignment padding + a small source-alignment initial fragment are copied through the sector-aligned buffer so the DMA disk offset lands on a sector boundary while the DMA source (the pinned byte[] data) is also sector-aligned; the sector-aligned interior is DMA'd straight from the byte[]; a small end fragment (plus any remainder past a 1GB segment boundary) is copied through the buffer.

Fixes the two bugs in the retained code: (1) it aligned the disk offset but DMA'd from an 8-byte-aligned (unaligned) source -> now the initial fragment is sized to sector-align the SOURCE and header padding sector-aligns the disk offset; (2) the buggy recursive multi-segment path -> replaced with a single-segment DMA + buffered fallback for the rare cross-segment remainder. The alignment math is extracted to ObjectLogDmaAlignment.Compute (non-generic, unit-tested for fragment sizes 0/sectorSize-1/sectorSize/2). The value's alignment padding is threaded via ObjectLogWriter.lastValueAlignmentPadding into the RDH page-count read hint (SetObjectLogPositionAndLengthHints/SetObjectLogLengthHints) so it spans header+padding+data; the reader already reads the padding from the ChunkHeader and skips it.

Validated: recordops 318, recovery 202, LargeObjectDiskWriteReadBigKeyAndValue 36 (multi-MB values now DMA'd), LargeOverflowValueChunkedTest (16-21MB DMA'd), all green.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 69a17cc0-e986-4422-bd1d-fd0d0b8a2c37
Adds RecoverFoldOverHeaderedOverflowValue covering FoldOver recovery of multiple headered overflow values (below- and above-128KB DMA threshold), verifying the v2.2 overflow header framing survives FoldOver recovery's in-place object-byte reuse and per-record position reconstruction without drift or corruption.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 69a17cc0-e986-4422-bd1d-fd0d0b8a2c37
…overflow

Root cause: during Snapshot recovery of a downlevel (v2.1) checkpoint, the
stable-boundary reposition path (SetRecoveredObjectLogRecordStartPosition)
unconditionally clears the ReuseObjectIdForSize flag and marks the record in
the current (v2.2) hint format, but does not insert the leading ChunkHeader
that a large overflow key (>= the 1023 KeyLength sentinel) or overflow value
(> kOutOfLineExactSizeCutoff) requires. The v2.1 object-log stream has no such
header, so the v2.2 reader would consume 8 bytes of the value as a bogus
header -- silent corruption.

The other v2.1 recovery paths are already safe: FoldOver recovery does not
re-flush the object log (it reads via the _v21 decode and upgrades lazily),
and the Snapshot fuzzy-region verbatim-copy path (RepointObjectLogPosition)
preserves the flag so the record stays downlevel. Only the reposition path
converts, and only large overflow keys/values differ between v2.1 (dense) and
v2.2 (headered) bytes -- everything else is byte-identical and repoints safely.

Change: detect a downlevel source in the reposition path and, if it would
convert to a headered overflow key/value, throw a clear "not yet implemented"
exception instead of silently corrupting. SetDeserializedValueObject now
preserves the flag bit across the deserialized-length store (masked back off
where the length is read) so the source is still detectable for object values.

All of this is a strict no-op for v2.2 sources (the flag is never set on a
v2.2 record); verified by the still-green recovery (204), recordops (318), and
main Tsavorite.test (301) suites.

What NOT to do (future agents): do not implement the full v2.1->v2.2 header
insertion (re-serialize-on-recovery, growing the object log) until there is a
test fixture that can produce a v2.1 checkpoint -- new records are never
written in v2.1, so the conversion is currently untestable and must not ship
unvalidated.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 69a17cc0-e986-4422-bd1d-fd0d0b8a2c37
….2 recovery

Update §10 (Versioning & downlevel) to the now-resolved state: v2.2 = checkpoint
version 8, v2.1 = version 7 (HybridLogRecoveryInfo), per-record bit-63
ReuseObjectIdForSize discriminator, and the full v2.1->v2.2 recovery path map
(FoldOver lazy upgrade; Snapshot fuzzy verbatim-copy preserves the flag; only the
Snapshot stable-boundary reposition converts and fails fast on a large overflow
key/value that would need a ChunkHeader). Note the 1<<30 per-chunk cap and that
full header-insertion conversion is untestable until a v2.1 fixture exists.

Update §9 to correct the flag-bit handling (RepointObjectLogPosition preserves
bit 63; SetRecoveredObjectLogRecordStartPosition clears it to convert) and add the
v2.1 reposition guard. Remove a stray duplicate "## 9. Recovery & positions"
header between §7 and §8.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 69a17cc0-e986-4422-bd1d-fd0d0b8a2c37
…age)

The copy-vs-live-page comment in ObjectAllocatorImpl.WriteAsync now explains the
core reason a Snapshot flush must copy: it serializes objects to a SEPARATE
snapshotFileObjectLogDevice that is disposed right after the checkpoint and
stamps records with positions in it, so stamping the live record (useLivePage)
would point it into a soon-disposed file. Also notes the disk-image-only
SetInvalid of v+1 CPR records, why ReadOnly no-copy is safe (main-tail position),
and the open ReadOnly-flush-racing-snapshot question under investigation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 69a17cc0-e986-4422-bd1d-fd0d0b8a2c37
…Page/ConvertToInline invariant

Move object values onto the same 12-bit out-of-line ValueLength encoding as overflow
(EncodeFlushOutOfLineValue: bits 0-9 exact/page-count, bit 10 hasHeader, bit 11 isExactSize,
cutoff 1023), with per-chunk ChunkHeader framing for objects whose serialized length exceeds
the cutoff. This frees RDH bits 12-23 for the future inline-portion+object record feature
(the "53432153" decision) and gives precise read-ahead sizing for medium objects.

On-disk headered-object layout: [1023-byte headerless prefix][hdr_1][chunk_1]...[hdr_N][chunk_N].
An object <= 1023 data bytes stays fully headerless (isExactSize). Each 8-byte ChunkHeader is
written on an 8-aligned object-log position (so it never straddles a buffer/segment boundary) and
its currentLength (| ContinuationFlag) is back-filled when the buffer fills; headers are appended
(position advances monotonically), never slide-inserted. The reader strips the prefix/padding/
headers and follows the continuation chain, extending read-ahead per-chunk for sentinel objects.
Recovery reconstructs the exact on-disk extent (prefix + padding + headers + data) via the reader.

Key changes:
- RecordDataHeader.SetObjectLogLengthHints: objects route through EncodeFlushOutOfLineValue with the
  writer-supplied on-disk extent (retiring EncodeFlushObjectValue for objects).
- LogRecord: GetObjectLogRecordStartPositionAndLengths decodes objects via the 12-bit
  DecodeFlushValueInitialReadExtent; position/extent hints threaded on flush and recovery.
- ObjectLogWriter: self-contained WriteObjectData buffer loop (headerless prefix + 8-aligned
  back-filled per-buffer ChunkHeaders), lastObjectExtent tracking, CopyRecoveredObjectBytes short-read.
- ObjectLogReader: header-stripping ReadObjectData/AdvanceToNextObjectChunk; DoDeserialize stores the
  on-disk extent for recovery.
- ObjectAllocatorImpl: pass the object extent on flush; snapshot verbatim-copy predicate uses
  FlushValueHasHeader; last-object-record fallback uses the RDH hint + allowShortRead.

Fix a latent useLivePage bug this encoding exposed (LogField.ClearObjectIdAndConvertToInline):
a no-copy (useLivePage) object-log flush stamps the flush ValueLength encoding into the LIVE record's
RDH. Readers are unaffected (the ValueLength property masks the raw field to ObjectIdSize for
out-of-line values), but record disposal (Delete/elision) flips the field to inline and previously
"kept the current length", after which the property returns the raw stamped value. A 4-byte object
stamps raw ValueLength = 2048|4 = 2052 (isExactSize bit), so the disposed record then claimed a
2052-byte inline value -> GetRecordLength/filler/scan-walk corruption (iteration over-counted deleted
keys; compaction lost live records). Baseline's EncodeFlushObjectValue returned raw = 4 (== ObjectIdSize)
for a 4-byte object, coincidentally preserving the invariant and masking the bug. Fix:
ClearObjectIdAndConvertToInline now sets the converted field's RDH length to ObjectIdSize (the physical
inline-slot size of the freed ObjectId slot) explicitly. No-op for SpanByte/normal in-memory records.

Validated: Tsavorite recovery 204/0/9, recordops 318/0/4 (incl. Revivification 87/0/4), main 301/0/26;
Garnet SeSaveRecover+RespObjectCommand 30/0/2.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 69a17cc0-e986-4422-bd1d-fd0d0b8a2c37
Update sections 2-9 for the unified 12-bit out-of-line ValueLength encoding (objects now share
EncodeFlushOutOfLineValue with overflow: isExactSize/hasHeader/page-count, cutoff 1023) and the
headered-object on-disk framing ([1023-byte prefix][8-aligned back-filled per-buffer ChunkHeaders]).
Document the useLivePage stamp invariant and the ClearObjectIdAndConvertToInline fix, the successor-diff
+ allowShortRead snapshot recovery sizing (removing the retired last-record fail-fast throw), and extend
the v2.1 reposition guard note to object values. Retire references to the old bit-23-chunked scheme.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 69a17cc0-e986-4422-bd1d-fd0d0b8a2c37
…ase-1 wire protocol)

The aof-chunk branch must not introduce changes to AOF or Migrate/Replication;
those belong to tedhar/aof-migrepl-chunk. Phase-2 (OAImpl object-log) had
reworked the migration wire protocol into a "prefix-only-when->=sentinel"
optimization built on the branch-added RDH overflow-length hints. This restores
the Phase-1 "always send a 4-byte length prefix" protocol on aof-chunk so the
cluster files are byte-identical to aof-migrepl-chunk.

Reverted to aof-migrepl-chunk (Phase-1, always-prefix):
- libs/cluster/Server/Migration/MigrateSessionCommonUtils.cs
- libs/cluster/Server/Replication/PrimaryOps/DisklessReplication/ReplicationSnapshotIterator.cs
- libs/cluster/Session/ChunkedRecordReassembler.cs

Necessary leakage (shared file, Phase-1 migration serialization adapted to the
Phase-2 LogRecord API):
- libs/storage/Tsavorite/cs/src/core/Allocator/DiskLogRecord.cs
  SerializeChunked / SerializeInlinePortionForMigration now call
  SetObjectLogPositionAndLengthHints (renamed from
  SetObjectLogRecordStartPositionAndLength); the dead GetSerializedSize()
  wrapper is removed. The object value length is still left zero; the receiver
  derives it from the always-prefix wire format.

The RDH-hint migration optimization can be re-added later as a separate change
on top of migrepl-chunk/main.

Validated (net10.0): recordops 318/0, recovery 204/0, cluster migrate 55/0,
replication.disklesssync 30/0, replication 107/0.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 69a17cc0-e986-4422-bd1d-fd0d0b8a2c37
…ffer_end-8 boundary

When the first post-prefix object ChunkHeader lands at exactly buffer_end - ChunkHeader.TotalSize
(RemainingCapacity == 8), the 8-byte header fills the write buffer with no data room. The writer
previously fail-fast threw ("not yet implemented"). It now pokes the placeholder header; the next
CopyObjectDataBytes sees a full buffer and AdvanceObjectBuffer back-fills it as a zero-length
continuation chunk (currentLength = 0 | ContinuationFlag), resuming the object data in the next
buffer. The reader (AdvanceToNextObjectChunk) already skips zero-length continuation chunks.

Only the first post-prefix header can hit this edge -- a fresh buffer always leaves at least one
sector for a header plus data. The PokeObjectChunkPlaceholder assert is relaxed to room >= 8
(room is always a multiple of 8, so the minimum non-zero value is 8).

Adds ObjectChunkZeroLengthFirstChunkTest: dense overflow byte-span fillers (each 8 + length
object-log bytes, no inter-record padding) position the object's start at buffer_end - (1023 + 8),
so the first 8-aligned header lands at buffer_end - 8. A small internal ObjectLogWriterDiagnostics
captures the first-header RemainingCapacity and the zero-length-chunk count so the test asserts the
boundary was actually hit; the object then round-trips from disk through the zero-length-chunk path.

Validated (net10.0): recordops 319/0, recovery 204/0, main Tsavorite.test 301/0.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 69a17cc0-e986-4422-bd1d-fd0d0b8a2c37
Ted Hart (TedHartMS) and others added 25 commits September 14, 2026 09:39
ComputeRecoveryOverflowKeyHash created and disposed a CircularDiskReadBuffer per
overflow-key record, renting its pooled object-log read buffer from the depot
each time. Pass the ring by ref from RecoveryStatus.IsolatedKeyReadBuffers so it
is created once per recovery and disposed with the RecoveryStatus. A ring is
bound to one device, so it is replaced when the main and snapshot object logs
alternate; recovery processes those in phases, so that is rare.

Also document why this path reads through the streaming ring rather than issuing
one direct read into a caller-allocated buffer: the record bounds a headered key
by a 4 KB page count that rounds up past the last record's written extent, and a
FoldOver Pass 1 has no durable object-log end to clamp against, so ReadDirect
would fault on the resulting short read at end of file.
ObjectLogWriterDiagnostics wrote a process-shared static on every headered
object and incremented a shared counter non-atomically from flush threads.

- Move recording behind [Conditional("DEBUG")] entry points so a Release build
  compiles out the calls and their arguments entirely, leaving no production
  cost. The zero-length test moves into OnChunkBackfilled so Release carries
  no comparison either, and the counter increment is interlocked.
- Guard the counter assertions in ObjectChunkZeroLengthFirstChunkTest with
  #if DEBUG; its on-disk round-trip still runs in every configuration.
- Drop comments describing the superseded boundary-sector zeroing. The comments
  now state only what the code does: bytes above the logical endpoint reach disk
  verbatim and recovery never parses them.
A page whose objectIdMap is empty has no overflow keys, overflow values, or heap
objects, so it has nothing to serialize to the object log. That shortcut applied
only to ReadOnly, so an all-inline Snapshot page still built an ObjectLogWriter,
rented a 4 MB object-log ring, and walked every record to write nothing.

Two things kept Snapshot out of it: the shortcut wrote the full page rather than
stopping at the checkpoint boundary, and it used the main-log file offset rather
than the snapshot device's. Move the gate below the partial-range and
alignedStartOffset computation so it can issue the same span the serializing path
computes -- [alignedStartOffset, RoundUp(endOffset, sectorSize)) at
alignedMainLogFlushPageAddress -- which resolves both. Snapshot page ordering and
fuzzy rules still apply through the coordination callbacks, and the write sets
snapshotDeviceWriteIssued so completion owns releasing the page.

Recovery continues to fall through to record stamping; a string-only
ObjectAllocator with no flush buffers takes the same direct write it did before.
…n, not per-record bit

Recovery already knows the checkpoint metadata version
(HybridLogRecoveryInfo.hybridLogRecoveryVersion). Thread it to the object-log
page-reading and recovery-flush methods so the downlevel (v2.1) split/dense
decode is selected from the version, not the per-record ObjectLogPosition
ReuseObjectIdForSize flag (bit 63).

- Add HybridLogRecoveryInfo.UsesDownlevelObjectLog(int) plus the
  ChunkFramedObjectLogCheckpointVersion boundary constant.
- Carry the version on RecoveryOptions (read path) and PageAsyncFlushResult
  (flush path, defaulting to the current version), populated from
  recoveredHLCInfo.info.hybridLogRecoveryVersion; also on RecoveryStatus for the
  pass-2 object load and snapshot-recovery flush helpers.
- GetObjectLogRecordStartPositionAndLengths, ReadRecordObjects,
  ReadOverflowKeyHashCodeForRecovery, CalculatePageObjectSizes,
  FindHeadAddressCutoffOnPage, LoadObjectsForRecoveryPass2,
  ComputeRecoveryOverflowKeyHash, DeserializeObjectsOnPage and
  AsyncFlushPagesForRecovery take the checkpoint version; runtime reads pass the
  current version.
- Delete the SetDeserializedValueObject flag-preservation hack and drop the
  wasReuseObjectIdForSize read in SetRecoveredObjectLogRecordStartPosition (now
  reached only for a downlevel source, so its guard is unconditional).
- Keep kReuseObjectIdForSizeBit defined (v7 files on disk still have it set) but
  document it as no longer read as the decode discriminator.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a425332a-56d9-47ab-bc5f-c144d87b2e4f
…ry tests

Synthesize real downlevel (checkpoint version 7) object-store checkpoints as a TEST UTILITY and recover them with the
current reader, verifying every value/overflow-key byte-for-byte. There is no v7 page-generation code in production; the
fixture takes a current-format FoldOver checkpoint of small (headerless) records — whose object-log bytes are already
dense and byte-identical to the v7 dense encoding — and rewrites, on disk, only what distinguishes v7:

- each out-of-line record's length is re-stamped into the v7 split form (RDH low bits + objectId-slot high bits) and its
  ObjectLogPosition word gets the ReuseObjectIdForSize flag (bit 63) with the current size-hint flags cleared, and
- the metadata is re-serialized through HybridLogRecoveryInfo.ToByteArray(targetVersion: 7).

The object-log bytes and record positions are left untouched, so no v7 object-log writer is needed.

Tests (Tsavorite.test.recovery/V7DownlevelRecoveryTests.cs):
- RecoverV7ObjectValueFoldOver: inline key + object value; exercises the v21 value decode.
- RecoverV7OverflowKeyFoldOver: overflow key + overflow value; exercises the v21 key decode (Pass 1 overflow-key hash)
  and value decode.
- V7FixtureHasDownlevelEncodingOnDisk: raw inspector (no production decoder) asserting metadata version 7 and bit 63
  set / size-hint flags clear on every out-of-line record.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a425332a-56d9-47ab-bc5f-c144d87b2e4f
…v7 records

Threading the checkpoint version through the recovery decode (previous commit) let a live object read decode a downlevel
(v7) record as current format. Recovering a v7 checkpoint under memory pressure evicts records to the main log WITHOUT
up-converting them, so those records remain in the v7 split-length encoding; a later runtime read (e.g. a pending Read of
an evicted record) reached GetObjectLogRecordStartPositionAndLengths with the current version and mis-decoded the length,
hanging on the bogus object-log read.

Select the downlevel decode per record via a new LogRecord.IsDownlevelObjectLogRecord(checkpointVersion): true when the
checkpoint version predates the chunk-framed format OR the per-record ReuseObjectIdForSize flag is set. Recovery still
drives the decode from the metadata version; the flag additionally covers live reads of a downlevel record left on the
main log by a memory-pressured downlevel recovery (a page can mix such records with up-converted ones, so the decision
must be per record). GetObjectLogRecordStartPositionAndLengths and the object-log reader's isLegacy both use it.

The bit therefore still carries its downlevel meaning and is not yet free for reuse; fully freeing it would require
up-converting every record during a downlevel recovery.

Adds V7DownlevelRecoveryTests.RecoverV7ObjectValueFoldOverLowMem, which recovers a v7 fixture under a tight
LogSizeTracker budget (forcing recovery eviction) and reads every record back byte-for-byte; it hangs without this fix.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a425332a-56d9-47ab-bc5f-c144d87b2e4f
…ures

The v7 fixtures covered only headerless records at or below the 511-byte
exact-size cutoff, leaving the size class that v7 stored dense but the current
format frames with a leading ChunkHeader unverified.

Add a general dense-object-log v7 fixture writer and fixtures over it:

- RecoverV7LargeObjectValueFoldOver at 513, 1024, 4096, and 20000 bytes
- RecoverV7LargeOverflowValueFoldOver at 512, 1024, 4096, and 20000 bytes
- RecoverV7OverflowKeyInlineValueFoldOver

Test-only; no production change. A throw injected into the downlevel decoder
fails 23 of the 24 v7 tests, the exception being the raw inspector that does not
recover, confirming every recovery fixture reaches that decoder.

Not covered: Snapshot v7 checkpoints, components above 4 MB, and multi-page v7
logs. The writer is single-page and its largest component is 20 KB, so the
reader's multi-buffer dense read is unverified.
The checkpoint already records pageSize and segmentSize, but not the object-log
segment size. That value is the split point for ObjectLogFilePositionInfo's
packed segment/offset word, so recovering under a different one resolves every
object-log position to a different segment and offset.

Append it after segmentSize, under the existing v8 log-geometry guard, and
include it in the metadata checksum. A v7 checkpoint writes neither trailing
value and passes zero for both, which reduces the checksum to the v7 formula.
PageSize and SegmentSize determine how logical addresses resolve to segment
files and offsets, and ObjectLogSegmentSize splits the packed object-log
position word. Recovering a checkpoint under different values reads the wrong
bytes with nothing to detect it, so compare all three against the recorded
geometry and fail recovery on a mismatch.

Checkpoints below v8 record no geometry and are skipped; the assumed values are
logged so an operator can confirm them against the original configuration.
A downlevel object log stores records headerless above the size that now
requires a chunk header, so its object bytes cannot be rewritten in place.
UpgradeObjectLogDevice receives those bytes in current format while the
original object log is read as the source.

Recovering a downlevel checkpoint that has an object-log device now fails
when no upgrade device is configured, rather than silently leaving the log
in a format a later release cannot decode. A store with no object-log
device has nothing to convert and is unaffected.
Populates KVSettings.UpgradeObjectLogDevice with a device alongside the
object log it will replace, so recovery has somewhere to write object
bytes in current format. Requires --recover, since a checkpoint must be
read to have anything to convert, and tiered storage, since without it
there is no object log on disk. A store with objects disabled needs no
conversion and gets no upgrade device.
Recovery of a v7 checkpoint now rewrites the log into the current format
instead of leaving records readable only through the per-record downlevel
decode path.

A dedicated ascending pass runs inside the hybrid-log read loop: each page
is read, indexed, and its objects deserialized, then flushed with its
objects re-serialized through the normal writer path -- which inserts the
ChunkHeaders the current format requires -- into KVSettings.UpgradeObjectLogDevice.
Main-log pages are rewritten in place because record inline sizes are
unchanged; a per-record assertion enforces that invariant. Pages convert
before any eviction can observe them, so no page is evicted still holding
downlevel object-log positions.

The v7 tail stays in objectLogTail so reads of the downlevel device remain
correctly bounded while conversion appends to a separate upgradeObjectLogTail.
On completion the device is swapped and SetObjectLogTail adopts the
conversion's append position rather than the checkpoint's downlevel tail.
…-log position

A page read from a downlevel checkpoint arrives with its PageHeader already
carrying an object-log position on the device the up-conversion is replacing.
WriteAsync stamped the header with objectLogTail through
SetLowestObjectLogPosition, which was doubly wrong during conversion:

- SetLowestObjectLogPosition only writes when the word is NotSet, so the call
  silently no-opped and the converted page kept its downlevel position.
- objectLogTail is deliberately unset during the hybrid-log phase; the
  conversion appends to upgradeObjectLogTail.

GetLowestObjectLogSegmentInUse derives truncation segments from these headers,
so a converted log would have driven TruncateUntilSegment and RemoveSegment
with downlevel segment numbers against the new device, silently discarding live
object data.

Add a force parameter to SetLowestObjectLogPosition and use it with
upgradeObjectLogTail while upgrading, so each converted page is re-stamped with
the position on the new device where that page's objects are written.

lowestObjectLogSegmentInUse needs no reset: conversion appends from segment 0
and the field is only raised by truncation, which cannot run during the
recovery read phase. Assert that. This also settles the SegmentSizeBits TODO in
GetLowestObjectLogSegmentInUse - recovery now verifies the checkpoint's
ObjectLogSegmentSize against the store's settings, so the live tail's segment
bits are the bits the header was stamped under.

The on-disk assertions in V7DownlevelRecoveryTests could not see either defect
because the fixture fit on one main-log page, where both tails are position 0.
Make the fixture transform and the on-disk walks page-aware via a shared
GetOnDiskRecordOffsets helper that skips PageHeaders and end-of-page filler,
grow RecoverV7UpConvertsMainLogOnDisk to span pages, and assert each page
header agrees with the first object record on that page. Both defects were
confirmed to fail this test before the fix.
Snapshot recovery of a downlevel checkpoint has two regions to convert, with
unrelated object-log address spaces: the hybrid-log region, whose objects are in
the main object log, and the snapshot region, whose objects are in the snapshot
object log. Only the first was converted; the snapshot region took the verbatim
snapshot-to-main object copy, which writes dense downlevel bytes into a
current-format object log and leaves the record flagged downlevel. That reads
back correctly today only because the per-record ReuseObjectIdForSize bit still
selects the downlevel decode, which is exactly the bit being freed.

Convert the snapshot region the same way the hybrid-log region is converted: an
ascending per-page pass inside the snapshot read loop deserializes the page's
objects from the downlevel snapshot object log, then flushes the page to the
main log, re-serializing those objects onto the upgrade device with ChunkHeader
framing where the current format requires it. Converting per batch keeps every
page converted before the next batch's read can evict it, since an evicted page
would keep its snapshot object-log positions. The conversion pass supersedes the
deferred object-load pass, which has nothing left to load.

In WriteAsync the up-conversion branch now precedes the snapshot verbatim copy,
since a converted snapshot page has its objects resident and must be
re-serialized rather than copied.

The conversion now spans both recovery phases, so it stays open across them.
BeginObjectLogUpgrade moves above the empty-range early return - a snapshot
recovery whose hybrid-log region is empty still has a snapshot region to
convert, and without this the whole snapshot path silently kept the old
behavior. CompleteObjectLogUpgrade moves to the end of the snapshot phase and
re-adopts the converted tail, because recovery sets the tail between the two
phases so the snapshot region appends after the hybrid-log objects.

Tests: three v7 Snapshot fixtures - snapshot-only, snapshot-only under a small
memory budget that forces eviction, and a mixed fixture with both a non-empty
hybrid-log region and a non-empty snapshot region - plus an on-disk assertion
that the converted main log carries no downlevel records. Disabling the
conversion pass fails all eight snapshot tests; dropping the cross-phase tail
re-adoption corrupts the object log.
…level selector

IsDownlevelObjectLogRecord ORed the per-record ReuseObjectIdForSize flag (bit 63)
into the version-based decode selection, so a live read of a record that a
memory-pressured downlevel recovery had evicted without up-converting would still
decode. Recovery now up-converts every record of a downlevel checkpoint, across
both the hybrid-log and snapshot regions, before any page can be evicted or
flushed, so a live read can no longer encounter a downlevel record. Select the
decode from the checkpoint metadata version alone, which frees bit 63: nothing in
production reads or writes it, and only tests that synthesize v7 record images
still touch it.

Delete SetRecoveredObjectLogRecordStartPosition and its caller. It up-converted a
downlevel record's encoding in place without re-serializing, and threw for any
large overflow or object that the current format frames with a leading
ChunkHeader. The up-conversion pass re-serializes instead, inserting that framing,
so the in-place path is unreachable. Its call site becomes a throw, since a
downlevel record reaching the recovery flush unconverted would otherwise persist a
page later releases cannot decode.

With the per-record fallback gone the existing low-memory v7 tests become
discriminating rather than incidental: disabling the conversion makes
RecoverV7ObjectValueFoldOverLowMem read a garbage extent off disk and hang, where
before it passed either way.
…upgrade

Recovery up-converts a downlevel object log into the upgrade device and repoints
the live allocator at it, but that only lasts for the process: the files on disk
still have the downlevel object log under the live name, and the recovered
checkpoint still describes it. Finish the upgrade on disk.

GarnetServer.RunUpgrade recovers, takes the checkpoint that records the converted
object-log positions, closes the store, and renames the converted object log in as
the live one. Program.Main routes an --upgrade run to it instead of Start, so the
process converts and exits rather than serving.

The swap is 2N file moves, so ObjectLogUpgradeSwap journals it: a marker naming the
live, upgrade, and retired base names is written before the first move and deleted
after the last. Every move is "move only if the source exists and the destination
does not", so finishing an interrupted swap is just re-running it. A marker found at
startup is resolved before any device opens the object log: an --upgrade run
completes it, and any other run refuses to start rather than open a half-renamed
object log. The downlevel segments are retained under a timestamped base name.

Guards, each of which otherwise loses data silently:

- Swap only when recovery actually converted something (LogAccessor.ObjectLogWasUpgraded).
  On a store already in the current format the upgrade device holds no data, so
  swapping it in would replace a good object log with an empty one.
- Refuse --upgrade when the upgrade device already has segments from a previous
  attempt, since it is written from its start and the two runs would interleave.
- Refuse --upgrade with the append-only file enabled. The run recovers, checkpoints,
  and exits; an AOF around that would be re-stamped at a version the next normal
  recovery discards.

Tests cover the guards, the journaled rename including resume, and an end-to-end
upgrade run against a current-format store that must leave every durable object-log
byte in place. Dropping the ObjectLogWasUpgraded guard retires and replaces that
store's object log.
Add the option to the configuration table, note the AOF restriction and the already-current no-op in defaults.conf, and describe the run: what it rewrites, where the original object log is retained, and how to finish an interrupted rename.
An upgrade run recovers through StoreWrapper.RecoverAsync, which already replays
the AOF, so the version arithmetic works out: the run recovers a checkpoint at
version V and runs at V+1, the post-checkpoint AOF records are also at V+1 and so
are applied rather than skipped, and the checkpoint a converting run takes captures
them. A later start recovers that checkpoint, runs at V+2, and correctly skips the
whole AOF as already checkpointed. A run that converts nothing takes no checkpoint,
leaving those records in the AOF to be replayed again, which is equally correct.
Drop the refusal.

Tests: an upgrade run with AOF enabled must preserve both checkpointed and
AOF-only writes, and the recover-replay-checkpoint-reopen sequence that a real
conversion depends on is covered separately. Skipping the AOF commit in the fixture
loses exactly the sixteen AOF-only keys, so the assertions distinguish the two
classes of loss.

Unresolved: while developing these tests, an upgrade-with-AOF run intermittently
tripped the CircularDiskReadBuffer assertion "Increment 47441 must be less than
SectorSize (512)" -- twice in six runs of the AOF test, and in three consecutive
runs of the fixture. It has not reproduced since, over nineteen runs spanning the
same code and invocations, so no cause has been established and nothing here
claims to fix it. The object-log read position it reports is far beyond the
buffer's ongoing position, which is worth chasing if it resurfaces.
Recovery scans down only to the checkpoint's headAddress: colder pages stay on
disk and are never read. The object-log up-conversion inherited that window, so on
any store larger than memory most records were left in the downlevel encoding
while the object log they point into was retired and replaced. Nothing in the
existing tests could see this, because their fixtures are small enough that the
recovery window is the whole log. An end-to-end Garnet store with a 400-record
FoldOver checkpoint left 300 of those records unconverted.

Those records are then unreadable: the per-record downlevel selector is gone, so a
live read decodes downlevel bytes with the current reader and gets a garbage
extent. This is the most likely source of the intermittent "Increment must be less
than SectorSize" assertion noted in the previous commit.

Scan from the checkpoint's beginAddress when up-converting. The conversion pass
already evicts each page as it walks up, so the cost is IO rather than memory.

Also dispose KVSettings.UpgradeObjectLogDevice in GarnetDatabase.Dispose. It was
the one device the database left open, so the converted object log could never be
renamed into place -- the rename failed with a sharing violation that no amount of
retrying would clear. Bound the rename's retry anyway, for the handle lag that
outlives a device Dispose.

Tests: UpgradeConvertsDownlevelStoreEndToEnd takes a real Garnet store through the
whole feature -- rewrite its checkpoint on disk as downlevel, run --upgrade, then
start normally and read every key -- with and without the append-only file. It
asserts that no record is left in the downlevel encoding, that the downlevel object
log is retired and the converted one promoted, and that no rename marker survives.
FoldOverCheckpointObjectsSurviveDiskReads is the control that isolates failures to
the up-conversion rather than to FoldOver object recovery.
…Ranges

The override that makes an up-conversion scan the whole log rather than the
preload window sat in InternalRecoverAsync, after SetRecoveryPageRanges had
already computed and clamped the same fields. That silently discarded the
FlushedUntilAddress clamp. Compute it with the rest of the page-range logic
instead, so every field that decides what recovery reads is derived in one place.

The clamp is now skipped only for an up-conversion, and says why: being durable
in this process says nothing about the on-disk encoding, so skipping those pages
would leave downlevel records behind -- the same defect the override exists to
prevent.

Also cover Snapshot checkpoints in the end-to-end upgrade test. It previously ran
only FoldOver, which is not Garnet's default, so the configuration most stores use
was untested. The fixture's on-disk v7 rewrite now handles both: FoldOver keeps
every record on the main log, while Snapshot splits them between the main log
below mainLogRecoveryEndAddress and the snapshot file above it. Skipping the
snapshot-region rewrite breaks the Snapshot cases, so the new coverage is load
bearing. Its control test is parameterized to match.
No caller ever passed RecoveryPhase.Pass2: recovery reads pages as Pass1 and the
post-recovery head read as None, and Pass 2 objects are loaded separately through
LoadObjectsForRecoveryPass2. The enum member and everything reachable only from it
are dead.

Drop the Pass2 branch in AsyncReadPagesForRecovery, which was the only code that
attached read buffers to a recovery page read, along with the buffer cleanup in its
catch and the objectLogDevice parameter that only that branch used. The read-buffer
field itself stays: the scan iterators still set it through
AsyncReadPageFromDeviceToFrame.

In the object-allocator read callback, selecting the page's objectIdMap was also
reachable only for Pass2 -- with None and Pass1 the only remaining phases, the
branch always resolved to the transient map. Use it directly.
…tore

Cluster startup recovers through the replication manager, which dispatches on the
node's role: a replica does not read its checkpoint at all unless
ClusterReplicaResumeWithData is set. An upgrade run routed through that path would
convert nothing and then report success, leaving a downlevel store that the next
release cannot read. Give the upgrade its own recovery entry point that performs
the standalone sequence unconditionally -- recover checkpoint, recover and replay
AOF -- so it examines the store whatever the node's role. The run never serves
requests or establishes replication, so the role has no bearing on what has to be
converted on disk.

This path has no automated end-to-end test. RunUpgrade itself was observed to
complete in cluster mode, but the test then hangs in teardown: something the
cluster server holds makes directory enumeration fail, and TestUtils.DeleteDirectory
retries that in an unbounded loop regardless of its wait argument. Both of those are
worth fixing, neither is the conversion logic, and a hanging test is worse than none.
The documentation says the path is untested and tells the operator what to check.

Refuse to up-convert a multi-database store. Every database has its own checkpoint
directory but they all share one object log, since the device is built from a fixed
"Store"/"hlog_objs" descriptor with no database id. Each database's allocator
therefore converts into the shared upgrade device from its own position zero, and
the second one overwrites the first. Fail before the rename, while the live object
log is still untouched, rather than promote a log missing a database's objects.

Sharing one hlog across databases already loses data without any upgrade involved:
two databases checkpointed with tiered storage recover with one of them empty. That
is pre-existing -- the device descriptor is unchanged from before this branch -- and
needs the log devices made per-database, as the checkpoint and AOF directories
already are.
…zing metadata

GarnetClusterCheckpointManager overrides GetLogCheckpointMetadata to normalize
legacy trailing-cookie metadata into the embedded-cookie layout, parsing the
on-disk bytes and re-serializing them. It used ToByteArray(), which always
stamps the current CheckpointVersion. While only one version was recoverable
that round-trip was a no-op, but now that recovery accepts a downlevel
checkpoint it silently relabels one as current: UsesDownlevelObjectLog then
returns false and a dense object log is decoded as chunk-framed.

Add ToByteArrayPreservingVersion and use it from both re-serialization sites,
so a checkpoint read on a cluster node keeps the version it was written with.
Also report that version in the log-geometry mismatch messages, which is what
surfaced this.
Recovery errors are swallowed by default so a server can still come up on
whatever it recovered. An upgrade run must not do that: it would report that
nothing needed converting and then rename a partially converted object log
into place. Set FailOnRecoveryError for the duration of the upgrade recovery.
Move the v7 checkpoint synthesis and verification helpers out of
ObjectLogUpgradeTests into a shared V7CheckpointFixture parameterized by the
store directory, link it into Garnet.test.cluster, and add an end-to-end
cluster test that runs --upgrade over a downlevel store across the AOF and
FoldOver/Snapshot axes. It asserts that the records were converted, that the
downlevel object log was retired with no swap left pending, and that node
identity, slot assignment and all data survive the restart.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant