From fbbda717edd88b85d0cbd43e227c079db017c719 Mon Sep 17 00:00:00 2001 From: MPCoreDeveloper Date: Thu, 3 Sep 2026 20:18:00 +0200 Subject: [PATCH] perf: apply DELETE tombstones at COMMIT for transactional deletes (no flush rewrite) Stap-0 profiling showed batch DELETE (ExecuteBatchSQL wraps every batch in a storage transaction) was NOT using the tombstone path: the rollback-safe deferral only counted _pendingLogicalDeletes, so db.Flush() still ran the #366 full-file CompactPendingDeletes rewrite (~0.5-0.7s; DELETE stuck at ~12-16K ops/s). - IStorage.BufferTombstoneForCommit (default no-op) + Storage implementation: buffered offsets per file, applied as in-place negative-prefix markers by ApplyBufferedTombstones() inside FlushBufferedAppendsAndOverwrites() (the commit path) AFTER buffered appends are on disk; rollback discards the buffer via ClearBufferedAppends(). - DeleteRecordsCore + the contiguous FW bulk path now buffer the deleted offsets when IsInTransaction instead of incrementing _pendingLogicalDeletes, so the flush-time full-file rewrite is off the batch-DELETE path entirely. - Regression: batch DELETE survives reopen even WITHOUT an explicit Flush after ExecuteBatchSQL (commit already tombstoned the rows). Measured (same machine, Release, comparative harness DELETE 10K of ~100K rows): SQL 0.82s/12K -> 0.24s/41K ops/s, Direct 0.63s/16K -> 0.17s/58K ops/s; --pk legacy 0.16s/62K, fixed-width 0.13s/78K ops/s. Full suite EXIT=0. --- docs/CHANGELOG.md | 24 ++++--- src/SharpCoreDB/DataStructures/Table.CRUD.cs | 27 +++++--- src/SharpCoreDB/Interfaces/IStorage.cs | 10 +++ src/SharpCoreDB/Services/Storage.Append.cs | 69 +++++++++++++++++++ .../FixedWidthBulkDeleteTests.cs | 24 ++++++- 5 files changed, 134 insertions(+), 20 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 8f0003ce..2046828d 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -42,17 +42,19 @@ 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. +- **Durable DELETE is now in-place via tombstones (no rewrite)** ÔÇö 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). Non-transactional deletes write the marker at + delete time; transactional deletes (e.g. any `ExecuteBatchSQL` batch, which runs inside a storage + transaction) buffer the offsets and apply the markers at COMMIT ÔÇö rollback discards the buffer, + so a rolled-back delete keeps its row, and the flush-time full-file rewrite (`CompactPendingDeletes`) + is no longer on the batch-DELETE path. 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.13s/~78K ops/s (fixed-width)**; comparative-harness DELETE (docs table): SQL ~0.82s/~12K ÔåÆ + **~0.24s/~41K ops/s**, Direct ~0.63s/~16K ÔåÆ **~0.17s/~58K ops/s** ÔÇö DELETE is back on par with UPDATE. - **Dedicated SQL batch-INSERT fast path (WP14)** ÔÇö `ExecuteBatchSQL` INSERTs no longer build a per-row `Dictionary`; VALUES clauses are parsed directly into column-ordered `object[]` rows (`PreparedInsertStatement.ParseValuesToArray`) and inserted via the new diff --git a/src/SharpCoreDB/DataStructures/Table.CRUD.cs b/src/SharpCoreDB/DataStructures/Table.CRUD.cs index 24c44b62..73929778 100644 --- a/src/SharpCoreDB/DataStructures/Table.CRUD.cs +++ b/src/SharpCoreDB/DataStructures/Table.CRUD.cs @@ -2655,11 +2655,16 @@ 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); + // Transactional delete: buffer the physical offsets so the in-place marker is + // applied at COMMIT (rollback discards the buffer). Durable in O(delete) — the + // flush-time full-file rewrite is no longer needed for transactional deletes. + foreach (var position in positions) + { + if (position >= 0) + { + this.storage.BufferTombstoneForCommit(DataFile, position); + } + } } else { @@ -3214,9 +3219,15 @@ this.storage is null || 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); + // Transactional delete: buffer the physical offsets so the in-place marker is applied + // at COMMIT (see DeleteRecordsCore — rollback discards the buffer). + foreach (var position in positions) + { + if (position >= 0) + { + this.storage.BufferTombstoneForCommit(DataFile, position); + } + } } else { diff --git a/src/SharpCoreDB/Interfaces/IStorage.cs b/src/SharpCoreDB/Interfaces/IStorage.cs index 3aecaf81..3e519e36 100644 --- a/src/SharpCoreDB/Interfaces/IStorage.cs +++ b/src/SharpCoreDB/Interfaces/IStorage.cs @@ -179,6 +179,16 @@ bool OverwriteRecordAtSameLength(string path, long offset, byte[] data) => /// bool TombstoneRecord(string path, long offset) => false; + /// + /// Registers a record position to be tombstoned when the CURRENT transaction commits. Deletes + /// issued inside a transaction must not write the marker at delete time (it would survive a + /// rollback that restores the row), so callers buffer the physical offset here and the storage + /// layer applies the in-place marker once the owning transaction commits — durable in O(delete) + /// without the flush-time full-file rewrite. On rollback the buffered offsets are discarded. + /// The default is a no-op (unsupported layout / mock storage). + /// + void BufferTombstoneForCommit(string path, long offset) { } + /// /// Enumerates every record in a table data file, yielding the literal file offset of the /// 4-byte length prefix (the offset returned by ) together with the diff --git a/src/SharpCoreDB/Services/Storage.Append.cs b/src/SharpCoreDB/Services/Storage.Append.cs index 4693d029..a0ac204b 100644 --- a/src/SharpCoreDB/Services/Storage.Append.cs +++ b/src/SharpCoreDB/Services/Storage.Append.cs @@ -49,6 +49,12 @@ public partial class Storage // append because OverwriteRecordAt refused to write inside a transaction. private readonly ConcurrentDictionary> bufferedOverwrites = new(StringComparer.Ordinal); + // ✅ Commit-time tombstones: physical offsets of records deleted inside the current + // transaction. The marker is NOT written at delete time (a rollback must keep the row); + // ApplyBufferedTombstones writes the in-place negative-prefix markers when the transaction + // commits, after the buffered appends are on disk. Rollback discards the buffer. + private readonly Dictionary> bufferedTombstones = new(StringComparer.Ordinal); + // Base file length captured at the first buffered operation of the transaction. In-place // overwrites are only safe below this boundary (records already flushed to disk); offsets // at or above it belong to still-buffered appends and must fall back to append. @@ -563,6 +569,67 @@ public bool HasBufferedOverwrite(string path) => /// public bool AreRecordsEncrypted(string path) => UseRecordEncryption && FileHasEncryptedHeader(path); + /// + public void BufferTombstoneForCommit(string path, long offset) + { + if (offset < 0) + { + return; + } + + lock (appendLock) + { + if (!bufferedTombstones.TryGetValue(path, out var list)) + { + list = new List(); + bufferedTombstones[path] = list; + } + + list.Add(offset); + } + } + + /// + /// Applies every buffered commit-time tombstone as an in-place negative-prefix marker. Runs + /// from (the commit path) AFTER the buffered + /// appends are on disk, so offsets of rows that were appended AND deleted in the same + /// transaction are valid. Rollback discards the buffer instead (). + /// + private void ApplyBufferedTombstones() + { + if (bufferedTombstones.Count == 0) + { + return; + } + + foreach (var (path, offsets) in bufferedTombstones) + { + if (offsets.Count == 0) + { + continue; + } + + try + { + long fileLength = File.Exists(path) ? new FileInfo(path).Length : 0; + foreach (var offset in offsets) + { + if (offset >= 0 && offset + 4 <= fileLength) + { + TombstoneRecord(path, offset); + } + } + } + catch (IOException) + { + // Best-effort: a failed marker leaves the row physical; the auto/explicit + // compaction reclaims it later. + } + } + + bufferedTombstones.Clear(); + } + /// public bool TombstoneRecord(string path, long offset) { @@ -746,6 +813,7 @@ internal void FlushBufferedAppendsAndOverwrites() { FlushBufferedAppends(); FlushBufferedOverwrites(); + ApplyBufferedTombstones(); } } @@ -822,6 +890,7 @@ internal void ClearBufferedAppends() cachedFileLengths.Clear(); // ✅ Clear cache too headerPendingFiles.Clear(); // ✅ Clear pending header markers on rollback bufferedFileBaseLengths.Clear(); + bufferedTombstones.Clear(); // Rollback: discard pending commit-time tombstones } } diff --git a/tests/SharpCoreDB.Tests/FixedWidthBulkDeleteTests.cs b/tests/SharpCoreDB.Tests/FixedWidthBulkDeleteTests.cs index 2ae0b69f..aff57eeb 100644 --- a/tests/SharpCoreDB.Tests/FixedWidthBulkDeleteTests.cs +++ b/tests/SharpCoreDB.Tests/FixedWidthBulkDeleteTests.cs @@ -218,7 +218,7 @@ public void DeletesPersistAcrossReopen_AfterFlushCompaction() } db.ExecuteBatchSQL(stmts); - db.Flush(); // flush-time compaction must physically remove the deleted rows + db.Flush(); // commit-time tombstones (or legacy flush compaction) remove the rows physically (db as IDisposable)?.Dispose(); db = CreateDb(); @@ -230,4 +230,26 @@ public void DeletesPersistAcrossReopen_AfterFlushCompaction() Assert.Equal(50L, Convert.ToInt64(c[0].Values.First())); (db as IDisposable)?.Dispose(); } + + [Fact] + public void BatchDelete_CommitTimeTombstones_SurviveReopenWithoutExplicitFlush() + { + // ExecuteBatchSQL wraps the whole DELETE batch in one storage transaction. The deleted + // positions must be tombstoned AT COMMIT (not at a later db.Flush()), so durability must + // hold even when the database is disposed without an explicit Flush afterwards. + IDatabase? db = CreateDb(); + db.ExecuteSQL("CREATE TABLE docs (id INTEGER PRIMARY KEY, name TEXT, score REAL)"); + InsertDocs(db, 1, 200); + db.Flush(); + + db.ExecuteBatchSQL(BuildDeletes(1, 100)); // commits inside ExecuteBatchSQL + (db as IDisposable)?.Dispose(); // no explicit Flush after the delete batch + + db = CreateDb(); + var scan = db.ExecuteQuery("SELECT id FROM docs ORDER BY id"); + Assert.Equal(100, scan.Count); + Assert.Equal(101L, Convert.ToInt64(scan[0]["id"])); + Assert.Equal(200L, Convert.ToInt64(scan[^1]["id"])); + (db as IDisposable)?.Dispose(); + } }