mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(db): bound lock waits and idle transactions so a parked coroutine can't wedge the pool (#721)
* fix(db): bound lock waits and idle transactions so a parked coroutine can't wedge the pool 2026-07-29 production incident: verb/evidence handlers and background dispatch coroutines held an open DB transaction across minutes of git subprocess work and asyncio lock queues (per-workspace ensure locks). Early writes in those transactions held tasks/agents row locks, every other write convoyed behind them, and blocked statements camped on pool connections until all 30 were waiters — 1000+ QueuePool timeouts per hour, one transaction open 1h20m. Two layers: - get_engine now passes asyncpg server_settings: idle_in_transaction_session_timeout (default 120s) kills any session parked mid-transaction on non-DB work, releasing its locks and pool slot; lock_timeout (default 30s) makes a statement queued on someone else's row lock give up instead of holding a connection for the wait. Both env-tunable (ROBOCO_DATABASE_IDLE_IN_TRANSACTION_TIMEOUT_MS / ROBOCO_DATABASE_LOCK_TIMEOUT_MS), 0 disables. Alembic runs its own sync engine and is untouched; best-effort writers (proactive-context injection) already swallow errors and now fail in 30s instead of camping for an hour. - ContentActions.evidence commits the request session before its fetch/diff git work, so a multi-minute evidence call no longer pins a pool connection for the duration (expire_on_commit=False keeps the loaded task usable; later reads reopen a transaction on demand). The deeper restructuring — claim flows committing their transition before briefing/workspace assembly — is scoped to the existing evidence-assembly-timeout task and not attempted here. * fix(db): bound lock waits and idle transactions so a parked coroutine can't wedge the pool 2026-07-29 production incident: verb/evidence handlers and background dispatch coroutines held an open DB transaction across minutes of git subprocess work and asyncio lock queues (per-workspace ensure locks). Early writes in those transactions held tasks/agents row locks, every other write convoyed behind them, and blocked statements camped on pool connections until all 30 were waiters — 1000+ QueuePool timeouts per hour, one transaction open 1h20m. Two layers: - get_engine now passes asyncpg server_settings: idle_in_transaction_session_timeout (default 20 min) kills any session parked mid-transaction on non-DB work, releasing its locks and pool slot; lock_timeout (default 60s) makes a statement queued on someone else's row lock give up with a clean retryable error instead of camping on a pool connection for the wait. Both env-tunable (ROBOCO_DATABASE_IDLE_IN_TRANSACTION_TIMEOUT_MS / ROBOCO_DATABASE_LOCK_TIMEOUT_MS), 0 disables. The idle default deliberately clears the longest LEGITIMATE in-transaction window — a cold-workspace claim holds its transaction across the clone (300s budget) + dep install (600s budget) under the 900s slow-verb wall — so routine claims never trip it while today's 80-minute parked transaction dies at 20 min. Alembic's env.py builds its own engine and never carries these; best-effort writers (proactive-context injection) already swallow errors and now fail in 60s instead of camping for an hour. - ContentActions.evidence ends the request transaction (commit, or rollback on a poisoned session) before its fetch/diff git work, so a multi-minute evidence call no longer pins a pool connection for the duration (expire_on_commit=False keeps the loaded task usable; later reads reopen a transaction on demand). The deeper restructuring — claim flows committing their transition before briefing/workspace assembly — is scoped to the existing evidence-assembly-timeout task and not attempted here. --------- Co-authored-by: Renn F <rennf93@users.noreply.github.com>
This commit is contained in:
@@ -146,6 +146,39 @@ async def test_evidence_populates_journal_highlights() -> None:
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_evidence_commits_session_before_git_work() -> None:
|
||||
"""2026-07-29 pool exhaustion: evidence() must release its DB transaction
|
||||
(commit) BEFORE the fetch/diff git work — those can run for minutes on a
|
||||
cold workspace, and an open transaction pins a pool connection for the
|
||||
whole duration."""
|
||||
order: list[str] = []
|
||||
agent_id = uuid4()
|
||||
task_id = uuid4()
|
||||
task_svc = AsyncMock()
|
||||
task_svc.get.return_value = _task_with_pr(task_id, commits=["abc"])
|
||||
task_svc.session.commit = AsyncMock(side_effect=lambda: order.append("commit"))
|
||||
git_svc = AsyncMock()
|
||||
git_svc.diff.return_value = ""
|
||||
git_svc.list_changed_files.return_value = []
|
||||
workspace_svc = AsyncMock()
|
||||
workspace_svc.fetch_branch_for_inspection = AsyncMock(
|
||||
side_effect=lambda **_kw: order.append("fetch")
|
||||
)
|
||||
evidence_repo = AsyncMock()
|
||||
evidence_repo.journal_highlights_for_task.return_value = []
|
||||
|
||||
ca = ContentActions(
|
||||
_deps_for_evidence(task_svc, git_svc, workspace_svc, evidence_repo)
|
||||
)
|
||||
env = await ca.evidence(agent_id=agent_id, task_id=task_id)
|
||||
|
||||
assert env.as_dict()["error"] is None
|
||||
assert order == ["commit", "fetch"], (
|
||||
f"session must be committed before git work, got order={order}"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_evidence_no_branch_skips_git_calls() -> None:
|
||||
"""A task without a branch_name has no PR yet — skip git entirely,
|
||||
|
||||
Reference in New Issue
Block a user