fix(core): retry transient SQLite lock-timeouts on durable event commits - #33159
fix(core): retry transient SQLite lock-timeouts on durable event commits#33159randomvariable wants to merge 2 commits into
Conversation
A concurrent MessageRemoved cascade-delete can remove an assistant message (and its parts) while an aborting fiber still flushes a trailing snapshot patch part. The PartUpdated projection then upserts a part whose message_id FK has no parent row; SQLite enforces the FK at COMMIT, the transaction re-fails as a SqlError, and Effect.orDie turns it into a defect that crashes the prompt fiber and prints a raw stack to the TUI with no log line. - event.ts: log (eventID/eventType/aggregateID) before orDie at the durable-commit funnel so DB commit failures are observable instead of surfacing only as a raw TUI stack. - projector.ts: in the PartUpdated projection, skip-and-warn when the parent message row is absent, mirroring the FK's onDelete: cascade intent. Replay-safe: fabricates nothing, deterministic under seq order. Fixes anomalyco#31990
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
|
The following comment was made by an LLM, it may be inaccurate: Related PR Found:
This PR is related because it also addresses SQLite |
Under multiple concurrent opencode processes sharing one SQLite DB, durable event commits (BEGIN IMMEDIATE) can exhaust busy_timeout and fail with a LockTimeoutError. That error flowed through the commit funnel's Effect.orDie and crashed the prompt fiber, halting the TUI. The subagent-interrupt feature (de994f3) amplified this by writing several durable rows per interrupt on the exact concurrent path. - event.ts: wrap the durable-commit transaction in a bounded Effect.retry gated to LockTimeoutError (exponential backoff + jitter, ~8 attempts) before the existing tapError + orDie. Retrying the whole BEGIN IMMEDIATE block is safe: rollback undoes all in-txn writes, seq is recomputed per attempt, and pubsub notify happens outside the txn so no event is double-emitted. Gate on reason._tag, not isRetryable, which is inverted in effect@4.0.0-beta.66. - database.ts: bump PRAGMA busy_timeout 5000 -> 10000. - sqlite.node.ts / sqlite.bun.ts: set PRAGMA busy_timeout = 10000 on every native connection (before WAL) for universal coverage.
210dd91 to
8bc1ed1
Compare
|
Automated PR Cleanup Thank you for contributing to opencode. Due to the high volume of PRs from users and AI agents, we periodically close older PRs using automated criteria so maintainers can focus review time on the most active and community-supported contributions. This PR was closed because it matched the following cleanup criteria:
PRs created within the last month are not affected by this cleanup. If you believe this PR was closed incorrectly, or if you are still actively working on it, please leave a comment explaining why it should be reopened. A maintainer can review and reopen it if appropriate. Thanks again for taking the time to contribute. |
1 participant
Issue for this PR
Related to #21215 and #19521.
Type of change
What does this PR do?
When several opencode processes share one SQLite database, durable event commits (
BEGIN IMMEDIATE) can exhaustbusy_timeoutand fail with aLockTimeoutError("database is locked"). That error flows through the event commit funnel'sEffect.orDie, which turns it into a defect that crashes the prompt fiber and halts the TUI mid-turn.Note on #21215: its premise was
busy_timeout=0. That has since been addressed —busy_timeout=5000is set in the pragma block (database.ts, added in 7f571d3). The remaining failure is genuine exhaustion of that 5s wait under heavy concurrent write contention, so a transient lock still crashes the fiber. High-frequency durable writes on the concurrent path (e.g. per-turn interrupt frames) make it easy to hit.This treats a transient lock as retryable instead of fatal:
event.ts: wrap the durable-commit transaction in a boundedEffect.retrygated toLockTimeoutError(exponential backoff + jitter, ~8 attempts) before the existingtapError+orDie. Retrying the wholeBEGIN IMMEDIATEblock is safe: rollback undoes all in-transaction writes,seqis recomputed from a fresh in-txn SELECT each attempt, and the pubsub notify happens outside the transaction, so no event is double-emitted. The gate keys onreason._tag === "LockTimeoutError", noterror.isRetryable, because ineffect@4.0.0-beta.66those flags are inverted (LockTimeoutError reports not-retryable, ConstraintError reports retryable); a comment pins this to the version.database.ts: bumpPRAGMA busy_timeout5000 -> 10000.sqlite.node.ts/sqlite.bun.ts: setPRAGMA busy_timeout = 10000on every native connection (before WAL) so all connections get the wait, not only the ones configured via the higher-level layer.busy_timeoutis the in-SQLite wait; the retry is the graceful outer fallback when even that wait is exceeded. They are complementary.This makes a transient lock benign and logged instead of fatal. It does not change the underlying write volume; reducing durable writes on the concurrent path and pruning the append-only event log are sensible follow-ups.
How did you verify your code works?
Screenshots / recordings
Not a UI change.
Checklist