Skip to content

fix: back off SQLite log writes after lock errors#391

Open
liaohch3 wants to merge 1 commit into
mainfrom
fix/sqlite-log-backoff
Open

fix: back off SQLite log writes after lock errors#391
liaohch3 wants to merge 1 commit into
mainfrom
fix/sqlite-log-backoff

Conversation

@liaohch3

@liaohch3 liaohch3 commented Jul 17, 2026

Copy link
Copy Markdown
Owner

Summary

  • pause auxiliary SQLite log writes for one second after a storage error
  • retry automatically after the cooldown instead of disabling logging
  • cover the lock retry path with a focused regression test

Problem

A long-running Talon proxy entered a sustained timeout loop while the shared 23 GB trace database was under write contention. Current main already bounds SQLite waits and prevents trace storage errors from aborting requests, but SQLiteLogHandler still retried every log record immediately. Each retry could block synchronously for up to one second, so concurrent requests recreated prolonged request latency.

Before the fix, the focused regression test reproduced the retry storm and failed with assert 3 == 2. The handler now skips auxiliary log persistence during a one-second cooldown and retries afterward.

Validation

  • uv lock --check
  • uv run ruff check .
  • uv run ruff format --check .
  • uv run pytest tests/ -x --timeout=60 (989 passed, 25 skipped)
  • coverage policy: Python 84.33%; viewer JS/CSS gates passed
  • real Codex CLI 0.144.4 E2E through the local branch, including a six-call resumed turn with real shell tools, using an isolated SQLite database and gpt-5.4 via the ChatGPT OAuth target
  • screenshot and viewer validation passed with no warnings

Evidence

Real Codex resume trace viewer

Real E2E trace viewer source: the screenshot was captured from the viewer HTML exported by uv run claude-tap export bb2b323f-f40e-4a08-aa18-e21bdfdcd5fa --format html after a resumed Codex 0.144.4 session through the local branch, using the isolated /tmp/claude-tap-log-backoff-e2e/traces.sqlite3 database.

Co-authored-by: Talon <talon@users.noreply.github.com>
@liaohch3
liaohch3 force-pushed the fix/sqlite-log-backoff branch from 7a34b19 to 9acaa4a Compare July 17, 2026 01:04
@YoungCan-Wang
YoungCan-Wang self-requested a review July 18, 2026 05:08

@YoungCan-Wang YoungCan-Wang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] The one-second cooldown still permits repeated event-loop stalls under sustained lock contention.

A failed append_log() can already block for the full one-second SQLite/write-lock timeout. Because _retry_after is then set to only one second after that failure completes, continuous log traffic can still trigger another synchronous one-second write attempt roughly every two seconds.

I reproduced the timing with a store that consumes the busy timeout before raising: over 5.11 seconds, the handler made three blocking attempts, spending about 60% of the interval blocked. Since proxy request paths call log.info() synchronously, these retries can continue pausing the proxy event loop and may preserve the timeout behavior this PR is intended to fix.

The added regression test raises OperationalError immediately, so it verifies attempt counting but does not model the blocking duration. Please use a materially longer or exponential circuit breaker, temporarily disable auxiliary persistence, or move these writes off the request/event-loop path, and add a test where the failed write consumes the busy timeout before raising.

The rest of the change looks sound; this sustained-contention behavior is the blocking concern.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants