Merge pull request #367 from MPCoreDeveloper/perf/tombstone-delete

perf: durable Columnar DELETE in-place via tombstone markers (no flush rewrite)

Author: MPCoreDeveloper

docs/CHANGELOG.md

@@ -42,6 +42,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   overflow arena is reclaimed on the next explicit VACUUM/compaction. Regression: delete half the
   rows, `Flush`, reopen ÔÇö exactly the remaining rows come back. Measured DELETE in the `--pk`
   harness now includes this durability rewrite (~18.6K ops/s when deleting 10K of 100K rows).
+- **Durable DELETE is now in-place via tombstones (no rewrite)** ÔÇö non-transactional Columnar
+  deletes write a tombstone marker (the record's 4-byte length prefix is replaced by the NEGATIVE
+  slot size) instead of queueing a flush-time rewrite, and every raw record enumerator/compactor
+  skips the slot, so DELETE survives a reopen in O(delete). Flush/dispose compaction now only
+  covers transactional deletes (a marker written before commit would survive a rollback that
+  restores the row in the PK index; those deletes are compacted against the current PK after commit
+  ÔÇö rollback-safe). Tombstoned space is reclaimed by the tombstone-aware `CompactTable`, including
+  the ULID-migration compaction (which previously produced an empty file when tombstones were
+  present because `CompactTable` broke on a negative prefix). Measured `--pk` DELETE (10K of 100K
+  rows): ~0.54s/18.6K ops/s (flush rewrite)  **~0.16s/~64K ops/s (legacy)** and
+  **~0.12s/~81K ops/s (fixed-width)** ÔÇö DELETE is back on par with UPDATE.
 - **Dedicated SQL batch-INSERT fast path (WP14)** ÔÇö `ExecuteBatchSQL` INSERTs no longer build a
   per-row `Dictionary<string, object>`; VALUES clauses are parsed directly into column-ordered
   `object[]` rows (`PreparedInsertStatement.ParseValuesToArray`) and inserted via the new

src/SharpCoreDB/DataStructures/Table.CRUD.cs

@@ -1364,7 +1364,20 @@ private List<Dictionary<string, object>> ScanRowsWithSimdAndFilterStale(byte[] d
                 dataSpan.Slice(filePosition, 4));
 
             const int MaxRecordSize = 1_000_000_000;
-            if (recordLength < 0 || recordLength > MaxRecordSize)
+            if (recordLength < 0)
+            {
+                // Tombstoned (deleted) record: the prefix stores the negative slot size to skip.
+                int slotSize = -recordLength;
+                if (slotSize < 4)
+                {
+                    break;
+                }
+
+                filePosition += slotSize;
+                continue;
+            }
+
+            if (recordLength > MaxRecordSize)
             {
                 break;
             }
@@ -2586,9 +2599,10 @@ private bool HasExplicitNamedIndex(string column)
             foreach (var (storagePosition, _) in recordsToDelete)
                 engine.Delete(Name, storagePosition);
         }
-        else
+        else if (StorageMode != StorageMode.Columnar)
         {
-            // Track Columnar logical deletes so flush-time compaction makes them durable.
+            // Legacy logical-delete accounting (Columnar deletes are tracked in the branch below,
+            // which decides between an immediate durable tombstone and deferred flush compaction).
             Interlocked.Add(ref _pendingLogicalDeletes, recordsToDelete.Count);
         }
 
@@ -2639,6 +2653,20 @@ private bool HasExplicitNamedIndex(string column)
                 }
             }
 
+            if (this.storage is { IsInTransaction: true })
+            {
+                // Transactional delete: writing the tombstone now would survive a rollback that
+                // restores the row in the PK index, so defer the physical removal to the post-commit
+                // flush compaction (rollback-safe: that rewrite is driven by the CURRENT PK, which
+                // still contains the row after a rollback).
+                Interlocked.Add(ref _pendingLogicalDeletes, recordsToDelete.Count);
+            }
+            else
+            {
+                // Durable DELETE: physically mark the removed records so a reopen skips them.
+                TombstoneDeletedPositions(positions);
+            }
+
             TryAutoCompact();
         }
 
