Skip to content

perf(server): compaction keeps its six-hour cadence across restarts - #21192

Closed
Alyx-ML wants to merge 1 commit into
pingdotgg:mainfrom
Alyx-ML:perf/compaction-cadence
Closed

Alyx-ML wants to merge 1 commit into
pingdotgg:mainfrom
Alyx-ML:perf/compaction-cadence

Conversation

@Alyx-ML

@Alyx-ML Alyx-ML commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

Database compaction runs one minute after every launch, whenever it last ran. A pass reads the whole event store, and its discovery queries ran on the synchronous connection, so the server's event loop stalled during the first minutes of every session. Relaunching never let the six-hour interval take effect.

Summary

 DatabaseStorage
-  sleep 1 minute, then compact every 6 hours
+  read <userdata>/compacted-at
+  sleep until due: clamp(last + 6 h − now, 1 minute, 6 hours)
+  compact every 6 hours, writing compacted-at after each pass
 ProjectionMaintenance discovery
-  sql`...`                          # synchronous connection
+  sql`...`.pipe(NodeSqliteClient.readOnly)  # reader worker when available

A missing or unreadable file keeps today's behavior: compact one minute after launch.

Evidence

The idle minute starts about one minute after launch, which is when compaction fires. The comparison is the same build with and without this change:

idle minute without with
app CPU 7.87% 2.78%
server CPU 6.45% 1.40%
total footprint 655 MB 476 MB
footprint after reopening the long chat 858 MB 644 MB
  • vp test run DatabaseStorage and runtimeLayer tests: 84 passed.

Measured on release desktop builds (build-desktop-artifact --target dir) on an Apple Silicon MacBook Pro running macOS 27.2. The seeded scratch HOME holds a 100,000-message chat and three chats streaming from a synthetic agent. The harness is in #21190.

Merge Danger

Door: two-way

Deleting compacted-at restores the old timing.

Blast Radius: small

Compaction can be postponed by up to six hours after a restart, which is the interval already intended.


Done with Claude Opus 5.5 in Claude Code.

🤖 Generated with Claude Code

Compaction ran one minute after every launch, scanning the whole event store on the synchronous connection and blocking the server. It now records when it last finished and waits until it is due (at least one minute, at most six hours), and its discovery reads run on the reader worker.

Done with Claude Opus 5.5 in Claude Code.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 18, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — This changes the default production maintenance schedule across restarts and adds persisted compaction state, rather than merely optimizing an isolated implementation. The reader-worker adjustment also changes execution behavior for compaction discovery, so the runtime impact warrants human review.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: ac460ebb-f85c-4c40-8cbd-7c953d920c2e
📥 Commits

Reviewing files that changed from the base of the PR and between 9c16146 and 512a53e.

📒 Files selected for processing: 2
  • apps/server/src/orchestration-v2/ProjectionMaintenance.ts
  • apps/server/src/persistence/DatabaseStorage.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

📝 Walkthrough

Projection discovery now routes event and command-receipt queries through the read-only SQLite client. Event-store compaction now uses a persisted completion timestamp to schedule runs at six-hour intervals.

Changes: Compaction maintenance

Layer / File(s) Summary
Read-only projection discovery
apps/server/src/orchestration-v2/ProjectionMaintenance.ts
Event and command-receipt discovery queries use NodeSqliteClient.readOnly. Discovery uses a reader worker when available; otherwise, it shares the synchronous connection and yields between pages when no rows are deleted.
Persisted compaction schedule
apps/server/src/persistence/DatabaseStorage.ts
Compaction reads its last successful completion time from compacted-at. Missing or invalid timestamps cause a one-minute initial delay; valid timestamps set a delay bounded between one minute and six hours. Successful runs record the current time, ignoring timestamp-write failures. Compaction failures remain logged, and runs repeat every six hours.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~15 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 512a5

The change preserves the six-hour compaction cadence across restarts and moves discovery reads to the reader worker when one is available. No actionable merge-blocking risk was identified.

Security architecture risk: 🔵 Low · up to 512a5

This changes when local database maintenance runs, without an identified expansion of data access or privileges. Scheduling failures are recoverable, but the new timing file can remain out of step with a restored database.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The directly affected assets are the configured local event database, its command receipts, and adjacent scheduling metadata. The inspected change does not add a cross-service destination, credential, infrastructure permission, or attacker-selected filesystem path.

Trust Boundaries and Controls

  • observed — The existing worker opens the same database with SQLite readOnly enabled and rejects non-WAL mode. Worker startup failure falls back to the existing main connection. The PR routes discovery SELECTs through this mechanism while leaving destructive SQL outside the read-only effects.

Resilience and Maintainability Implications

  • inferred — Malformed or future-dated scheduling metadata cannot produce an initial delay beyond six hours in a continuously running instance. The marker is not used to authorize access, select deletion candidates, or advance a replay cursor, limiting the demonstrated restore mismatch to maintenance timing.
Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Description check Warning The description explains the problem, implementation, impact, and verification results. However, it does not include the required Scope and approval section or explain why the change qualifies for an … Add a Scope and approval section. Link the triaged issue or maintainer approval, or explain why this focused fix is an obvious bug that does not require prior approval.
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: preserving the six-hour compaction cadence across restarts.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description explains the problem, implementation, impact, and verification results. However, it does not include the required Scope and approval section or explain why the change qualifies for an exception.

Resolution

Add a Scope and approval section. Link the triaged issue or maintainer approval, or explain why this focused fix is an obvious bug that does not require prior approval.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@juliusmarminge juliusmarminge self-assigned this Oct 19, 2026
@juliusmarminge

Copy link
Copy Markdown
Member

Taking over this one as maintainer for batch usage-billing-01. This combines persistent compaction cadence with reader-worker discovery queries, so I will rebuild it as two focused stack layers on origin, credit the original work, and close this PR only after the replacement layers are published and verified. No merge is planned.

@juliusmarminge

Copy link
Copy Markdown
Member

Thanks @Alyx-ML — the persisted six-hour cadence and use of the existing SQLite reader-worker route were the right pieces to address repeated startup scans and writer blocking. This original PR is now superseded by two focused replacement layers:

  • #21880: persist the last successful compaction completion and calculate the bounded startup delay.
  • #21881: route the two bounded discovery SELECTs through the existing reader worker while retaining writer-side deletion.

These concerns can be reviewed independently. I compared both replacement production diffs with the original: both behaviors are preserved, including zero-deletion completion timestamps, best-effort timestamp I/O, normal periodic scheduling, synchronous fallback, pagination and retention. The completion timestamp is now saved immediately after compaction, before logging. No intended behavior was dropped.

Both replacement heads have passing CI and two independent full-diff review rounds from Opus 5.5 and Sol 6 with no code blockers. Focused tests grew from 47 to 59; the cadence and actual reader-connection regressions have failing-control evidence. Verification output. The cadence layer still needs human policy approval; native platform/performance claims were not verified on this Linux host. Neither replacement has been merged.

Rebuilt with Codex gpt-6-astra for tests and gpt-6.1-sol for implementation through T3 Code, with credit to the original work.

juliusmarminge added a commit that referenced this pull request Oct 20, 2026
Rebuild the reader portion of PR #21192 by Alyx-ML; contributor credit retained in the replacement PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants