From bd1ca5b7d5966f3c0da59a5e7fd6c9d07d6dfa7f Mon Sep 17 00:00:00 2001 From: Garvit Arora Date: Fri, 9 Oct 2026 18:35:31 +0530 Subject: [PATCH 1/5] docs(storage): investigate runtime database VACUUM placement --- .../workflows/runtime-vacuum-benchmark.yml | 23 +++++ .../runtime-database-vacuum-placement-6000.md | 56 ++++++++++++ scripts/perf/runtime-vacuum-benchmark.mjs | 86 +++++++++++++++++++ 3 files changed, 165 insertions(+) create mode 100644 .github/workflows/runtime-vacuum-benchmark.yml create mode 100644 docs/design/runtime-database-vacuum-placement-6000.md create mode 100644 scripts/perf/runtime-vacuum-benchmark.mjs diff --git a/.github/workflows/runtime-vacuum-benchmark.yml b/.github/workflows/runtime-vacuum-benchmark.yml new file mode 100644 index 00000000000..d85348a4c70 --- /dev/null +++ b/.github/workflows/runtime-vacuum-benchmark.yml @@ -0,0 +1,23 @@ +name: Runtime SQLite VACUUM benchmark + +on: + workflow_dispatch: + +permissions: + contents: read + +jobs: + benchmark: + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest] + runs-on: ${{ matrix.os }} + timeout-minutes: 30 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: 24.18.1 + - name: Measure synthetic runtime database conversion + run: node scripts/perf/runtime-vacuum-benchmark.mjs 100 1024 3072 diff --git a/docs/design/runtime-database-vacuum-placement-6000.md b/docs/design/runtime-database-vacuum-placement-6000.md new file mode 100644 index 00000000000..87fbaba7d99 --- /dev/null +++ b/docs/design/runtime-database-vacuum-placement-6000.md @@ -0,0 +1,56 @@ +# Runtime database one-time VACUUM placement (#6000) + +**Status:** investigation for maintainer decision; no runtime behavior change is proposed here. + +## Decision requested + +Choose when and where the explicit `Compact database` request runs the one-time full `VACUUM` needed to convert an existing `runtime.sqlite` from `auto_vacuum=NONE` to `INCREMENTAL`. The Host currently serves requests against the runtime database, uses a 5 s SQLite busy timeout, has a 75 s startup deadline, and expects clients to observe Host liveness within 8 s. + +## Measurements + +### Prior production-derived measurement from #5855 + +The #5855 review history reports a roughly 1 GiB runtime database taking about **55 s before listen**, and about **13 s per GiB when run synchronously after Ready**. The latter blocked the serving event loop long enough to exceed the 8 s client liveness timeout. Those measurements are the closest available production-shaped evidence; #5855’s merged scope removed runtime database compaction. + +### Reproducible local fixture + +The added harness, [`scripts/perf/runtime-vacuum-benchmark.mjs`](../../scripts/perf/runtime-vacuum-benchmark.mjs), uses Node's built-in SQLite, creates a `runtime_events(event_id, event_json)` table with incompressible 32 KiB payloads, deletes one third of rows to leave free pages, then measures `PRAGMA auto_vacuum=INCREMENTAL; VACUUM`. It also records the lower-bound delay to a 50 ms timer while the synchronous call occupies the event loop. This isolates SQLite's conversion cost; it does not model Maka event/query mix or filesystem-specific durability. Times are wall clock and are not directly comparable to the #5855 production-derived numbers. + +Windows 10 x64, AMD Ryzen AI 7 350, Node v24.15.0, SQLite 3.51.3; run on 2026-10-09: + +| Fixture size | DB bytes before deletion | VACUUM conversion | 50 ms timer delay (lower bound) | +| ---: | ---: | ---: | ---: | +| 100 MiB | 106,504,192 | 0.93 s | 0.88 s | +| 1,024 MiB | 1,090,572,288 | 10.17 s | 10.12 s | +| 3,072 MiB | 3,271,692,288 | 36.18 s | 36.13 s | + +The timer-delay column is the observed synchronous stall minus its 50 ms scheduled delay (or zero if the call finishes before the timer); it is a lower bound, not a complete event-loop latency distribution. The 1 GiB fixture independently crosses the 8 s timeout. The harness is also exposed as a manual two-runner workflow (`ubuntu-latest`, `windows-latest`); the Ubuntu numbers should be appended after that run completes. No second-platform measurement is claimed yet. + +## Placement options + +| Option | Host writer / client effects | 75 s startup / 8 s liveness | Crash, power loss, insufficient disk | +| --- | --- | --- | --- | +| Worker thread, separate SQLite connection, after Ready | It is still a SQLite writer. WAL permits readers in ordinary cases, but the conversion must obtain write locks and may block/fail concurrent writers. The Host must fence/gate its own runtime writes for the operation and report the maintenance state; relying only on SQLite's 5 s busy timeout can make normal writes fail. Do not open a second writer without this Host-level admission. | Does not consume startup deadline. A worker keeps the Host event loop responsive, but liveness alone is not enough: user writes must have an explicit bounded wait or be refused as maintenance-busy while the conversion owns the writer gate. | Regular `VACUUM` uses SQLite's journaling/transaction mechanisms; document recovery as SQLite-managed and verify integrity on reopen. Preflight at least 2x database size free space, as SQLite documents, with headroom for WAL/journal and concurrent filesystem use. `SQLITE_FULL` keeps the user's request retryable; hard errors clear it and are surfaced. | +| Run only while no client is attached | “No client” is not equivalent to “no Host writer”: maintenance, scheduled work, cleanup, and reconnect can still write. Must gate new clients and drain existing sessions before starting. | After Ready avoids startup deadline, but clients can repeatedly reconnect or wait through a long maintenance window; a 1 GiB run is already longer than liveness if performed on the event loop. | Same SQLite recovery and free-space behavior as worker option. Restart must retain the explicit request; a crash may leave it pending for the next no-client window. | +| Before listen, with startup progress state | No attached clients, but migration/startup ownership still matters; the operation must run after any schema migrations and before serving writers. | At the reported 1 GiB / 55 s, it consumes most of the 75 s deadline. The 3 GiB fixture is 36 s on this Windows host, while the production-derived 1 GiB time is already high; large/slower disks can exceed startup deadline. Progress UI does not change the deadline. | Same SQLite recovery and disk requirements. A restart repeats or resumes only by rerunning VACUUM; persist the request across failure and show startup recovery state. | +| Report-only | No writer contention or data-change risk. Exposes freelist/reclaimable bytes and says compaction is unavailable/pending. | No effect on startup or liveness. | No VACUUM crash/disk risk, but reclaimable space is never returned and incremental vacuum cannot take effect on existing `NONE` databases. | + +SQLite references: [`VACUUM`](https://www.sqlite.org/lang_vacuum.html) (temporary space can reach twice the database size; VACUUM can fail on locking and uses transaction machinery), [`auto_vacuum`](https://sqlite.org/pragma.html#pragma_auto_vacuum) (existing databases need VACUUM after changing from NONE), and [WAL concurrency](https://www.sqlite.org/wal.html) (one writer at a time and possible `SQLITE_BUSY`). + +## Recommendation + +Use an **after-Ready worker thread on a separate connection**, but treat it as a Host-wide writer maintenance transaction: durably record the explicit request first, close the Host's runtime-write admission gate, wait for current Host writes to drain, run conversion in the worker, reopen the gate in `finally`, and expose progress/status to clients. Bound the busy response and tell clients to retry rather than allowing the 5 s SQLite busy timeout to turn ordinary writes into surprising storage failures. Keep reads available where SQLite permits. Require a conservative free-space preflight of at least twice the database size plus reserved operational headroom; preserve a request on insufficient space and clear/report hard failures. Do not run automatically at startup or merely because no client is attached. + +This recommendation keeps work outside the 75 s startup path and the event loop. The writer gate is essential: a worker thread by itself does not make SQLite's single-writer model disappear. If the Host cannot provide a correct writer gate with bounded client behavior, use report-only as the safe first release and defer conversion. + +## Smallest first PR after placement is accepted + +Add the explicit persisted request/status and free-space preflight, plus a Host-owned writer gate and worker-thread conversion behind the existing `Compact database` action. Keep it opt-in, make insufficient space retryable across restart, test cancellation/crash/reopen and concurrent write admission, and report progress/results without adding a compatibility epoch until merge. Do not combine it with automatic compaction policy or background thresholds. + +## Reproduce + +```sh +node scripts/perf/runtime-vacuum-benchmark.mjs 100 1024 3072 +``` + +The manual GitHub Actions workflow runs the same fixture on Ubuntu and Windows. Preserve the emitted JSON with each review so runtime/platform/SQLite versions remain attached to the timings. diff --git a/scripts/perf/runtime-vacuum-benchmark.mjs b/scripts/perf/runtime-vacuum-benchmark.mjs new file mode 100644 index 00000000000..7143c8d186a --- /dev/null +++ b/scripts/perf/runtime-vacuum-benchmark.mjs @@ -0,0 +1,86 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { DatabaseSync } from 'node:sqlite'; +import { cpSync, mkdtempSync, rmSync, statSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { performance } from 'node:perf_hooks'; + +const MiB = 1024 * 1024; +const sizes = (process.argv.slice(2).length ? process.argv.slice(2) : ['100', '1024', '3072']) + .map(Number); +const root = mkdtempSync(join(tmpdir(), 'maka-vacuum-bench-')); + +try { + for (const targetMiB of sizes) { + const source = join(root, `runtime-${targetMiB}.sqlite`); + const db = new DatabaseSync(source); + db.exec('PRAGMA page_size=4096; PRAGMA auto_vacuum=NONE; PRAGMA journal_mode=DELETE;'); + db.exec('CREATE TABLE runtime_events (event_id INTEGER PRIMARY KEY, event_json BLOB NOT NULL)'); + const insert = db.prepare('INSERT INTO runtime_events(event_json) VALUES (?)'); + const payload = Buffer.alloc(32 * 1024); + for (let i = 0; i < payload.length; i += 1) payload[i] = (i * 37 + 19) & 0xff; + const started = performance.now(); + db.exec('BEGIN'); + let inserted = 0; + while (inserted * payload.length < targetMiB * MiB) { + insert.run(payload); + inserted += 1; + } + db.exec('COMMIT'); + const populatedMs = performance.now() - started; + const before = statSync(source).size; + + // Leave realistic free pages for the one-time conversion to incremental vacuum. + db.exec('DELETE FROM runtime_events WHERE event_id % 3 = 0'); + db.close(); + const copy = join(root, `runtime-${targetMiB}-conversion.sqlite`); + cpSync(source, copy); + const conversion = new DatabaseSync(copy); + conversion.exec('PRAGMA busy_timeout=5000; PRAGMA auto_vacuum=INCREMENTAL;'); + + let timerFired = false; + let lag = 0; + const tickStart = performance.now(); + const timer = setTimeout(() => { + timerFired = true; + lag = Math.max(0, performance.now() - tickStart - 50); + }, 50); + const vacuumStart = performance.now(); + conversion.exec('VACUUM'); + const vacuumMs = performance.now() - vacuumStart; + clearTimeout(timer); + if (!timerFired) lag = Math.max(0, vacuumMs - 50); + conversion.close(); + console.log(JSON.stringify({ + platform: `${process.platform}-${process.arch}`, + node: process.version, + sqlite: process.versions.sqlite, + targetMiB, + sourceBytesBeforeDelete: before, + insertedRows: inserted, + populationMs: Math.round(populatedMs), + conversionVacuumMs: Math.round(vacuumMs), + mainThreadTimerDelayMs: Math.round(lag), + })); + } +} finally { + rmSync(root, { recursive: true, force: true }); +} From fc896403ebebbd16cb0169eb4f7ed511eba54d7f Mon Sep 17 00:00:00 2001 From: Garvit Arora Date: Fri, 9 Oct 2026 18:43:05 +0530 Subject: [PATCH 2/5] chore: add canonical headers to benchmark artifacts --- .../workflows/runtime-vacuum-benchmark.yml | 17 +++++++++++++++++ .../runtime-database-vacuum-placement-6000.md | 19 +++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/.github/workflows/runtime-vacuum-benchmark.yml b/.github/workflows/runtime-vacuum-benchmark.yml index d85348a4c70..d852b59be5f 100644 --- a/.github/workflows/runtime-vacuum-benchmark.yml +++ b/.github/workflows/runtime-vacuum-benchmark.yml @@ -1,3 +1,20 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + name: Runtime SQLite VACUUM benchmark on: diff --git a/docs/design/runtime-database-vacuum-placement-6000.md b/docs/design/runtime-database-vacuum-placement-6000.md index 87fbaba7d99..014623723a8 100644 --- a/docs/design/runtime-database-vacuum-placement-6000.md +++ b/docs/design/runtime-database-vacuum-placement-6000.md @@ -1,3 +1,22 @@ + + # Runtime database one-time VACUUM placement (#6000) **Status:** investigation for maintainer decision; no runtime behavior change is proposed here. From 23a6565cb5bf4cd940e457a4d423ce315f2a8aae Mon Sep 17 00:00:00 2001 From: Garvit Arora Date: Fri, 9 Oct 2026 23:47:09 +0530 Subject: [PATCH 3/5] fix(storage): correct VACUUM design benchmark checks --- .../workflows/runtime-vacuum-benchmark.yml | 4 +++- .../runtime-database-vacuum-placement-6000.md | 22 +++++++++---------- scripts/perf/runtime-vacuum-benchmark.mjs | 12 +--------- 3 files changed, 15 insertions(+), 23 deletions(-) diff --git a/.github/workflows/runtime-vacuum-benchmark.yml b/.github/workflows/runtime-vacuum-benchmark.yml index d852b59be5f..cff633fe15e 100644 --- a/.github/workflows/runtime-vacuum-benchmark.yml +++ b/.github/workflows/runtime-vacuum-benchmark.yml @@ -28,11 +28,13 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-latest, windows-latest] + os: [ubuntu-24.04, windows-latest] runs-on: ${{ matrix.os }} timeout-minutes: 30 steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: node-version: 24.18.1 diff --git a/docs/design/runtime-database-vacuum-placement-6000.md b/docs/design/runtime-database-vacuum-placement-6000.md index 014623723a8..5008847bde3 100644 --- a/docs/design/runtime-database-vacuum-placement-6000.md +++ b/docs/design/runtime-database-vacuum-placement-6000.md @@ -23,7 +23,7 @@ ## Decision requested -Choose when and where the explicit `Compact database` request runs the one-time full `VACUUM` needed to convert an existing `runtime.sqlite` from `auto_vacuum=NONE` to `INCREMENTAL`. The Host currently serves requests against the runtime database, uses a 5 s SQLite busy timeout, has a 75 s startup deadline, and expects clients to observe Host liveness within 8 s. +Choose when and where the planned explicit `Compact database` request runs the one-time full `VACUUM` needed to convert an existing `runtime.sqlite` from `auto_vacuum=NONE` to `INCREMENTAL`. The action and protocol operation do not yet exist. The Host currently serves requests against the runtime database, uses a 5 s SQLite busy timeout, has a 75 s startup deadline, and expects clients to observe Host liveness within 8 s. ## Measurements @@ -33,23 +33,23 @@ The #5855 review history reports a roughly 1 GiB runtime database taking about * ### Reproducible local fixture -The added harness, [`scripts/perf/runtime-vacuum-benchmark.mjs`](../../scripts/perf/runtime-vacuum-benchmark.mjs), uses Node's built-in SQLite, creates a `runtime_events(event_id, event_json)` table with incompressible 32 KiB payloads, deletes one third of rows to leave free pages, then measures `PRAGMA auto_vacuum=INCREMENTAL; VACUUM`. It also records the lower-bound delay to a 50 ms timer while the synchronous call occupies the event loop. This isolates SQLite's conversion cost; it does not model Maka event/query mix or filesystem-specific durability. Times are wall clock and are not directly comparable to the #5855 production-derived numbers. +The added harness, [`scripts/perf/runtime-vacuum-benchmark.mjs`](../../scripts/perf/runtime-vacuum-benchmark.mjs), uses Node's built-in SQLite in WAL mode, creates a `runtime_events(event_id, event_json)` table with incompressible 32 KiB payloads, deletes one third of rows to leave free pages, then measures `PRAGMA auto_vacuum=INCREMENTAL; VACUUM`. The synchronous call blocks its calling thread for the measured duration. This isolates SQLite's conversion cost; it does not model Maka event/query mix or filesystem-specific durability, and does not measure peak WAL growth. Times are wall clock and are not directly comparable to the #5855 production-derived numbers. Windows 10 x64, AMD Ryzen AI 7 350, Node v24.15.0, SQLite 3.51.3; run on 2026-10-09: -| Fixture size | DB bytes before deletion | VACUUM conversion | 50 ms timer delay (lower bound) | -| ---: | ---: | ---: | ---: | -| 100 MiB | 106,504,192 | 0.93 s | 0.88 s | -| 1,024 MiB | 1,090,572,288 | 10.17 s | 10.12 s | -| 3,072 MiB | 3,271,692,288 | 36.18 s | 36.13 s | +| Fixture size | DB bytes before deletion | VACUUM conversion | +| ---: | ---: | ---: | +| 100 MiB | 106,504,192 | 0.58 s | +| 1,024 MiB | 1,090,572,288 | 10.73 s | +| 3,072 MiB | 3,271,692,288 | 140.36 s | -The timer-delay column is the observed synchronous stall minus its 50 ms scheduled delay (or zero if the call finishes before the timer); it is a lower bound, not a complete event-loop latency distribution. The 1 GiB fixture independently crosses the 8 s timeout. The harness is also exposed as a manual two-runner workflow (`ubuntu-latest`, `windows-latest`); the Ubuntu numbers should be appended after that run completes. No second-platform measurement is claimed yet. +The previous DELETE-mode run measured 0.93 s, 10.17 s and 36.18 s at these sizes; it did not match production WAL mode and is excluded from the table. The synchronous duration directly indicates how long an event loop would be blocked. The current manual workflow uses `ubuntu-24.04` and `windows-latest`. Windows WAL-mode timings are now recorded; Ubuntu remains pending because GitHub will only dispatch the workflow after it exists on the fork's default branch. No cross-platform comparison is claimed yet. ## Placement options | Option | Host writer / client effects | 75 s startup / 8 s liveness | Crash, power loss, insufficient disk | | --- | --- | --- | --- | -| Worker thread, separate SQLite connection, after Ready | It is still a SQLite writer. WAL permits readers in ordinary cases, but the conversion must obtain write locks and may block/fail concurrent writers. The Host must fence/gate its own runtime writes for the operation and report the maintenance state; relying only on SQLite's 5 s busy timeout can make normal writes fail. Do not open a second writer without this Host-level admission. | Does not consume startup deadline. A worker keeps the Host event loop responsive, but liveness alone is not enough: user writes must have an explicit bounded wait or be refused as maintenance-busy while the conversion owns the writer gate. | Regular `VACUUM` uses SQLite's journaling/transaction mechanisms; document recovery as SQLite-managed and verify integrity on reopen. Preflight at least 2x database size free space, as SQLite documents, with headroom for WAL/journal and concurrent filesystem use. `SQLITE_FULL` keeps the user's request retryable; hard errors clear it and are surfaced. | +| Worker thread, separate SQLite connection, after Ready | It is still a SQLite writer. WAL permits readers in ordinary cases, but the conversion must obtain write locks and may block/fail concurrent writers. The Host must fence/gate its own runtime writes for the operation and report the maintenance state; relying only on SQLite's 5 s busy timeout can make normal writes fail. Do not open a second writer without this Host-level admission. | Does not consume startup deadline. A worker keeps the Host event loop responsive, but liveness alone is not enough: user writes must have an explicit bounded wait or be refused as maintenance-busy while the conversion owns the writer gate. | Regular `VACUUM` uses SQLite's journaling/transaction mechanisms; document recovery as SQLite-managed and verify integrity on reopen. SQLite documents up to 2x database-size temporary space; WAL-mode peak usage must also account for the WAL and existing sidecars, so preflight must measure the complete database file set and reserve additional headroom. `SQLITE_FULL` keeps the user's request retryable; hard errors clear it and are surfaced. | | Run only while no client is attached | “No client” is not equivalent to “no Host writer”: maintenance, scheduled work, cleanup, and reconnect can still write. Must gate new clients and drain existing sessions before starting. | After Ready avoids startup deadline, but clients can repeatedly reconnect or wait through a long maintenance window; a 1 GiB run is already longer than liveness if performed on the event loop. | Same SQLite recovery and free-space behavior as worker option. Restart must retain the explicit request; a crash may leave it pending for the next no-client window. | | Before listen, with startup progress state | No attached clients, but migration/startup ownership still matters; the operation must run after any schema migrations and before serving writers. | At the reported 1 GiB / 55 s, it consumes most of the 75 s deadline. The 3 GiB fixture is 36 s on this Windows host, while the production-derived 1 GiB time is already high; large/slower disks can exceed startup deadline. Progress UI does not change the deadline. | Same SQLite recovery and disk requirements. A restart repeats or resumes only by rerunning VACUUM; persist the request across failure and show startup recovery state. | | Report-only | No writer contention or data-change risk. Exposes freelist/reclaimable bytes and says compaction is unavailable/pending. | No effect on startup or liveness. | No VACUUM crash/disk risk, but reclaimable space is never returned and incremental vacuum cannot take effect on existing `NONE` databases. | @@ -58,13 +58,13 @@ SQLite references: [`VACUUM`](https://www.sqlite.org/lang_vacuum.html) (temporar ## Recommendation -Use an **after-Ready worker thread on a separate connection**, but treat it as a Host-wide writer maintenance transaction: durably record the explicit request first, close the Host's runtime-write admission gate, wait for current Host writes to drain, run conversion in the worker, reopen the gate in `finally`, and expose progress/status to clients. Bound the busy response and tell clients to retry rather than allowing the 5 s SQLite busy timeout to turn ordinary writes into surprising storage failures. Keep reads available where SQLite permits. Require a conservative free-space preflight of at least twice the database size plus reserved operational headroom; preserve a request on insufficient space and clear/report hard failures. Do not run automatically at startup or merely because no client is attached. +Use an **after-Ready worker thread on a separate connection**, but treat it as a Host-wide writer maintenance transaction: durably record the explicit request first, close the Host's runtime-write admission gate, wait for current Host writes to drain, run conversion in the worker, reopen the gate in `finally`, and expose progress/status to clients. Bound the busy response and tell clients to retry rather than allowing the 5 s SQLite busy timeout to turn ordinary writes into surprising storage failures. Keep reads available where SQLite permits. Require a conservative free-space preflight for the complete database file set, including WAL, with reserved operational headroom; preserve a request on insufficient space and clear/report hard failures. Do not run automatically at startup or merely because no client is attached. This recommendation keeps work outside the 75 s startup path and the event loop. The writer gate is essential: a worker thread by itself does not make SQLite's single-writer model disappear. If the Host cannot provide a correct writer gate with bounded client behavior, use report-only as the safe first release and defer conversion. ## Smallest first PR after placement is accepted -Add the explicit persisted request/status and free-space preflight, plus a Host-owned writer gate and worker-thread conversion behind the existing `Compact database` action. Keep it opt-in, make insufficient space retryable across restart, test cancellation/crash/reopen and concurrent write admission, and report progress/results without adding a compatibility epoch until merge. Do not combine it with automatic compaction policy or background thresholds. +Add the planned explicit `Compact database` action and protocol request/status, persist the request, and implement database-file-set free-space preflight plus a Host-owned writer gate and worker-thread conversion. Keep it opt-in, make insufficient space retryable across restart, test cancellation/crash/reopen and concurrent write admission, and report progress/results. Claim the required compatibility epoch at merge time. Do not combine it with automatic compaction policy or background thresholds. ## Reproduce diff --git a/scripts/perf/runtime-vacuum-benchmark.mjs b/scripts/perf/runtime-vacuum-benchmark.mjs index 7143c8d186a..3cb4689ca8c 100644 --- a/scripts/perf/runtime-vacuum-benchmark.mjs +++ b/scripts/perf/runtime-vacuum-benchmark.mjs @@ -32,7 +32,7 @@ try { for (const targetMiB of sizes) { const source = join(root, `runtime-${targetMiB}.sqlite`); const db = new DatabaseSync(source); - db.exec('PRAGMA page_size=4096; PRAGMA auto_vacuum=NONE; PRAGMA journal_mode=DELETE;'); + db.exec('PRAGMA page_size=4096; PRAGMA auto_vacuum=NONE; PRAGMA journal_mode=WAL;'); db.exec('CREATE TABLE runtime_events (event_id INTEGER PRIMARY KEY, event_json BLOB NOT NULL)'); const insert = db.prepare('INSERT INTO runtime_events(event_json) VALUES (?)'); const payload = Buffer.alloc(32 * 1024); @@ -56,18 +56,9 @@ try { const conversion = new DatabaseSync(copy); conversion.exec('PRAGMA busy_timeout=5000; PRAGMA auto_vacuum=INCREMENTAL;'); - let timerFired = false; - let lag = 0; - const tickStart = performance.now(); - const timer = setTimeout(() => { - timerFired = true; - lag = Math.max(0, performance.now() - tickStart - 50); - }, 50); const vacuumStart = performance.now(); conversion.exec('VACUUM'); const vacuumMs = performance.now() - vacuumStart; - clearTimeout(timer); - if (!timerFired) lag = Math.max(0, vacuumMs - 50); conversion.close(); console.log(JSON.stringify({ platform: `${process.platform}-${process.arch}`, @@ -78,7 +69,6 @@ try { insertedRows: inserted, populationMs: Math.round(populatedMs), conversionVacuumMs: Math.round(vacuumMs), - mainThreadTimerDelayMs: Math.round(lag), })); } } finally { From 1d8124a0dd7fed56634911a417c34bbbde8d6768 Mon Sep 17 00:00:00 2001 From: Garvit Arora Date: Sat, 10 Oct 2026 00:06:02 +0530 Subject: [PATCH 4/5] style(storage): format VACUUM benchmark --- scripts/perf/runtime-vacuum-benchmark.mjs | 27 +++++++++++++---------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/scripts/perf/runtime-vacuum-benchmark.mjs b/scripts/perf/runtime-vacuum-benchmark.mjs index 3cb4689ca8c..53f2193815a 100644 --- a/scripts/perf/runtime-vacuum-benchmark.mjs +++ b/scripts/perf/runtime-vacuum-benchmark.mjs @@ -24,8 +24,9 @@ import { join } from 'node:path'; import { performance } from 'node:perf_hooks'; const MiB = 1024 * 1024; -const sizes = (process.argv.slice(2).length ? process.argv.slice(2) : ['100', '1024', '3072']) - .map(Number); +const sizes = (process.argv.slice(2).length ? process.argv.slice(2) : ['100', '1024', '3072']).map( + Number, +); const root = mkdtempSync(join(tmpdir(), 'maka-vacuum-bench-')); try { @@ -60,16 +61,18 @@ try { conversion.exec('VACUUM'); const vacuumMs = performance.now() - vacuumStart; conversion.close(); - console.log(JSON.stringify({ - platform: `${process.platform}-${process.arch}`, - node: process.version, - sqlite: process.versions.sqlite, - targetMiB, - sourceBytesBeforeDelete: before, - insertedRows: inserted, - populationMs: Math.round(populatedMs), - conversionVacuumMs: Math.round(vacuumMs), - })); + console.log( + JSON.stringify({ + platform: `${process.platform}-${process.arch}`, + node: process.version, + sqlite: process.versions.sqlite, + targetMiB, + sourceBytesBeforeDelete: before, + insertedRows: inserted, + populationMs: Math.round(populatedMs), + conversionVacuumMs: Math.round(vacuumMs), + }), + ); } } finally { rmSync(root, { recursive: true, force: true }); From e923f845559c625bb3edd57c15fe459faaeeb944 Mon Sep 17 00:00:00 2001 From: Garvit Arora Date: Sat, 10 Oct 2026 11:27:37 +0530 Subject: [PATCH 5/5] docs(storage): correct runtime vacuum investigation --- .../workflows/runtime-vacuum-benchmark.yml | 42 -------- .../runtime-database-vacuum-placement-6000.md | 75 -------------- scripts/perf/README.md | 98 +++++-------------- scripts/perf/runtime-vacuum-benchmark.mjs | 18 +++- 4 files changed, 41 insertions(+), 192 deletions(-) delete mode 100644 .github/workflows/runtime-vacuum-benchmark.yml delete mode 100644 docs/design/runtime-database-vacuum-placement-6000.md diff --git a/.github/workflows/runtime-vacuum-benchmark.yml b/.github/workflows/runtime-vacuum-benchmark.yml deleted file mode 100644 index cff633fe15e..00000000000 --- a/.github/workflows/runtime-vacuum-benchmark.yml +++ /dev/null @@ -1,42 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. - -name: Runtime SQLite VACUUM benchmark - -on: - workflow_dispatch: - -permissions: - contents: read - -jobs: - benchmark: - strategy: - fail-fast: false - matrix: - os: [ubuntu-24.04, windows-latest] - runs-on: ${{ matrix.os }} - timeout-minutes: 30 - steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - persist-credentials: false - - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 - with: - node-version: 24.18.1 - - name: Measure synthetic runtime database conversion - run: node scripts/perf/runtime-vacuum-benchmark.mjs 100 1024 3072 diff --git a/docs/design/runtime-database-vacuum-placement-6000.md b/docs/design/runtime-database-vacuum-placement-6000.md deleted file mode 100644 index 5008847bde3..00000000000 --- a/docs/design/runtime-database-vacuum-placement-6000.md +++ /dev/null @@ -1,75 +0,0 @@ - - -# Runtime database one-time VACUUM placement (#6000) - -**Status:** investigation for maintainer decision; no runtime behavior change is proposed here. - -## Decision requested - -Choose when and where the planned explicit `Compact database` request runs the one-time full `VACUUM` needed to convert an existing `runtime.sqlite` from `auto_vacuum=NONE` to `INCREMENTAL`. The action and protocol operation do not yet exist. The Host currently serves requests against the runtime database, uses a 5 s SQLite busy timeout, has a 75 s startup deadline, and expects clients to observe Host liveness within 8 s. - -## Measurements - -### Prior production-derived measurement from #5855 - -The #5855 review history reports a roughly 1 GiB runtime database taking about **55 s before listen**, and about **13 s per GiB when run synchronously after Ready**. The latter blocked the serving event loop long enough to exceed the 8 s client liveness timeout. Those measurements are the closest available production-shaped evidence; #5855’s merged scope removed runtime database compaction. - -### Reproducible local fixture - -The added harness, [`scripts/perf/runtime-vacuum-benchmark.mjs`](../../scripts/perf/runtime-vacuum-benchmark.mjs), uses Node's built-in SQLite in WAL mode, creates a `runtime_events(event_id, event_json)` table with incompressible 32 KiB payloads, deletes one third of rows to leave free pages, then measures `PRAGMA auto_vacuum=INCREMENTAL; VACUUM`. The synchronous call blocks its calling thread for the measured duration. This isolates SQLite's conversion cost; it does not model Maka event/query mix or filesystem-specific durability, and does not measure peak WAL growth. Times are wall clock and are not directly comparable to the #5855 production-derived numbers. - -Windows 10 x64, AMD Ryzen AI 7 350, Node v24.15.0, SQLite 3.51.3; run on 2026-10-09: - -| Fixture size | DB bytes before deletion | VACUUM conversion | -| ---: | ---: | ---: | -| 100 MiB | 106,504,192 | 0.58 s | -| 1,024 MiB | 1,090,572,288 | 10.73 s | -| 3,072 MiB | 3,271,692,288 | 140.36 s | - -The previous DELETE-mode run measured 0.93 s, 10.17 s and 36.18 s at these sizes; it did not match production WAL mode and is excluded from the table. The synchronous duration directly indicates how long an event loop would be blocked. The current manual workflow uses `ubuntu-24.04` and `windows-latest`. Windows WAL-mode timings are now recorded; Ubuntu remains pending because GitHub will only dispatch the workflow after it exists on the fork's default branch. No cross-platform comparison is claimed yet. - -## Placement options - -| Option | Host writer / client effects | 75 s startup / 8 s liveness | Crash, power loss, insufficient disk | -| --- | --- | --- | --- | -| Worker thread, separate SQLite connection, after Ready | It is still a SQLite writer. WAL permits readers in ordinary cases, but the conversion must obtain write locks and may block/fail concurrent writers. The Host must fence/gate its own runtime writes for the operation and report the maintenance state; relying only on SQLite's 5 s busy timeout can make normal writes fail. Do not open a second writer without this Host-level admission. | Does not consume startup deadline. A worker keeps the Host event loop responsive, but liveness alone is not enough: user writes must have an explicit bounded wait or be refused as maintenance-busy while the conversion owns the writer gate. | Regular `VACUUM` uses SQLite's journaling/transaction mechanisms; document recovery as SQLite-managed and verify integrity on reopen. SQLite documents up to 2x database-size temporary space; WAL-mode peak usage must also account for the WAL and existing sidecars, so preflight must measure the complete database file set and reserve additional headroom. `SQLITE_FULL` keeps the user's request retryable; hard errors clear it and are surfaced. | -| Run only while no client is attached | “No client” is not equivalent to “no Host writer”: maintenance, scheduled work, cleanup, and reconnect can still write. Must gate new clients and drain existing sessions before starting. | After Ready avoids startup deadline, but clients can repeatedly reconnect or wait through a long maintenance window; a 1 GiB run is already longer than liveness if performed on the event loop. | Same SQLite recovery and free-space behavior as worker option. Restart must retain the explicit request; a crash may leave it pending for the next no-client window. | -| Before listen, with startup progress state | No attached clients, but migration/startup ownership still matters; the operation must run after any schema migrations and before serving writers. | At the reported 1 GiB / 55 s, it consumes most of the 75 s deadline. The 3 GiB fixture is 36 s on this Windows host, while the production-derived 1 GiB time is already high; large/slower disks can exceed startup deadline. Progress UI does not change the deadline. | Same SQLite recovery and disk requirements. A restart repeats or resumes only by rerunning VACUUM; persist the request across failure and show startup recovery state. | -| Report-only | No writer contention or data-change risk. Exposes freelist/reclaimable bytes and says compaction is unavailable/pending. | No effect on startup or liveness. | No VACUUM crash/disk risk, but reclaimable space is never returned and incremental vacuum cannot take effect on existing `NONE` databases. | - -SQLite references: [`VACUUM`](https://www.sqlite.org/lang_vacuum.html) (temporary space can reach twice the database size; VACUUM can fail on locking and uses transaction machinery), [`auto_vacuum`](https://sqlite.org/pragma.html#pragma_auto_vacuum) (existing databases need VACUUM after changing from NONE), and [WAL concurrency](https://www.sqlite.org/wal.html) (one writer at a time and possible `SQLITE_BUSY`). - -## Recommendation - -Use an **after-Ready worker thread on a separate connection**, but treat it as a Host-wide writer maintenance transaction: durably record the explicit request first, close the Host's runtime-write admission gate, wait for current Host writes to drain, run conversion in the worker, reopen the gate in `finally`, and expose progress/status to clients. Bound the busy response and tell clients to retry rather than allowing the 5 s SQLite busy timeout to turn ordinary writes into surprising storage failures. Keep reads available where SQLite permits. Require a conservative free-space preflight for the complete database file set, including WAL, with reserved operational headroom; preserve a request on insufficient space and clear/report hard failures. Do not run automatically at startup or merely because no client is attached. - -This recommendation keeps work outside the 75 s startup path and the event loop. The writer gate is essential: a worker thread by itself does not make SQLite's single-writer model disappear. If the Host cannot provide a correct writer gate with bounded client behavior, use report-only as the safe first release and defer conversion. - -## Smallest first PR after placement is accepted - -Add the planned explicit `Compact database` action and protocol request/status, persist the request, and implement database-file-set free-space preflight plus a Host-owned writer gate and worker-thread conversion. Keep it opt-in, make insufficient space retryable across restart, test cancellation/crash/reopen and concurrent write admission, and report progress/results. Claim the required compatibility epoch at merge time. Do not combine it with automatic compaction policy or background thresholds. - -## Reproduce - -```sh -node scripts/perf/runtime-vacuum-benchmark.mjs 100 1024 3072 -``` - -The manual GitHub Actions workflow runs the same fixture on Ubuntu and Windows. Preserve the emitted JSON with each review so runtime/platform/SQLite versions remain attached to the timings. diff --git a/scripts/perf/README.md b/scripts/perf/README.md index c6c719bd644..f0b0109cdf8 100644 --- a/scripts/perf/README.md +++ b/scripts/perf/README.md @@ -9,88 +9,40 @@ http://www.apache.org/licenses/LICENSE-2.0 - Unless required by applicable law or agreed to in writing, - software distributed under the License is distributed on an - "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - KIND, either express or implied. See the License for the - specific language governing permissions and limitations - under the License. + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. --> -# Renderer performance probes +# Runtime SQLite VACUUM benchmark -For the two manual CI lanes, raw reports, fixtures and their coverage limits, see [CI.md](./CI.md). Those lanes verify DOM events and consumer state; they do not use the legacy native-input helpers below. +Run `node scripts/perf/runtime-vacuum-benchmark.mjs 100 1024 3072`. It uses Node's built-in SQLite with WAL, creates incompressible 32 KiB rows, deletes one third to leave free pages, converts with `PRAGMA auto_vacuum=INCREMENTAL; VACUUM`, and includes connection close/checkpoint time. It then verifies `auto_vacuum = 2`, an empty freelist, and `integrity_check = 'ok'`. -Measuring what a session switch costs in the running Desktop app, over CDP. The -findings in #4109 were produced with these; they live here so the next -measurement is a command rather than a rebuild. +This is a synthetic conversion-cost fixture, not a production database workload. It does not yet measure peak WAL size or platform-specific free-space requirements. Preserve the emitted JSON, including OS, Node and SQLite versions, with results. -They are ad-hoc tools, not a benchmark suite: they attach to a dev app you -started, and they answer "how much" only for the interaction they drive. +## Current Windows run -## Running +Windows x64, Node v24.15.0, SQLite 3.51.3; rerun 2026-10-10: -Start the dev app with the debugger open: +| Fixture size | Source bytes before deletion | Conversion and close | +| ---: | ---: | ---: | +| 100 MiB | 106,504,192 | 1.384 s | +| 1,024 MiB | 1,090,572,288 | 16.397 s | +| 3,072 MiB | 3,271,692,288 | 64.150 s | -```bash -npm run dev -w @maka/desktop -- --remote-debugging-port=9334 -``` +All three post-close checks passed (`auto_vacuum=2`, `freelist_count=0`, `integrity_check=ok`). Earlier pre-close WAL timings were 0.58 s, 10.73 s and 140.36 s; those are superseded. The 36.18 s 3 GiB figure was from the excluded DELETE-mode run and is not a WAL result. -Then, with a populated sidebar (about 30 rows is what the #4109 numbers used): +Only Windows has been measured so far. The requested second-platform run is outstanding; a prior one-off benchmark workflow was removed from this repository. The corrected 1-to-3 GiB increase is still super-linear (16.397 s to 64.150 s); WAL/journal I/O, SQLite page-cache behavior, storage and antivirus scanning are plausible contributors, but this fixture does not isolate their individual contributions. -```bash -node scripts/perf/session-switch-commits.mjs before -node scripts/perf/session-switch-busy-js.mjs -``` +## Placement notes for issue #6000 -- `session-switch-commits.mjs` — React commits per switch, how many are - full-tree renders, and how many fibers actually re-rendered. Reloads the page - first, so `react-commit-probe.js` is in place before React boots. -- `session-switch-busy-js.mjs` — renderer busy JS per switch, with the top - self-time entries. It reports the running build, one configuration. A - before/after needs both configurations in one instance; see below. -- `react-commit-probe.js` — the in-page half: a minimal - `__REACT_DEVTOOLS_GLOBAL_HOOK__` plus a `Function.prototype.bind` wrapper that - catches React creating a `dispatchSetState`, so a commit can be attributed to - the `setState` that caused it. -- `cdp-client.mjs` — the protocol client and the row-clicking helper. -- `xterm-hidden-selection.mjs ` — functional regression for the patched - xterm CJS/ESM bundles in real Chromium: hidden selection updates must not - redraw rows, and showing the terminal must paint the latest selection without - another write. Use a disposable Electron fixture; the probe briefly embeds - its own terminal frame and bypasses CSP, then removes the frame and restores - CSP. This is a behavior check, not a performance score. +- `runtime.sqlite` has one process-local owner connection; repositories borrow leases. A worker's second connection departs from that ownership model, and a Host-only gate cannot fence secondary processes using `require_current`. +- A worker thread keeps the event loop responsive and reads available where SQLite permits. It does not remove `VACUUM`'s write lock: concurrent writers still wait or hit the configured 5 s `SQLITE_BUSY` timeout. +- In WAL mode, reserve close to 3x the database size plus operational headroom until measured peak usage supports a tighter bound. The prior 2x estimate omitted WAL growth. +- Under WAL with `synchronous=FULL`, a crash before the VACUUM transaction commits discards the uncommitted conversion; the original database remains. Reopen and verify integrity. +- `VACUUM INTO` can build a staging copy while the live database stays available. Swap it on the next startup before listen; do not plan a live swap on Windows while handles are open. Maka already uses a private copy for context offload (`context-offload-snapshot.ts`). +- The smallest first implementation is to create new `runtime.sqlite` databases with `auto_vacuum=INCREMENTAL` before schema creation and WAL activation. No full VACUUM is required for new databases. Converting existing databases is a later, separate decision. -## The one rule - -**Never compare numbers from two app launches.** Restarting the app shifts these -metrics by orders of magnitude, while the spread inside a single running -instance is small. An A/B means: one instance, a `globalThis` switch to select -the configuration, at least three repetitions per configuration, alternating, -compared pairwise. Two runs of "before" and "after" against two launches will -produce a confident number that means nothing. - -The switch is a throwaway, not a shipped affordance. Branch a worktree, put a -`globalThis` flag in the one place that reads it — one that restores the old -behaviour when set, so the regressed configuration is reachable without -rebuilding the defect by hand — alternate the flag inside the running instance, -then delete the worktree. Measurement discipline is ours; product code carries -none of it. A flag that reaches `main` is a branch nothing executes and a -defect replica that has to be maintained against the code it replicates. - -Two smaller ones that cost time to rediscover: - -- Clicks must be `Input.dispatchMouseEvent`. A synthesised `element.click()` - does not activate an Astryx `SideNavItem`, so the app does nothing and the - measurement describes it. -- Disarm the commit probe (`__MAKA_PROBE__.arm = false`) before sampling. Its - fiber walk otherwise shows up in the profile as the app's own work; - `session-switch-busy-js.mjs` does this for you. - -## What these do not tell you - -The commit count is unweighted, and busy JS on this workload is spread across -about a thousand cheap fibers rather than concentrated in one component. A -change that lowers the commit count has not necessarily lowered the time, and a -change that moves the top self-time entry has not necessarily lowered anything. -Read both, and read the medians. +No placement recommendation is made until the required second-platform measurement and owner-process fencing design are settled. The full analysis and option-by-option effects belong in the #6000 issue discussion; this README only preserves the fixture and its caveats. diff --git a/scripts/perf/runtime-vacuum-benchmark.mjs b/scripts/perf/runtime-vacuum-benchmark.mjs index 53f2193815a..83f7547841c 100644 --- a/scripts/perf/runtime-vacuum-benchmark.mjs +++ b/scripts/perf/runtime-vacuum-benchmark.mjs @@ -59,8 +59,19 @@ try { const vacuumStart = performance.now(); conversion.exec('VACUUM'); - const vacuumMs = performance.now() - vacuumStart; conversion.close(); + // Include close(), which checkpoints the WAL and is part of completion. + const conversionMs = performance.now() - vacuumStart; + const verified = new DatabaseSync(copy); + const autoVacuum = verified.prepare('PRAGMA auto_vacuum').get().auto_vacuum; + const freelistCount = verified.prepare('PRAGMA freelist_count').get().freelist_count; + const integrity = verified.prepare('PRAGMA integrity_check').get().integrity_check; + verified.close(); + if (autoVacuum !== 2 || freelistCount !== 0 || integrity !== 'ok') { + throw new Error( + `post-conversion verification failed: auto_vacuum=${autoVacuum}, freelist=${freelistCount}, integrity=${integrity}`, + ); + } console.log( JSON.stringify({ platform: `${process.platform}-${process.arch}`, @@ -70,7 +81,10 @@ try { sourceBytesBeforeDelete: before, insertedRows: inserted, populationMs: Math.round(populatedMs), - conversionVacuumMs: Math.round(vacuumMs), + conversionAndCloseMs: Math.round(conversionMs), + autoVacuum, + freelistCount, + integrity, }), ); }