Skip to content

Commit 4ff03bc

Browse files
feat(storage): opt-in buffered appends - 25x single-row INSERTs, plus a default-config DROP TABLE fix
Owner decision (option A): DatabaseConfig.EnableBufferedAppends (default FALSE) routes single-row appends through the in-memory buffer the transaction path already uses, so N rows cost ONE open/write/close per flush boundary instead of a write-through FileStream open/close PER ROW. The default configuration stays byte-for-byte unchanged, and a test pins exactly that. Measured end-to-end (2000 single-row INSERT statements via ExecuteSQL, Release build): raw off 732 -> on 18,775 ops/s (25.6x) at-rest off 974 -> on 23,822 ops/s (24.4x) The residual is the SQL/engine path (~50us/row: parse, plan-cache lookup, index updates), not the I/O - the 0.43us/record storage ceiling still needs a batch API. Reported honestly rather than as the 100x+ the storage-level ratio suggests. Correctness is enforced by two mechanisms, each with tests in the new BufferedAppendTests (14 tests): - overlay: ReadBytesFrom resolves a buffered position from a position->payload index (the shape that already existed for buffered overwrites) and ReadAllRecords appends the buffered tail in offset order, including a file that does not exist yet because its first rows are still buffered; - flush-first at every boundary that replaces or deletes the file: Database.Flush(), commit, BeginTransaction (Rollback clears the buffer and would otherwise discard rows the caller already has), compaction, fixed-width migration, overflow-arena compaction, DROP TABLE (otherwise the pending flush recreates the file the user just dropped), dispose. The fixed-width bulk fast paths bail out exactly like the existing HasBufferedOverwrite gates. Threshold/age bound the durability window: AppendBufferFlushThresholdBytes (1 MB), AppendBufferFlushIntervalMs (10 ms); the age bound is "flushed by the next append", so durability never depends on a timer thread. Also found and fixed a PRE-EXISTING default-configuration bug, unrelated to buffering: CREATE TABLE; INSERT; DROP TABLE failed on Windows with "file is being used by another process" whenever at-rest encryption was on (the default) and at least one row had been inserted, while NoEncryptMode=true passed the same sequence. The DROP path probed the data file with FileShare.None, and an exclusive open is refused by ANY live handle - including the read handle SharpCoreDB caches for at-rest records - even though the File.Delete that follows succeeds with those handles present. The probe now asks for the sharing a delete actually needs (FileShare.ReadWrite|FileShare.Delete); genuine locks still throw and are retried by the existing backoff. Identical fix in the CREATE-path probe. The once-per-file write-decision header probe also moved to a short-lived open, so the first INSERT no longer pins a read handle on the data file for the lifetime of the database. Gate: full SharpCoreDB.Tests 1848 total / 0 failed / 16 skipped (1834 + 14 new); SharpCoreDB.slnx 0 errors. Docs: plan section 5 (implementation status, the audit matrix, the measured table, the honest residual, the DDL finding) and CHANGELOG (Added: the option; Fixed: the DROP TABLE bug and the pinned read handle).
1 parent 461882c commit 4ff03bc

13 files changed

Lines changed: 769 additions & 31 deletions

File tree

docs/CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1515
- **Tuning knobs**: `MaxNeighbors` (R), `ConstructionSearchListSize` (L_build), `QuerySearchListSize` (L_search floor), `Alpha`, `BuildPasses`, plus a per-query beam override `Search(query, k, searchListSize)` so recall/latency can be traded per query without rebuilding the graph.
1616
- **SQL DDL**: `CREATE VECTOR INDEX … USING DISKANN` is recognised by the parser, stored as table metadata, and now builds a real `DiskAnnIndex` through the optimiser (see Fixed below).
1717

18+
### Added (INSERT performance — opt-in buffered appends)
19+
20+
- **`DatabaseConfig.EnableBufferedAppends` (default `false`)** — single-row `INSERT`s can now share the in-memory append buffer the transaction path already uses, so N rows cost **one** open/write/close per flush boundary instead of a write-through `FileStream` open/close *per row* (measured ~512 µs → ~4.5 µs per 64-byte record for the write itself). `AppendBufferFlushThresholdBytes` (default 1 MB) and `AppendBufferFlushIntervalMs` (default 10 ms) bound the durability window and the memory footprint. **Measured end-to-end** (2000 single-row `INSERT` statements through `ExecuteSQL`, Release build): raw **732 → 18,775 INSERT/s (25.6×)**, default encrypted posture **974 → 23,822 INSERT/s (24.4×)**. The residual is the SQL/engine path (~50 µs/row), not the I/O — the 0.43 µs/record storage ceiling still needs a batch API.
21+
- **Correctness of buffered rows is explicit, not assumed** — a buffered row is visible immediately: point lookups overlay a `position → payload` index over the buffer (the shape that already existed for buffered *overwrites*) and `ReadAllRecords` appends the buffered tail in offset order (also covering a file that does not exist yet because its first rows are still buffered). Every structural boundary flushes first — `Database.Flush()`, commit, `BeginTransaction`, compaction, fixed-width migration, overflow-arena compaction, `DROP TABLE`, dispose — because those operations replace or delete the file the buffered offsets were computed against. Guarded by `BufferedAppendTests` (14 tests: read-your-writes, threshold auto-flush, flush + reopen, rollback keeps pre-transaction rows, compaction keeps every row exactly once, `DROP TABLE` does not recreate the file, and a test that pins the default configuration as unbuffered).
22+
1823
### Changed
1924

2025
- **Table data is now encrypted at rest by default.** `DatabaseConfig.EnableAtRestRecordEncryption` defaults to `true`, so a new database protects its table payloads — records *and* the overflow arena — with per-record AES-256-GCM, alongside the metadata and transaction files that were already encrypted. Previously the default encrypted the metadata while leaving the user's data on disk in the clear: the cost of protection without the guarantee. `NoEncryptMode = true` stays the single, documented raw-speed opt-out (every file plaintext). Measured cost of the default versus that opt-out: ≈1.11× CREATE/INSERT, no measurable UPDATE penalty on the contiguous paths, roughly double the file size for the per-record GCM framing, and one whole-file decrypt per full-scan-shaped query. Existing plaintext databases remain byte-for-byte readable and are never mixed with encrypted records; their tables are upgraded to the encrypted format when they are compacted. Guarded by `EncryptionCoverageTests`, which fails if the default ever stops protecting table payloads.
2126

2227
### Fixed
2328

29+
- **`CREATE TABLE; INSERT; DROP TABLE` failed on Windows in the default configuration** — with at-rest encryption enabled (the default), dropping a table that had received at least one row threw `IOException: The process cannot access the file ... because it is being used by another process`, while the identical sequence passed with `NoEncryptMode = true`. The DROP path validated the data file by opening it with `FileShare.None`, and on Windows an exclusive open is refused by *any* live handle — including the read handle SharpCoreDB itself caches for at-rest record files — even though the `File.Delete` that follows succeeds with those handles present. The probe now asks for the sharing a delete actually needs (`FileShare.ReadWrite | FileShare.Delete`), so genuine locks still throw and are retried by the existing backoff loop; the identical probe in the CREATE path is fixed the same way.
30+
- **The first `INSERT` pinned a read handle on the data file for the lifetime of the database** — the once-per-file write decision (does this file already carry the encrypted magic header?) used the *per-read* cached-handle probe, so an at-rest database kept a live read handle on every table it had inserted into, which also blocked the DDL exclusive-open probe above. The decision — memoised per path, so it is not a hot path — now opens the file briefly and closes it, while the per-record read probe keeps its cache.
2431
- **An at-rest database could not be scanned** — with `EnableAtRestRecordEncryption = true` a whole-table scan (and `COUNT(*)`) returned **zero rows**, in-session and after a reopen, while primary-key lookups kept working. The scan compared each record's offset in its decrypted buffer against the PK index's physical file offsets, so every row was misread as a superseded version; the parallel scan and the `StructRow` scan had the same shape, and the hash indexes — rebuilt from that scan — came back empty after a reopen. Scans now receive the records' physical offsets, so scans, counts and index rebuilds are correct on encrypted files.
2532
- **`WHERE <numeric column> = <literal with decimals>` matched nothing** — a simple numeric equality compared the row value's *text* against the literal, so `score = 5.0` could never match a stored 5.0 (`double.ToString()` yields `"5"`) while `score = 5` matched by accident, and the ordering operators parsed literals with the machine's culture (a decimal literal did not even parse under a comma-decimal culture). Numbers are now compared numerically against an invariant-culture parse of the literal; string comparisons are unchanged.
2633
- **A hash index built on a fixed-width table missed rows**`CREATE TABLE` registers a hash index for every column, and the lazy build decoded fixed-width records with the variable-length parser, so on a default table only the rows that happened to parse were indexed: `WHERE <non-unique column> = value` returned **a single row instead of every match**, and the build stopped at the first tombstone, hiding every live row behind a deleted one. The build now decodes with the layout the records were written in and skips tombstoned and empty slots.

docs/performance/INSERT_UPDATE_PERFORMANCE_PLAN.md

Lines changed: 59 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -762,7 +762,7 @@ Because "found by accident" is not a method, the remaining sites were then audit
762762

763763
| site | frequency | verdict |
764764
|---|---|---|
765-
| `AppendBytes` (`FileMode.Append` + `FileOptions.WriteThrough`) | **once per single-row INSERT** | **hot — and a durability choice; moved to §5 as the first Phase 3 item with its measurement** |
765+
| `AppendBytes` (`FileMode.Append` + `FileOptions.WriteThrough`) | **once per single-row INSERT** | **resolved in §5: opt-in buffered appends (`EnableBufferedAppends`, default off) replace the per-row open with one flush per boundary — measured 25× end-to-end (732 → 18,775 INSERT/s)** |
766766
| `AppendBytesMultiple` | once per *batch* | fine (65 KB buffer, one write-through for the batch) |
767767
| `FlushBufferedAppends` | once per file per commit | fine |
768768
| `TryUpdateInPlaceSameLength`, `ReadBytesFrom`, `ReadBytesRange` | per record | fine — cached read handle already |
@@ -838,6 +838,33 @@ answered explicitly — a process crash is already safe (the OS cache survives i
838838
open question and is what the WAL/`WalDurabilityMode` settings exist to answer. Because it changes when
839839
bytes reach the platter, it needs the owner's call and a crash-recovery test, not a unilateral edit.
840840

841+
**Decision taken (owner, option A): implemented — opt-in, off by default.** `DatabaseConfig.EnableBufferedAppends`
842+
routes single-row appends through the append buffer the transaction path already uses;
843+
`AppendBufferFlushThresholdBytes` (default 1 MB) and `AppendBufferFlushIntervalMs` (default 10 ms) bound the
844+
window, and every structural boundary flushes first: `Database.Flush()`, commit, `BeginTransaction`,
845+
compaction, fixed-width migration, overflow-arena compaction, `DROP TABLE`, dispose. Option (b) — letting
846+
`WalDurabilityMode` govern the table append — was **not** taken, because the project's own benchmark doc
847+
establishes that the mode is only honoured by `GroupCommitWal` (default off): it would have made the data file
848+
promise something the WAL itself does not.
849+
850+
MEASURED end-to-end (2000 single-row `INSERT` statements through `ExecuteSQL`, Release build):
851+
852+
| config | single-row INSERT ops/s | factor |
853+
|---|---:|---:|
854+
| `NoEncryptMode=true`, buffering off | 732 ||
855+
| `NoEncryptMode=true`, buffering on | **18,775** | **25.6×** |
856+
| encrypted (default posture), buffering off | 974 ||
857+
| encrypted (default posture), buffering on | **23,822** | **24.4×** |
858+
859+
Honest residual: that is ~25×, not the 100×+ the storage-level ratio (477.97 µs → 0.43 µs/record) suggests,
860+
because the SQL/engine path (~50 µs/row: statement parse, plan-cache lookup, index updates) becomes the floor
861+
once the ~500 µs append is gone. The 0.43 µs/row ceiling stays reachable only through a batch API; lifting the
862+
single-statement path past ~20K rows/s is a separate item (prepared statements / statement cache), not part of
863+
this change.
864+
865+
The default configuration is byte-for-byte unchanged, which
866+
`BufferedAppendTests.DefaultConfig_DoesNotBuffer_FirstInsertIsAlreadyOnDisk` pins.
867+
841868
**The "just cache the handle" fix was tried, measured and reverted — it breaks readers.** The obvious way
842869
to remove the open cost without touching durability is a persistent write-through handle. It works
843870
(512 µs → 264 µs for a 64-byte record with `FileOptions.WriteThrough` unchanged) — but a **live write
@@ -855,19 +882,37 @@ append path opens per call: it is a constraint to design around, not a cost to o
855882
| cached stream + `Flush(flushToDisk: true)` per record | 514 | per record (fsync) | yes | ✗ blocks readers |
856883
| cached buffered stream + `WriteThrough` | **4.5** | per 4 KB, not per record | yes | ✗ blocks readers *and* changes durability |
857884

858-
**So the only remaining levers are (a) buffer appends and flush at a boundary — which needs the read path
859-
to see buffered appends (read-your-writes), i.e. real work — or (b) let the existing `WalDurabilityMode`
860-
govern the table append, which is the owner's decision.** Both need a crash-recovery test; neither should
861-
be a config flip. The room is large — 512 µs → 4.5 µs per row — but it is bought with durability, and that
862-
is not a trade to make silently on a database.
863-
864-
The file-sharing constraint narrows the shape of (a): a *long-lived buffered stream* has the same problem
865-
as the cached handle, so the viable form is **buffer the records in memory and flush with a single
866-
open/write/close at the boundary** — exactly the shape the transaction path already uses
867-
(`bufferedAppends` + `FlushBufferedAppends`), where the handle is only live during the flush and readers
868-
are unaffected. The real work in (a) is therefore not the buffering, which exists, but read-your-writes:
869-
`ReadBytesFrom` does not consult buffered appends, so a row appended in buffered mode is invisible until
870-
the flush. That is the piece to design (and to decide on) before this item can move.
885+
**What makes it safe.** The invariant: every position the engine handed out must be visible to every reader,
886+
and no structural operation may silently lose or duplicate it. Two mechanisms enforce it — an overlay for
887+
point reads, and a flush-first hook for everything that rewrites or replaces a file:
888+
889+
| # | entry point | handling |
890+
|---|---|---|
891+
| 1 | `ReadBytesFrom` (PK point lookup) | overlay from a `position → payload` index over the buffer — the same shape that already existed for buffered *overwrites* |
892+
| 2 | `ReadAllRecords` (scans, index rebuild, arena reload) | on-disk walk + the buffered tail in offset order; also covers a file that does not exist yet because its first rows are still buffered |
893+
| 3 | `ReadBytesWithRecordOffsets` (whole-file snapshots) | flush first when outside a transaction (inside one the buffer belongs to the transaction and flushing it would break rollback) |
894+
| 4 | fixed-width bulk UPDATE/DELETE fast paths | bail to the safe per-record path, exactly like the existing `HasBufferedOverwrite` gate |
895+
| 5 | compaction, fixed-width migration, overflow-arena compaction | flush first — these REPLACE the file, so a later flush would append the same rows a second time |
896+
| 6 | `BeginTransaction` | flush first — `Rollback()` clears the buffer and would otherwise discard rows the caller already has |
897+
| 7 | `DROP TABLE` | flush first — otherwise the pending flush recreates the file the user just deleted |
898+
| 8 | threshold / age / `Database.Flush()` / dispose | the durability boundary; the age bound is "flushed by the next append", so the explicit boundaries never depend on a timer |
899+
900+
Rows 1–2 and 5–8 have dedicated tests in `BufferedAppendTests`; rows 3–4 are guarded by paths the existing
901+
suite already exercises (every full-scan test goes through `ReadBytesWithRecordOffsets`, the fixed-width bulk
902+
update/delete tests through the gates), so they are listed here instead of assumed.
903+
904+
**This work also found a pre-existing default-configuration bug, unrelated to buffering.**
905+
`CREATE TABLE; INSERT; DROP TABLE` failed on Windows with
906+
`IOException: The process cannot access the file ... because it is being used by another process` in the
907+
**default** configuration (at-rest encryption on), while the identical sequence passed with
908+
`NoEncryptMode=true`. The DROP path validated the data file by opening it with `FileShare.None`; on Windows an
909+
exclusive open is refused by *any* live handle — including the read handle SharpCoreDB caches for at-rest
910+
records — even though the `File.Delete` that follows succeeds with those handles present. The probe now asks
911+
for the sharing a delete actually needs (`FileShare.ReadWrite | FileShare.Delete`), so genuine locks still
912+
throw and are retried by the existing backoff loop, while a cached read handle no longer blocks DDL. Fixed in
913+
both the DROP TABLE path and the identical probe in the CREATE path. (The write-decision header probe was also
914+
moved to a short-lived open, so the first INSERT no longer pins a read handle on the data file for the
915+
lifetime of the database.)
871916

872917
INSERT is otherwise already 73.5–84.3K (SQL) / 108.5–132.1K (Direct) / 125.8–138.4K (StructRow) against
873918
SQLite's 133.7–145.1K, and WP14's batch fast path already bought +80%. The remaining, ranked items:

src/SharpCoreDB/DataStructures/OverflowArena.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,11 @@ public Dictionary<long, long> Compact(IReadOnlyCollection<long> activeOffsets)
205205
}
206206
}
207207

208+
// ✅ Buffered append mode: the arena file below is DELETED and the temp file moved over it, so
209+
// the temp blocks must be on disk first (a buffered append would move nothing and then flush
210+
// into the stale path). (No-op by default.)
211+
_storage.FlushPendingAppends();
212+
208213
if (File.Exists(_filePath))
209214
{
210215
File.Delete(_filePath);

src/SharpCoreDB/DataStructures/Table.CRUD.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2461,7 +2461,8 @@ private bool TryBulkUpdateContiguousFixedWidth(
24612461
this.TableCheckConstraints.Count > 0 ||
24622462
HasColumnCheckConstraints() ||
24632463
this.storage is null ||
2464-
this.storage.HasBufferedOverwrite(DataFile))
2464+
this.storage.HasBufferedOverwrite(DataFile) ||
2465+
this.storage.HasBufferedAppends(DataFile))
24652466
{
24662467
return false;
24672468
}
@@ -3996,7 +3997,8 @@ private bool TryBulkDeleteContiguousFixedWidth(List<string> whereConditions)
39963997
StorageMode != StorageMode.Columnar ||
39973998
this.PrimaryKeyIndex < 0 ||
39983999
this.storage is null ||
3999-
this.storage.HasBufferedOverwrite(DataFile))
4000+
this.storage.HasBufferedOverwrite(DataFile) ||
4001+
this.storage.HasBufferedAppends(DataFile))
40004002
{
40014003
return false;
40024004
}

src/SharpCoreDB/DataStructures/Table.FixedWidthMigration.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,11 @@ private void WriteMigratedRecordsAndSwap(string tempPath, List<byte[]> records)
133133
if (records.Count > 0)
134134
{
135135
storage.AppendBytesMultiple(tempPath, records);
136+
137+
// ✅ Buffered append mode: the swap below moves the temp file over the data file, so the
138+
// migrated records must be on disk first (a buffered append would leave the temp file
139+
// missing and flush into the moved-away path later).
140+
storage.FlushPendingAppends();
136141
}
137142
else
138143
{

src/SharpCoreDB/DatabaseConfig.cs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,52 @@ public class DatabaseConfig
7777
/// </summary>
7878
public bool EnableAtRestRecordEncryption { get; init; } = true;
7979

80+
/// <summary>
81+
/// <para>
82+
/// Opt-in buffered append mode for the single-row INSERT path. When <c>true</c>, a row appended
83+
/// outside a transaction is buffered in memory with the rest of the pending appends and written
84+
/// with ONE open/write/close when a flush boundary is reached — instead of a
85+
/// <see cref="System.IO.FileStream"/> open, write-through and close per row.
86+
/// </para>
87+
/// <para>
88+
/// MEASURED: the per-row open dominates this path. A 64-byte record costs ~512 µs through the
89+
/// write-through open/close, of which only ~4.5 µs is the actual buffered write; the same records
90+
/// reach ~0.43 µs/row through the batched path. The whole gap is the handle, not the data.
91+
/// </para>
92+
/// <para>
93+
/// DURABILITY TRADE (this is the reason the feature is opt-in and off by default): the default
94+
/// behavior forces each record to the device before the call returns. Buffered mode hands the
95+
/// bytes to the operating system at the flush boundary instead — a process crash still cannot
96+
/// lose them, but a power loss can lose everything after the last flush. The window is bounded by
97+
/// <see cref="AppendBufferFlushThresholdBytes"/> and <see cref="AppendBufferFlushIntervalMs"/>, and
98+
/// every structural operation (commit, <c>Database.Flush()</c>, compaction, fixed-width migration,
99+
/// overflow-arena compaction, <c>DROP TABLE</c>, dispose) flushes first, so the data is never
100+
/// invisible to the engine — only the durability window changes.
101+
/// </para>
102+
/// <para>
103+
/// Reads stay correct while rows are buffered: point lookups and full scans overlay the buffer, so
104+
/// a row is visible to the same session the moment it is inserted.
105+
/// </para>
106+
/// <para>Default <c>false</c>: byte-for-byte identical behavior to the unbuffered engine.</para>
107+
/// </summary>
108+
public bool EnableBufferedAppends { get; init; } = false;
109+
110+
/// <summary>
111+
/// Gets the pending-append threshold, in bytes, at which buffered appends are flushed
112+
/// (only used when <see cref="EnableBufferedAppends"/> is enabled). Bounds both the memory the
113+
/// buffer can hold and the amount of work a crash can lose. Default 1 MB.
114+
/// </summary>
115+
public int AppendBufferFlushThresholdBytes { get; init; } = 1024 * 1024;
116+
117+
/// <summary>
118+
/// Gets the maximum age, in milliseconds, of an unflushed buffered append before it is flushed by
119+
/// the next append on that database (only used when <see cref="EnableBufferedAppends"/> is enabled).
120+
/// Together with <see cref="AppendBufferFlushThresholdBytes"/> this bounds the durability window for
121+
/// slow, low-volume writers. Default 10 ms. Set to 0 to flush only on the byte threshold and on the
122+
/// explicit structural boundaries.
123+
/// </summary>
124+
public int AppendBufferFlushIntervalMs { get; init; } = 10;
125+
80126
/// <summary>
81127
/// Gets a value indicating whether batch encryption is enabled during bulk operations.
82128
/// When true, rows are accumulated in plaintext and encrypted in 64KB batches.

0 commit comments

Comments
 (0)