perf(performance-monitor): move blocking sqlite3 I/O off the event loop - #1200
perf(performance-monitor): move blocking sqlite3 I/O off the event loop#1200groupthinking wants to merge 1 commit into
Conversation
`PerformanceMonitor` used the fully synchronous `sqlite3` driver directly inside six `async def` methods. Each call ran connect + statement + commit (which fsyncs) + close on the event loop, so nothing else on the loop could be scheduled for the duration of the disk write. This is on a live request path: `api/v1/router.py:69` imports the monitor and awaits `record_metric()` at `:1165` and `:1183`, and `record_metric` calls `_store_metric` unconditionally. Each method's database work is now a nested synchronous function dispatched via `await asyncio.to_thread(...)`. Statement text, transaction boundaries, return values and error handling are unchanged; only the thread the work runs on changes. Connections are created and closed inside each call, so there is no shared handle a caller could tear down mid-flight. Sites moved off-loop: _store_metric, _store_alert, _basic_cleanup, get_current_performance_summary, _get_recent_metrics_summary, _store_benchmark_result `_init_database` is left alone: it is a synchronous method called from `__init__`, so it never runs on the event loop. Tests: 113 pre-existing pass with zero edits, plus 6 new in `TestSqliteDoesNotBlockEventLoop`. Non-vacuity was proven by behavioural mutation: replacing all six `to_thread` dispatches with direct calls fails exactly the 3 heartbeat tests and no others. The other 3 new tests are preserved-behaviour guards -- they pass under both old and new code and exist to pin behaviour the change must not regress, not to prove the change. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
Agent Completion Truth Gate: BLOCKEDReasons: Machine-readable verdict{
"details": {
"invalid_fields": [
"issue.number",
"policy.agent_login",
"policy.run_id"
]
},
"reasons": [
"invalid_payload"
],
"verdict": "blocked"
} |
CI status & blockers (head
|
|
Closing as a duplicate — no action needed. This PR carries the exact same commit ( I opened #1200 from my working branch ~20 min after #1196 had already merged-and-closed, so #1196 didn't appear in the open-PR scan. The two red checks discussed above ( Generated by Claude Code |
1 participant
Canonical issue
Closes #
Outcome
PerformanceMonitorused the fully synchronoussqlite3driver directly inside sixasync defmethods. Each call ran connect + statement + commit (fsync) + close on the event loop, stalling every other coroutine for the duration of the disk write. Becauserecord_metric()is awaited on live request paths (api/v1/router.py:1165and:1183), each metric write blocked request handling. This PR moves that I/O onto a worker thread so the loop stays responsive.Scope
_store_metric,_store_alert,_basic_cleanup,get_current_performance_summary,_get_recent_metrics_summary,_store_benchmark_result— each DB block is now a nested sync function dispatched viaawait asyncio.to_thread(...). Statement text, transaction boundaries, return values, and error handling are unchanged; only the thread the work runs on changes._init_database— it is a synchronous method called from__init__, so it never runs on the event loop.Risk
to_threadcall, so there is no shared handle a caller could tear down mid-flight; a thread-pool exhaustion would degrade to the same latency as before, not error.Verification
pytest tests/unit/test_performance_monitor.py: 119 passed (113 pre-existing unchanged + 6 new inTestSqliteDoesNotBlockEventLoop)to_threaddispatches with direct calls fails exactly the 3 heartbeat tests and no othersProduction evidence
Not applicable — internal service latency change with no user-facing surface; effect is reduced event-loop stall during metric persistence, covered by the heartbeat tests.
Agent handoff
main)Generated by Claude Code