@@ -3184,8 +3212,19 @@ this.storage is null ||
             hashIdx.RemoveBatchKeys(decoded, positions);
         }
 
+        if (this.storage is { IsInTransaction: true })
+        {
+            // Transactional delete: defer physical removal to the post-commit flush compaction
+            // (see DeleteRecordsCore — a tombstone would survive a rollback that restores the row).
+            Interlocked.Add(ref _pendingLogicalDeletes, count);
+        }
+        else
+        {
+            // Durable DELETE: physically mark the removed records so a reopen skips them.
+            TombstoneDeletedPositions(positions);
+        }
+
         Interlocked.Add(ref _cachedRowCount, -count);
-        Interlocked.Add(ref _pendingLogicalDeletes, count);
         Interlocked.Increment(ref _bulkContiguousDeleteBatches);
         return true;
     }

src/SharpCoreDB/DataStructures/Table.Compaction.cs

@@ -8,6 +8,7 @@ namespace SharpCoreDB.DataStructures;
 using SharpCoreDB.Storage.Engines;
 using System;
 using System.Collections.Generic;
+using System.IO;
 using System.Linq;
 using System.Threading.Tasks;
 
@@ -52,12 +53,14 @@ public void TryAutoCompact()
     }
 
     /// <summary>
-    /// Physically removes rows that were logically deleted since the last flush (Columnar tables
-    /// with a primary key) so DELETE survives a reopen — the on-load PK-index rebuild would otherwise
-    /// resurrect them from the untouched <c>.dat</c>. Runs synchronously at flush/dispose, outside a
-    /// transaction, when any logical deletes are pending. Data-file only: the overflow arena is left
-    /// untouched (its space is reclaimed by a later explicit compaction/VACUUM) so the flush stays
-    /// proportional to the rewritten live rows.
+    /// Makes TRANSACTIONAL deletes durable (Columnar tables with a primary key). Deletes issued
+    /// inside a transaction are intentionally not tombstoned at delete time (a marker would survive a
+    /// rollback that restores the row in the PK index), so they are physically removed here by
+    /// compacting against the CURRENT PK once the owning transaction has committed. Runs at
+    /// flush/dispose, outside a transaction, when any transactional deletes are pending. No-op when
+    /// none are pending — non-transactional DELETE is already durable via the per-row tombstone and
+    /// must not pay a rewrite. Data-file only: the overflow arena is left untouched (its space is
+    /// reclaimed by a later explicit compaction/VACUUM).
     /// </summary>
     public void CompactPendingDeletes()
     {
@@ -105,6 +108,30 @@ public void CompactPendingDeletes()
         }
     }
 
+    /// <summary>
+    /// Writes the deleted-record marker over the length prefix of each position that is physically
+    /// present in the data file, making the Columnar logical delete durable across reopen (readers
+    /// skip the marker) without rewriting the remaining rows. Rows appended inside an uncommitted
+    /// transaction (positions beyond the current file length) are skipped — their delete commits
+    /// with the rest of the transaction.
+    /// </summary>
+    private void TombstoneDeletedPositions(long[] positions)
+    {
+        if (this.storage is null || positions.Length == 0)
+        {
+            return;
+        }
+
+        long fileLength = File.Exists(DataFile) ? new FileInfo(DataFile).Length : 0;
+        foreach (var position in positions)
+        {
+            if (position >= 0 && position + 4 <= fileLength)
+            {
+                this.storage.TombstoneRecord(DataFile, position);
+            }
+        }
+    }
+
     /// <summary>
     /// Compacts the table storage by removing deleted and stale rows.
     /// Only applicable for columnar (append-only) storage mode.

src/SharpCoreDB/DataStructures/Table.ParallelScan.cs

@@ -173,6 +173,18 @@ private List<Dictionary<string, object>> ScanRowsParallel(byte[] data, string? w
                 int recordLength = System.Buffers.Binary.BinaryPrimitives.ReadInt32LittleEndian(
                     data.AsSpan(filePosition, 4));
 
+                if (recordLength < 0)
+                {
+                    int slotSize = -recordLength; // tombstoned slot: skip the full slot
+                    if (slotSize < 4)
+                    {
+                        break;
+                    }
+
+                    filePosition += slotSize;
+                    continue;
+                }
+
                 if (recordLength <= 0 || recordLength > 1_000_000_000) break;
 
                 currentRecordCount++;
@@ -207,6 +219,18 @@ private List<Dictionary<string, object>> ScanRowsParallel(byte[] data, string? w
                 int recordLength = System.Buffers.Binary.BinaryPrimitives.ReadInt32LittleEndian(
                     partitionData.Slice(localFilePosition, 4));
 
+                if (recordLength < 0)
+                {
+                    int slotSize = -recordLength; // tombstoned slot: skip the full slot
+                    if (slotSize < 4)
+                    {
+                        break;
+                    }
+
+                    localFilePosition += slotSize;
+                    continue;
+                }
+
                 if (recordLength <= 0 || recordLength > 1_000_000_000) break;
 
                 if (localFilePosition + 4 + recordLength > partitionData.Length) break;

src/SharpCoreDB/DataStructures/Table.Scanning.cs

@@ -65,10 +65,23 @@ private List<Dictionary<string, object>> ScanRowsWithSimd(byte[] data, string? w
             // ✅ C# 14: Range operator - extract length prefix span first
             var lengthSpan = dataSpan[filePosition..(filePosition + 4)];
             int recordLength = System.Buffers.Binary.BinaryPrimitives.ReadInt32LittleEndian(lengthSpan);
-            
+
+            if (recordLength < 0)
+            {
+                // Tombstoned (deleted) record: the prefix stores the negative slot size to skip.
+                int slotSize = -recordLength;
+                if (slotSize < 4)
+                {
+                    break;
+                }
+
+                filePosition += slotSize;
+                continue;
+            }
+
             // Sanity check: record length must be reasonable
             const int MaxRecordSize = 1_000_000_000; // 1 GB max per record
-            if (recordLength < 0 || recordLength > MaxRecordSize)
+            if (recordLength > MaxRecordSize)
             {
                 break;
             }

src/SharpCoreDB/DataStructures/Table.cs

@@ -733,9 +733,10 @@ public void Flush()
                 }
             }
 
-            // Durable DELETE across reopen: physically compact rows that were logically deleted
-            // since the last flush (Columnar + PK, outside a transaction). Runs after the engine
-            // and any transaction buffer have been flushed.
+            // Transactional deletes (which are intentionally NOT tombstoned at delete time so a
+            // rollback stays correct) become durable by compacting against the current PK once the
+            // owning transaction has committed. No-op when there are no deferred transactional
+            // deletes, so non-transactional DELETE keeps the O(delete) tombstone path.
             CompactPendingDeletes();
 
             // Flush indexes
@@ -783,7 +784,8 @@ protected virtual void Dispose(bool disposing)
     {
         if (disposing)
         {
-            // Durable DELETE across reopen for flows that dispose without an explicit flush.
+            // Transactional deletes deferred past their commit flush (e.g. dispose-without-flush
+            // flows) are compacted here; no-op when none are pending.
             CompactPendingDeletes();
 
             // Dispose storage engine first

src/SharpCoreDB/Interfaces/IStorage.cs

@@ -170,6 +170,15 @@ bool OverwriteRecordAtSameLength(string path, long offset, byte[] data) =>
     /// </summary>
     bool AreRecordsEncrypted(string path) => false;
 
+    /// <summary>
+    /// Marks the record whose 4-byte length prefix sits at <paramref name="offset"/> as deleted by
+    /// replacing the prefix with the NEGATIVE slot size (4-byte prefix + payload). Every record
+    /// enumerator treats a negative prefix as a deleted record and skips |value| bytes, so the
+    /// delete survives a reopen without rewriting the file. The default returns false (unsupported
+    /// layout / mock storage).
+    /// </summary>
+    bool TombstoneRecord(string path, long offset) => false;
+
     /// <summary>
     /// Enumerates every record in a table data file, yielding the literal file offset of the
     /// 4-byte length prefix (the offset returned by <see cref="AppendBytes"/>) together with the

src/SharpCoreDB/Services/Storage.Append.cs

@@ -563,6 +563,56 @@ public bool HasBufferedOverwrite(string path) =>
     /// <inheritdoc />
     public bool AreRecordsEncrypted(string path) => UseRecordEncryption && FileHasEncryptedHeader(path);
 
+    /// <inheritdoc />
+    public bool TombstoneRecord(string path, long offset)
+    {
+        // Read the current slot size so the marker can encode the exact number of bytes to skip
+        // (4-byte prefix + payload), keeping every record enumerator aligned.
+        int slotSize;
+        try
+        {
+            SafeFileHandle readHandle = GetOrOpenReadHandle(path);
+            Span<byte> lengthBuffer = stackalloc byte[4];
+            if (RandomAccess.Read(readHandle, lengthBuffer, offset) != 4)
+            {
+                return false;
+            }
+
+            int currentLength = BinaryPrimitives.ReadInt32LittleEndian(lengthBuffer);
+            if (currentLength <= 0)
+            {
+                return false; // already tombstoned or invalid
+            }
+
+            slotSize = 4 + currentLength;
+        }
+        catch (IOException)
+        {
+            return false;
+        }
+
+        Span<byte> marker = stackalloc byte[4];
+        BinaryPrimitives.WriteInt32LittleEndian(marker, -slotSize);
+
+        try
+        {
+            WriteRecordInPlace(path, offset, marker, ReadOnlySpan<byte>.Empty);
+        }
+        catch (IOException)
+        {
+            return false;
+        }
+
+        // Invalidate the app-level page cache (mirrors the other in-place writers).
+        if (this.pageCache != null)
+        {
+            int pageId = ComputePageId(path, offset);
+            this.pageCache.EvictPage(pageId);
+        }
+
+        return true;
+    }
+
     /// <inheritdoc />
     [MethodImpl(MethodImplOptions.AggressiveOptimization)]
     public long[] AppendBytesMultiple(string path, List<byte[]> dataBlocks)
@@ -924,6 +974,19 @@ private void FlushBufferedOverwrites()
             }
 
             int length = BinaryPrimitives.ReadInt32LittleEndian(lengthBuffer);
+            if (length < 0)
+            {
+                // Tombstoned (deleted) record: the prefix stores the negative slot size to skip.
+                int slotSize = -length;
+                if (slotSize < 4)
+                {
+                    yield break;
+                }
+
+                position += slotSize;
+                continue;
+            }
+
             if (length <= 0 || length > MaxRecordSize || position + 4 + length > fileLength)
             {
                 yield break; // Invalid or incomplete record tail

src/SharpCoreDB/Storage/Engines/AppendOnlyEngine.cs

@@ -214,8 +214,21 @@ public void Delete(string tableName, long storageReference)
             // Read record length (4 bytes, little-endian)
             int recordLength = System.Buffers.Binary.BinaryPrimitives.ReadInt32LittleEndian(
                 allData.AsSpan((int)position, 4));
-            
-            if (recordLength <= 0 || position + 4 + recordLength > allData.Length)
+
+            if (recordLength < 0)
+            {
+                // Tombstoned (deleted) record: the prefix stores the negative slot size to skip.
+                int slotSize = -recordLength;
+                if (slotSize < 4)
+                {
+                    break;
+                }
+
+                position += slotSize;
+                continue;
+            }
+
+            if (recordLength == 0 || position + 4 + recordLength > allData.Length)
             {
                 break; // Invalid or incomplete record
             }
@@ -354,10 +367,23 @@ public long CompactTable(string tableName, List<long> activePositions)
 
             int recordLength = System.Buffers.Binary.BinaryPrimitives.ReadInt32LittleEndian(
                 allData.AsSpan((int)position, 4));
-            
-            if (recordLength <= 0 || position + 4 + recordLength > allData.Length)
+
+            if (recordLength < 0)
+            {
+                // Tombstoned (deleted) record: the prefix stores the negative slot size to skip.
+                int slotSize = -recordLength;
+                if (slotSize < 4)
+                {
+                    break;
+                }
+
+                position += slotSize;
+                continue;
+            }
+
+            if (recordLength == 0 || position + 4 + recordLength > allData.Length)
                 break;
-            
+
             // Check if this position is active
             if (activeSet.Contains(position))
             {