Skip to content

Commit fcafdfa

Browse files
Merge pull request #366 from MPCoreDeveloper/perf/compaction-fast-durable-delete
perf: make durable-delete flush compaction proportional (data-only + single-pass index rebuild)
2 parents 80c468a + 8e0110a commit fcafdfa

2 files changed

Lines changed: 110 additions & 30 deletions

File tree

docs/CHANGELOG.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3636
- **DELETE now survives a reopen (durability)** ÔÇö Columnar deletes were logical only (index removal),
3737
so the on-load PK-index rebuild resurrected deleted rows from the untouched `.dat`. Logically
3838
deleted rows are now counted (`_pendingLogicalDeletes`) and physically compacted at flush/dispose
39-
(`Table.CompactPendingDeletes`, outside a transaction, Columnar tables with a PK). Regression:
40-
delete half the rows, `Flush`, reopen ÔÇö exactly the remaining rows come back.
39+
(`Table.CompactPendingDeletes`, outside a transaction, Columnar tables with a PK). Flush
40+
compaction rewrites only the data file (live PK positions via B-tree traversal, single-pass index
41+
rebuild) so the cost is proportional to the remaining rows (~0.4s for a 90K-live table); the
42+
overflow arena is reclaimed on the next explicit VACUUM/compaction. Regression: delete half the
43+
rows, `Flush`, reopen ÔÇö exactly the remaining rows come back. Measured DELETE in the `--pk`
44+
harness now includes this durability rewrite (~18.6K ops/s when deleting 10K of 100K rows).
4145
- **Dedicated SQL batch-INSERT fast path (WP14)** ÔÇö `ExecuteBatchSQL` INSERTs no longer build a
4246
per-row `Dictionary<string, object>`; VALUES clauses are parsed directly into column-ordered
4347
`object[]` rows (`PreparedInsertStatement.ParseValuesToArray`) and inserted via the new

src/SharpCoreDB/DataStructures/Table.Compaction.cs

Lines changed: 104 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,9 @@ public void TryAutoCompact()
5555
/// Physically removes rows that were logically deleted since the last flush (Columnar tables
5656
/// with a primary key) so DELETE survives a reopen — the on-load PK-index rebuild would otherwise
5757
/// resurrect them from the untouched <c>.dat</c>. Runs synchronously at flush/dispose, outside a
58-
/// transaction, when any logical deletes are pending.
58+
/// transaction, when any logical deletes are pending. Data-file only: the overflow arena is left
59+
/// untouched (its space is reclaimed by a later explicit compaction/VACUUM) so the flush stays
60+
/// proportional to the rewritten live rows.
5961
/// </summary>
6062
public void CompactPendingDeletes()
6163
{
@@ -71,13 +73,35 @@ public void CompactPendingDeletes()
7173
return; // defer until the transaction commits (the next flush will run again)
7274
}
7375

76+
rwLock.EnterWriteLock();
7477
try
7578
{
76-
CompactStorage();
79+
var engine = GetOrCreateStorageEngine();
80+
if (engine is not AppendOnlyEngine appendEngine)
81+
{
82+
return;
83+
}
84+
85+
var activePositions = new List<long>();
86+
if (this.Index is BTree<string, long> pkTree)
87+
{
88+
foreach (var (_, position) in pkTree.InOrderTraversal())
89+
{
90+
activePositions.Add(position);
91+
}
92+
}
93+
else
94+
{
95+
return; // no enumerable PK tree — cannot rewrite safely; keep logical deletes
96+
}
97+
98+
appendEngine.CompactTable(Name, activePositions);
99+
RebuildAllIndexesFromFile();
77100
}
78101
finally
79102
{
80103
Interlocked.Exchange(ref _pendingLogicalDeletes, 0);
104+
rwLock.ExitWriteLock();
81105
}
82106
}
83107

@@ -108,24 +132,30 @@ public CompactionStats CompactStorage()
108132

109133
if (PrimaryKeyIndex >= 0)
110134
{
111-
// Collect all positions from primary key index.
112-
// ✅ FIX (1.9.5): Include the hidden _rowid column when present — otherwise rows in
113-
// tables with an internal ULID primary key cannot be resolved to storage positions
114-
// and compaction would drop every row.
115-
var pkColumn = Columns[PrimaryKeyIndex];
116-
var allRows = HasInternalRowId
117-
? SelectIncludingRowId(where: null, orderBy: null, asc: true, noEncrypt: false)
118-
: Select();
119-
120-
foreach (var row in allRows)
135+
if (this.Index is BTree<string, long> pkTree)
136+
{
137+
// Collect the live (key → position) pairs straight from the PK B-tree — no row
138+
// materialization and no per-row re-search. Covers the hidden _rowid PK too,
139+
// because every live row has an entry in the tree.
140+
foreach (var (_, position) in pkTree.InOrderTraversal())
141+
{
142+
activePositions.Add(position);
143+
}
144+
}
145+
else
121146
{
122-
if (row.TryGetValue(pkColumn, out var pkValue) && pkValue != null)
147+
// Non-BTree index fallback: resolve positions through the current rows.
148+
var pkColumn = Columns[PrimaryKeyIndex];
149+
var allRows = SelectIncludingRowId(where: null, orderBy: null, asc: true, noEncrypt: false);
150+
foreach (var row in allRows)
123151
{
124-
var pkStr = pkValue.ToString() ?? string.Empty;
125-
var searchResult = Index.Search(pkStr);
126-
if (searchResult.Found)
152+
if (row.TryGetValue(pkColumn, out var pkValue) && pkValue != null)
127153
{
128-
activePositions.Add(searchResult.Value);
154+
var searchResult = Index.Search(pkValue.ToString() ?? string.Empty);
155+
if (searchResult.Found)
156+
{
157+
activePositions.Add(searchResult.Value);
158+
}
129159
}
130160
}
131161
}
@@ -150,17 +180,13 @@ public CompactionStats CompactStorage()
150180
// Reset counters
151181
Interlocked.Exchange(ref _deletedRowCount, 0);
152182
Interlocked.Exchange(ref _updatedRowCount, 0);
153-
154-
// Rebuild primary key index with new positions
155-
// Note: After compaction, positions change! We need to rebuild the index.
156-
RebuildPrimaryKeyIndex();
157-
158-
// Rebuild hash indexes
159-
foreach (var col in loadedIndexes.ToList())
160-
{
161-
RebuildHashIndex(col);
162-
}
163-
183+
Interlocked.Exchange(ref _pendingLogicalDeletes, 0);
184+
185+
// Rebuild the PK B-tree and every loaded hash index in ONE file pass (positions change
186+
// after compaction; a per-index rescan would re-read + re-decode the whole file once
187+
// per index, which is pathological on large tables).
188+
RebuildAllIndexesFromFile();
189+
164190
return new CompactionStats
165191
{
166192
BytesReclaimed = bytesReclaimed,
@@ -235,6 +261,56 @@ private static void CollectVariableOffsets(byte[] record, FixedWidthRecordLayout
235261
private static byte[]? RepointVariableSlots(byte[] record, FixedWidthRecordLayout layout, Dictionary<long, long> mapping)
236262
=> FixedWidthCodec.RepointVariableSlots(record, layout, mapping);
237263

264+
/// <summary>
265+
/// Rebuilds the PK B-tree and every loaded hash index from the rewritten data file in ONE pass:
266+
/// each record is read and decoded once and feeds all indexes, instead of rescanning the whole
267+
/// file (with full deserialization, including overflow-arena reads) once per index.
268+
/// </summary>
269+
private void RebuildAllIndexesFromFile()
270+
{
271+
var engine = GetOrCreateStorageEngine();
272+
273+
if (PrimaryKeyIndex >= 0)
274+
{
275+
Index = new BTree<string, long>();
276+
}
277+
278+
foreach (var hashIndex in hashIndexes.Values)
279+
{
280+
hashIndex.Clear();
281+
}
282+
283+
var loadedHashIndexes = new List<HashIndex>();
284+
foreach (var kvp in hashIndexes)
285+
{
286+
if (loadedIndexes.Contains(kvp.Key))
287+
{
288+
loadedHashIndexes.Add(kvp.Value);
289+
}
290+
}
291+
292+
foreach (var (position, data) in engine.GetAllRecords(Name))
293+
{
294+
var row = DeserializeRow(data);
295+
if (row is null)
296+
{
297+
continue;
298+
}
299+
300+
if (PrimaryKeyIndex >= 0 &&
301+
row.TryGetValue(Columns[PrimaryKeyIndex], out var pkValue) &&
302+
pkValue != null)
303+
{
304+
Index.Insert(pkValue.ToString() ?? string.Empty, position);
305+
}
306+
307+
foreach (var hashIndex in loadedHashIndexes)
308+
{
309+
hashIndex.Add(row, position);
310+
}
311+
}
312+
}
313+
238314
/// <summary>
239315
/// Rebuilds the primary key index after compaction.
240316
/// Positions change after compaction, so we need to rescan the file.

0 commit comments

Comments
 (0)