Repository navigation
perf(orchestration): remove mutation receipt capacity scans - #13647
Conversation
📝 WalkthroughWalkthroughMutation receipt capacity management moved into a shared module. Schema version 26 adds a completed-receipt index, a count ledger, and maintenance triggers. Capacity enforcement expires old completed receipts, prunes older records in batches, and raises 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 17f90707-1ddd-4a4b-881c-492fb55be813
📒 Files selected for processing (5)
src/main/runtime/orchestration/db.tssrc/main/runtime/orchestration/mutation-receipt-capacity.test.tssrc/main/runtime/orchestration/mutation-receipt-capacity.tssrc/main/runtime/orchestration/orchestration-db-retention-pagination.test.tstests/tools/benchmarks/mutation-receipt-capacity-bench.mjs
| try { | ||
| const legacyPath = join(fixtureDir, 'legacy.db') | ||
| const optimizedPath = join(fixtureDir, 'optimized.db') | ||
| const legacyDb = createFixture(legacyPath, options.payloadBytes, false) | ||
| const optimizedDb = createFixture(optimizedPath, options.payloadBytes, true) | ||
| const databaseBytes = statSync(legacyPath).size | ||
| const legacy = measure(legacyDb, options.iterations, runLegacyMutation) | ||
| const optimized = measure(optimizedDb, options.iterations, runOptimizedMutation) | ||
| legacyDb.close() | ||
| optimizedDb.close() | ||
|
|
||
| process.stdout.write( | ||
| `${JSON.stringify( | ||
| { | ||
| rows: ROW_LIMIT, | ||
| payloadBytes: options.payloadBytes, | ||
| databaseBytes, | ||
| iterations: options.iterations, | ||
| legacy, | ||
| optimized, | ||
| medianSpeedup: legacy.medianMs / optimized.medianMs, | ||
| totalSpeedup: legacy.totalMs / optimized.totalMs | ||
| }, | ||
| null, | ||
| 2 | ||
| )}\n` | ||
| ) | ||
| } finally { | ||
| rmSync(fixtureDir, { recursive: true, force: true }) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
file="tests/tools/benchmarks/mutation-receipt-capacity-bench.mjs"
printf '%s\n' "== file outline =="
ast-grep outline "$file" || true
printf '%s\n' "== relevant source =="
cat -n "$file" | sed -n '1,220p'
printf '%s\n' "== runtime availability =="
node --version || true
printf '%s\n' "== DatabaseSync references =="
rg -n "DatabaseSync|createFixture|function measure|const measure" "$file"Repository: stablyai/orca
Length of output: 8872
🏁 Script executed:
#!/bin/bash
set -eu
node --input-type=module <<'JS'
import { DatabaseSync } from 'node:sqlite'
import { existsSync, mkdtempSync, rmSync, statSync } from 'node:fs'
import { join } from 'node:path'
import { tmpdir } from 'node:os'
const directory = mkdtempSync(join(tmpdir(), 'sqlite-cleanup-check-'))
const databasePath = join(directory, 'receipt.db')
const db = new DatabaseSync(databasePath)
let removalResult
try {
db.exec('CREATE TABLE receipts (id INTEGER PRIMARY KEY)')
removalResult = 'succeeded'
try {
rmSync(directory, { recursive: true, force: true })
} catch (error) {
removalResult = `failed: ${error.code ?? error.name}: ${error.message}`
}
console.log(JSON.stringify({
platform: process.platform,
node: process.version,
removalResult,
directoryExistsAfterRemoval: existsSync(directory),
databaseExistsAfterRemoval: existsSync(databasePath)
}))
} finally {
db.close()
if (existsSync(directory)) rmSync(directory, { recursive: true, force: true })
}
JSRepository: stablyai/orca
Length of output: 289
🌐 Web query:
Node.js node:sqlite DatabaseSync close() documentation open database file handles Windows fs.rmSync recursive behavior
💡 Result:
In Node.js, the DatabaseSync.close method is a wrapper around the SQLite native function sqlite3_close_v2 [1][2]. When called, it attempts to close the database connection and release associated file handles [1][2]. Regarding your specific concerns: 1. Windows File Locking and Deletion: SQLite, by design, requires all prepared statements to be finalized before a connection can be fully closed and the underlying file released [3]. If you encounter issues deleting a database file on Windows after calling close, it is often because there are unfinalized statements or active sessions associated with that connection [4][3]. Ensure that all statements created with prepare are explicitly finalized (or have their scope managed, if applicable) and that any active sessions are closed before calling database.close [4][3]. 2. Node.js Specific Behavior: Recent updates have addressed issues where DatabaseSync.close could be called reentrantly from within user-defined function callbacks, which previously caused segmentation faults [5][6]. Node.js now tracks callback depth and will throw an ERR_INVALID_STATE error if you attempt to close the database while a callback is still executing [6]. Additionally, ensure that you are not holding stale references to objects like sessions or prepared statements, as these may retain native handles that prevent the file from being released even after the main database connection is closed [4]. 3. fs.rmSync and Recursive Behavior: The fs.rmSync function in Node.js, when used with the { recursive: true } option, behaves as follows: - If the path points to a file, it deletes the file. - If the path points to a directory, it deletes the directory and all of its contents (recursively). - On Windows, if any file within that structure is locked by another process (or a lingering, unfinalized handle from your application), the operation will fail with an EBUSY or EPERM error. To reliably delete the database file, verify that the connection and all associated statements/sessions are truly closed. If using journal modes like WAL (Write-Ahead Logging), be aware that temporary files (such as -wal or -shm files) are also created and must be handled; deleting the main database file while these remain can result in errors or database corruption [3]. Top Results: [1][6][4][3]
Citations:
- 1: https://nodejs.org/api/sqlite.html
- 2: https://nodejs.org/api/sqlite.md
- 3: https://sqlite.org/forum/forumpost/ee70b06f80
- 4: sqlite: Session methods crash after DatabaseSync is closed and reopened nodejs/node#64782
- 5: node:sqlite segfaults when db.close() is called from a user-defined function callback during query execution nodejs/node#63180
- 6: sqlite: prevent database close during callbacks nodejs/node#64743
Close databases during all cleanup paths.
If measure throws after createFixture returns, the current close calls do not run. Move both close calls into finally before rmSync. If createFixture throws after opening DatabaseSync, close that local connection inside createFixture.
Grok Electron QA evidence (HEAD
|
| Previous capacity path | Measured time |
|---|---|
| Median per mutation | 32.620 ms |
| p95 per mutation | 41.840 ms |
| Maximum | 51.255 ms |
| Total for 64 mutations | 2,174.174 ms |
After
A trigger-maintained ledger provides the exact row count without scanning the receipt table. A partial covering index handles age retention and oldest-completed pruning, while pruning 64 completed rows at the high-water mark amortizes cleanup.
| New capacity path | Measured time | Improvement |
|---|---|---|
| Median per mutation | 0.031 ms | 1,055× faster |
| p95 per mutation | 0.045 ms | 929× faster |
| Maximum | 0.606 ms | 85× faster |
| Total for 64 mutations | 2.657 ms | 818× faster |
That removes 32.589 ms from the median mutation and 2,171.517 ms across the 64-mutation run. The median capacity check is now effectively sub-millisecond while preserving the 10,000-row hard cap, 30-day retention, oldest-first replay retention, pending-receipt fail-closed behavior, and exact counts for older binaries through database triggers.
Checkout / identity
- Exact HEAD:
2b7792f1d263572ed0d04d219d7ba1a5eb9f90e2 - Isolated launch:
ORCA_DEV_USER_DATA_PATH=/tmp/orca-qa-13647-…-userdata, CDP61992 window.api.app.getIdentity()→devRepoRoot: /Users/brennanbenson/orca/workspaces/orca/perf-mutation-ledger-capacity(isDev: true, branchbrennanb2025/perf-mutation-ledger-capacity)
UI exercise (CDP / agent-browser only)
- App loaded at
http://localhost:5179/with full chrome (sidebar, onboarding, projects empty state) - Opened Automations panel (templates + Overview/Runs visible); Settings click also exercised
- No Computer Use / AppleScript / OS input injection
Screenshots
Backend capacity validation (this profile + focused tests)
- Live
orchestration.dbPRAGMA user_version = 26 - Present:
idx_mutation_receipts_completed_updated,mutation_receipt_ledger(singleton, count=0), insert/delete count triggers EXPLAIN QUERY PLANoldest-completed walk:SCAN … USING COVERING INDEX idx_mutation_receipts_completed_updated(no temp B-tree)- Vitest:
mutation-receipt-capacity.test.ts— 4/4 passed - Light bench (
--iterations 8 --payload-bytes 1024): median ~63× speedup (legacy 11.21ms → optimized 0.18ms)
Logs / console
- CDP console during attach: 0 errors/exceptions (debug/info/warning only)
- Dev log noise only (unrelated): WS transport port contention on multi-instance host,
legacy worker provider-ready recovery failed: terminal_liveness_unavailable, Claude usage 429 — no mutation-receipt / schema-v26 failures
Cleanup
- Killed only the QA Electron/dev tree and removed temp user-data under
/tmp/orca-qa-13647-…
Residual risk
- Additive v26 migration still does one index-build scan on existing large receipt tables; older binaries keep correct trigger-maintained counts but retain slower capacity queries until upgraded. Fresh-profile QA did not load a 10k-row production WAL into the live app UI path.
1 participant
Summary
Performance
Reproduction:
On a 102,776,832-byte (98.02 MiB), 10,000-row WAL fixture:
That is a 1,055x median speedup and 818x total reduction in this run.
EXPLAIN QUERY PLANregression coverage verifies the retention seek and oldest-completed walk useidx_mutation_receipts_completed_updatedwithout a temporary B-tree.Validation
pnpm run typecheck:nodepnpm lintElectron evidence
Launched this checkout through
REMOTE_DEBUGGING_PORT=9336 node config/scripts/run-electron-vite-dev.mjs, attached only through CDP, and verified the visible app was rendered with zero console errors. The automation identity reporteddevRepoRootas/Users/brennanbenson/orca/workspaces/orca/perf-mutation-ledger-capacity; the live database reported schema v26, both count triggers, the ledger row, and the covering-index query plan.Residual risk
The additive migration performs one index-build scan on an existing receipt table. Older binaries can continue operating against the additive schema and keep the counter exact through database triggers, but they retain the slower capacity query until updated.