Add object-log chunking and no-copy flushing - #2081
Draft
Ted Hart (TedHartMS) wants to merge 77 commits into
Draft
Add object-log chunking and no-copy flushing#2081Ted Hart (TedHartMS) wants to merge 77 commits into
Ted Hart (TedHartMS) wants to merge 77 commits into
Conversation
…work, coming next)
…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
Flip the remaining value has-header decisions off the RDH ValueLength hint onto the objectId ValueIsExactSize position flag: ReadRecordObjects (overflow-value exact-vs-headered and object-value exact-vs-headered) and the snapshot-recovery-copy successor-delta trigger now test !ValueIsExactSize instead of FlushValueHasHeader(GetValueLengthRaw()). The has-header boolean is identical between the two sources (both mean data length > the 511-byte cutoff), so this is a pure source switch with no behavior change. Add LogRecord.ValueIsExactSize and LogRecord.ValueObjectIdSizeHint accessors for the value's position flag and objectId size hint. The object-value page-count SENTINEL (which gates the proactive per-chunk read-ahead extension in AdvanceToNextObjectChunk) stays on the RDH sentinel (1023 pages, ~4 MB). That extension is only correct when the object's on-disk extent exceeds the initial read-ahead block (one BufferSize, ~4 MB); the coarser objectId size-hint sentinel saturates at ~2 MB, so keying objectSentinel to it over-extends for objects in the [2 MB, 4 MB) window and leaves dangling in-flight reads (OnEndReadRecords assert). Fully retiring the RDH ValueLength hint therefore needs an extent-vs-read-ahead signal; deferred to W4b. RDH ValueLength remains dual-written. Validated: objectSizeBoundary net 103 pass (incl. 2/3/5 MB), full test.recovery 309 pass, test.recordops 373 pass; core builds warning-clean; format verified. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 70b25af1-de77-463d-9cbe-fd2b99763854
…ndaries Adds eviction-during-recovery sweeps (object and overflow values) across every object-log size boundary, recovering under a tight LogSizeTracker budget so the deferred object load must evict and read objects back from the copied main object-log (FindHeadAddressCutoffOnPage / FlushSnapshotPageForRecovery). Previously the eviction-during-recovery machinery was only exercised with tiny fixed-size objects, and the size-boundary sweep only ran at full memory; this covers the intersection. Includes an anchor test asserting head-advancing eviction for a 64 KB chunked object. Quarantines one newly-surfaced boundary as a tracked [Ignore] gap: small-memory Snapshot recovery of an out-of-line value larger than one 4 MB read-ahead buffer that is also the last object record on its page truncates the verbatim copy (single-buffer allowShortRead fallback). Sub-4 MB and all FoldOver cases are green. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 70b25af1-de77-463d-9cbe-fd2b99763854
Adds the checkpoint/recover counterpart of the disk-IO overflow-key sweep. Surfaces a pre-existing gap: recovering an out-of-line (overflow) KEY NREs in RecoverFromPage -> GetKeyHashCode64 -> LogRecord.Key because the objectIdMap is not populated during the recovery index-build pass. Size- and checkpoint-type-independent; inline-key recovery is unaffected. Quarantined as [Ignore] with the harness ready to validate once overflow keys are hashable at recovery (lands with the overflow-key/hybrid-value work). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 70b25af1-de77-463d-9cbe-fd2b99763854
WriteOverflowDma previously DMA'd only the first object-log segment then handed the segment-crossing remainder to the buffered path, which entered with segmentRemainingLen==0 and corrupted the object-log layout at the boundary, mis-decoding a later record's ChunkHeader during recovery Pass2. Now the whole sector-aligned interior is DMA'd straight from the pinned byte[], iterating across 1 GB segment boundaries with one FlushToDevice per segment. Segment fill is detected from the remainder captured BEFORE the write (the Offset setter masks SegmentSize->0 on exact fill, so a post-write RemainingSizeInSegment==0 check would leave SegmentId stale). The pin is refcounted when the interior spans >1 segment so it is freed only after the last write completes; a single write uses the plain handle. Reader (ReadOverflowHeaderAndExtend) is length-based and the read-ahead ring already crosses segments, so no read-side change is needed. Un-ignores and shortens the segment-boundary overflow recovery test (Snapshot+FoldOver) to fit under Windows MAX_PATH for the deep checkpoint device path. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 70b25af1-de77-463d-9cbe-fd2b99763854
During recovery Pass 1 (index build), RecoverFromPage hashed every record's key via GetKeyHashCode64(logRecord). For an overflow key that dereferences objectIdMap.GetOverflowByteArray (LogRecord.Key), but the transient objectIdMap is not populated until Pass 2, so overflow-key records NRE'd on a null objectIdMap. Add ComputeRecoveryOverflowKeyHash on the allocator: it reads only the record's overflow key bytes on demand from the object log (main object log for FoldOver/hybrid-log pages, snapshot object log for snapshot pages) into a temporary pinned buffer and hashes them via the store comparer (which hashes key bytes only). RecoverFromPage now guards on DataHeader.KeyIsInline, keeping the inline-key path unchanged. Recovery is a rare startup operation, so the extra per-record IO for the rare overflow-key case is acceptable. Un-ignore RecoverOverflowKey (Snapshot + FoldOver). Full recovery project: 371 passed / 0 failed / 11 skipped (env-gated + the two remaining Gap 1 quarantines). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 70b25af1-de77-463d-9cbe-fd2b99763854
…ry copy The last object record on a snapshot page has no successor to bound its exact object-log extent. The previous fallback sized the verbatim copy from the record's size hint and read a single 4 MB read-ahead buffer, so an out-of-line value spanning multiple buffers (e.g. a 5 MB object or overflow) was truncated at ~4 MB and mis-decoded on read-back during small-memory Snapshot recovery. Fix: for the last-on-page record, follow the record's ChunkHeader framing to its exact on-disk extent (CopyRecoveredObjectBytesFollowingFraming -> ObjectLogReader.CopyRecordObjectsFollowingFraming). The reader walk decodes each chunk header, self-extends the snapshot read-ahead as chunks are consumed, and tees every raw byte into the main object-log, so the value is copied whole. In copy-to-end mode ReadObjectData self-terminates after the final non-continuing data chunk (only serialize-completion back-fills a non-continuing header, so it is provably last), gated so the normal deserialize path is byte-for-byte unchanged. Also remove the now-dead allowShortRead path from CopyRecoveredObjectBytes, and the unused ChunkHeader.nextLength field. Tests: fold the 5 MB multi-buffer point back into the small-memory object/overflow low-mem sweeps (Snapshot and FoldOver) and drop the two quarantined [Ignore]d placeholders. Full test.recovery 375/0, test.recordops 373/0. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 70b25af1-de77-463d-9cbe-fd2b99763854
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Move initial read metadata entirely into objectId slots, use absolute chunk endpoints, and direct-read large aligned overflow payloads across segments. Preserve snapshot framing alignment and expand boundary coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a425332a-56d9-47ab-bc5f-c144d87b2e4f
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a425332a-56d9-47ab-bc5f-c144d87b2e4f
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a425332a-56d9-47ab-bc5f-c144d87b2e4f
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a425332a-56d9-47ab-bc5f-c144d87b2e4f
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a425332a-56d9-47ab-bc5f-c144d87b2e4f
…check, drain guard - GarnetLog.EnqueueObjectChunked now applies the per-key replication backpressure wait (via a hash-based BackpressureWaitKeyHash overload) before selecting/writing the sublog, matching the span Enqueue overloads so a large-object upsert cannot advance its sublog past the shipped watermark. - Bump AofHeaderVersion 4 -> 5: the chunked format repurposed the 0b0100 flags bit into AofHeaderTypeMask (0b0111), so a down-level v4 reader would misparse a chunk header as its non-chunked base type. The bump makes down-level recovery fail safe. - AofChunkedRecordReader rejects a segment length that exceeds the entry's remaining chunk region before AppendChunk dereferences the pointer (prevents an OOB read on a truncated/corrupt AOF). - ChunkedObjectSerializer.FlushFinal adds a no-progress guard: a non-empty ring the consumer did not shrink now throws instead of spinning forever (mirrors the full-buffer guard in Write). Adds a regression test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: dcc36669-1128-4a5f-ba8a-3a6c8479ccd1
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a425332a-56d9-47ab-bc5f-c144d87b2e4f
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a425332a-56d9-47ab-bc5f-c144d87b2e4f
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a425332a-56d9-47ab-bc5f-c144d87b2e4f
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a425332a-56d9-47ab-bc5f-c144d87b2e4f
Coordinate Snapshot and ReadOnly page writes, bound object-log read-ahead, and merge Snapshot recovery at the durable main-log boundary. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a425332a-56d9-47ab-bc5f-c144d87b2e4f
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a425332a-56d9-47ab-bc5f-c144d87b2e4f
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a425332a-56d9-47ab-bc5f-c144d87b2e4f
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
To be done
snapshotFinalLogicalAddresswithpageSize, appendsegmentSizetoHybridLogRecoveryInfo, preserve downlevel metadata, and complete guarded v2.1 recovery.OnDisposecopy-off/deferred-layout work required by the no-copy flush contract.