Skip to content

fix(sqlite): report lock contention as retryable - #512

Open
chuiwenwei wants to merge 1 commit into
EverMind-AI:mainfrom
chuiwenwei:fix/sqlite-lock-retryable
Open

chuiwenwei wants to merge 1 commit into
EverMind-AI:mainfrom
chuiwenwei:fix/sqlite-lock-retryable

Conversation

@chuiwenwei

@chuiwenwei chuiwenwei commented Oct 18, 2026 •

Copy link
Copy Markdown

Summary

  • classify SQLite SQLITE_BUSY / SQLITE_LOCKED failures as DatabaseBusyError
  • return 503 DATABASE_BUSY with Retry-After: 1
  • leave unrelated OperationalErrors unchanged
  • cover the lock → persistence translation → HTTP response path through the
    production handler set

Closes #498.

Automatic retry is intentionally avoided in the generic session_scope:
replaying an arbitrary transaction after the context body has executed could
duplicate caller-side effects. This change instead exposes an explicit
retryable API contract.

Area

  • Architecture method
  • Benchmark
  • Use case
  • Documentation
  • Developer experience
  • CI, build, or release

Verification

make lint        passed
make test        2704 passed, 4 skipped
make integration 187 passed, 7 skipped, 7 deselected
make package     sdist + wheel built; wheel import smoke passed
make docs-check  passed

Additional regression coverage:

  • deterministic lock contention using BEGIN IMMEDIATE
    (tests/integration/test_api/test_sqlite_busy_retry.py)
  • unrelated OperationalError remains unchanged
  • API response is 503 DATABASE_BUSY with Retry-After: 1
  • the same HTTP request succeeds after the lock is released

Checklist

  • I kept the change scoped to the relevant area.
  • I am opening this from a separate branch, not pushing directly to main.
  • I updated docs, examples, or setup notes when behavior changed.
  • I added or updated tests when the change affects behavior.
  • I did not commit secrets, .env files, dependency folders, or generated output.
  • Active relative links in Markdown files resolve.

Notes for Reviewers

The classifier prefers SQLite's primary result codes and retains message
matching as a fallback for wrappers that do not expose sqlite_errorcode.

Please pay particular attention to whether DATABASE_BUSY should remain a
dedicated public error code or reuse the generic infrastructure error code.

  • The end-to-end test uses a minimal app with the production handler set,
    not the full /flush route; /flush reaches the same session_scope.
  • DatabaseBusyError is a StorageError today. If you want the cascade
    worker to retry it inline (like VectorStoreBusyError), moving it under
    ExternalServiceError or adding a retryable-storage branch is a small
    follow-up — happy to do it.
  • The OME's raw aiosqlite store is a separate path and is not covered here.
  • The template has no "core runtime" area; I marked Architecture method
    following fix(reflection): batch source deprecation, revert it on failure, cap sources per merge #445.

By submitting this pull request, I agree that my contribution is licensed under
the Apache License 2.0.

A SQLite writer that loses the lock race raises OperationalError and
becomes HTTP 500. The condition is transient, so session_scope now
classifies it as DatabaseBusyError and the API maps that to 503 +
Retry-After: 1 with the DATABASE_BUSY code.

Non-lock OperationalErrors still propagate unchanged, so a malformed
statement is not mislabelled as retryable.

Refs EverMind-AI#498.

This branch has not been deployed

No deployments
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.

flush returns 500 when SQLite write hits the busy timeout

1 participant