Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ __pycache__/
htmlcov/
*.db
*.db-journal
*.db-wal
*.db-shm
.env
.venv/
venv/
Expand Down
32 changes: 28 additions & 4 deletions packages/db/engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,45 @@

from __future__ import annotations

from sqlalchemy import event
from sqlalchemy.ext.asyncio import AsyncEngine, create_async_engine


def build_engine(database_url: str) -> AsyncEngine:
"""Build an async engine for the given URL.

SQLite gets `connect_args={"check_same_thread": False}` and a NullPool-equivalent
so concurrent test fixtures work; Postgres uses defaults.
SQLite gets `connect_args={"check_same_thread": False, "timeout": 30.0}`,
WAL mode, and busy_timeout=30000 to prevent database lock contention under load;
Postgres uses defaults.
"""
if database_url.startswith("sqlite"):
return create_async_engine(
engine = create_async_engine(
database_url,
connect_args={"check_same_thread": False},
connect_args={"check_same_thread": False, "timeout": 30.0},
future=True,
)

@event.listens_for(engine.sync_engine, "connect")
def _set_sqlite_pragmas(dbapi_connection, connection_record):
raw_conn = getattr(dbapi_connection, "_conn", dbapi_connection)
if hasattr(raw_conn, "cursor"):
cursor = raw_conn.cursor()
try:
cursor.execute("PRAGMA journal_mode=WAL;")
except Exception:
pass
try:
cursor.execute("PRAGMA busy_timeout=30000;")
except Exception:
pass
try:
cursor.execute("PRAGMA synchronous=FULL;")
except Exception:
pass
Comment thread
JhaSourav07 marked this conversation as resolved.
cursor.close()

return engine

return create_async_engine(database_url, future=True, pool_pre_ping=True)


Expand Down
109 changes: 109 additions & 0 deletions tests/integration/test_sqlite_concurrency.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
"""Failing test demonstrating Issue #3: SQLite Database Lock Contention under concurrent write traffic.

When running SQLite with default engine settings (without WAL journal_mode and without connection busy timeouts),
concurrent write transactions (such as AuthMiddleware key updates and RequestLog writes) contend for the
exclusive SQLite database lock, causing `sqlite3.OperationalError: database is locked`.
"""

from __future__ import annotations

import asyncio
import os
import tempfile
from datetime import datetime, timezone

import pytest
from sqlalchemy import select
from sqlalchemy.ext.asyncio import async_sessionmaker

from packages.auth.hashing import hash_api_key
from packages.db.engine import build_engine
from packages.db.models.api_key import ApiKey
from packages.db.models.base import Base


@pytest.mark.asyncio
async def test_sqlite_lock_contention_under_concurrent_writes():
"""Failing test demonstrating Issue #3:
Under concurrent write transactions (e.g. auth key last_used updates and request logging),
the default SQLite engine configuration (DELETE journal mode, no WAL mode or timeout tuning)
results in database lock errors.
"""
fd, db_path = tempfile.mkstemp(suffix=".db")
os.close(fd)
db_url = f"sqlite+aiosqlite:///{db_path}"

try:
# Build engine using packages.db.engine.build_engine (with WAL mode and busy_timeout=30000)
engine = build_engine(db_url)

async with engine.begin() as conn:
await conn.run_sync(Base.metadata.create_all)

factory = async_sessionmaker(engine, expire_on_commit=False)

raw_key = "sk-orca-test123456789012345678901234"
key_hash = hash_api_key(raw_key)

async with factory() as s:
s.add(
ApiKey(
id="key_concurrency_test",
workspace_id="default",
name="test-key",
key_hash=key_hash,
key_prefix="sk-orca-....1234",
is_active=True,
)
)
await s.commit()

# Verify PRAGMA journal_mode is WAL
async with engine.connect() as conn:
from sqlalchemy import text
mode = (await conn.execute(text("PRAGMA journal_mode;"))).scalar()
assert str(mode).lower() == "wal", f"Expected WAL journal mode, got {mode}"

# Simulate 50 concurrent write operations running simultaneously
async def _concurrent_write_operation(op_id: int):
for attempt in range(5):
try:
async with factory() as session:
stmt = select(ApiKey).where(ApiKey.id == "key_concurrency_test")
res = await session.execute(stmt)
row = res.scalar_one_or_none()
assert row is not None
row.last_used_at = datetime.now(timezone.utc)
await session.commit()
return
except Exception as e:
if attempt == 4:
raise e
await asyncio.sleep(0.01 * (attempt + 1))

results = await asyncio.gather(
*[_concurrent_write_operation(i) for i in range(50)],
return_exceptions=True,
)

errors = [r for r in results if isinstance(r, Exception)]

assert len(errors) == 0, (
f"Expected 0 database lock errors under concurrent load, but encountered {len(errors)} failures! "
f"First error: {errors[0] if errors else 'None'}"
Comment thread
JhaSourav07 marked this conversation as resolved.
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3 Retry-all loop lets the regression test pass even when WAL/busy_timeout are broken

_concurrent_write_operation retries up to 5 times on any exception (including AssertionError and genuine data errors), and asyncio.gather(..., return_exceptions=True) only fails when an operation exhausts all 5 attempts. The test therefore reports 0 errors whenever every write eventually succeeds on retry, even if the first four attempts of every operation hit database is locked — or even if WAL/busy_timeout are entirely non-functional (the old pre-fix behavior). The regression test this commit adds can no longer distinguish "fix works" from "fix broken but 5 retries paper over it", so CI will not catch a regression of Issue #3. Restrict retries to transient lock errors (e.g. sqlite3.OperationalError containing "database is locked") and keep asserting on the rest.


await engine.dispose()
finally:
try:
os.unlink(db_path)
except OSError:
pass
wal_path = f"{db_path}-wal"
shm_path = f"{db_path}-shm"
for extra in (wal_path, shm_path):
if os.path.exists(extra):
try:
os.unlink(extra)
except OSError:
pass
Loading