Skip to content

FIX: SQLite analytics timeouts - #3065

Open
Roman Lutz (romanlutz) wants to merge 1 commit into
microsoft:mainfrom
romanlutz:romanlutz-analytics-timeout-investigation
Open

Roman Lutz (romanlutz) wants to merge 1 commit into
microsoft:mainfrom
romanlutz:romanlutz-analytics-timeout-investigation

Conversation

@romanlutz

Copy link
Copy Markdown
Contributor

Description

SQLite's native busy timeout accumulates requested sleep durations rather than enforcing an absolute deadline. This can make analytics lock waits outlast their shared request budget. The existing lock-wait test failed both on pre-PR main and during the second merge-queue attempt for #3048; the failures were not caused by that PR's changes.

  • Disable native busy sleeps only on the request-owned analytics connection. Retry only plain SQLITE_BUSY reads and consistent-report setup with paced asynchronous waits against the existing QueryControl deadline.
  • Keep the same statement, session, and read transaction across retries. Do not retry stale snapshots, extended busy codes, or unrelated database errors. SQL Server timeout handling and public reader contracts remain unchanged.
  • Finish connection-setting restoration under repeated task cancellation using the existing SQLite cleanup helper. Restore the original busy timeout, remove the request's progress handler, and discard the connection if a reset fails while preserving the original cancellation.

Existing deadlines, assertions, and coverage thresholds are unchanged. Retries add wakeups and do not provide a real-time guarantee when the OS deschedules the process. This addresses the verified lock-wait limitation, not the separate first macOS report timeout, whose historical cause remains undetermined.

This is complementary to the SDK work in #3059: the SDK owns admission and response capacity, while the reader owns database retries and cleanup. The public methods and result shapes are preserved, but the combined SDK branch has not been tested.

Tests and Documentation

Added regression coverage for lock release, deadline expiry, bounded retry sleeps, cancellation, repeated cancellation during cleanup, connection reset failures, all analytics projections and report setup, non-retryable errors, and unchanged SQL Server behavior. Updated the memory-models guide to describe the retry and cleanup policy.

Windows / Python 3.12.13, using this worktree's uv-managed environment. Explicit invocation through uv run --no-project avoided a local uv interpreter-discovery stall.

Parallel analytics and SQLite cancellation suites with sysmon coverage:

$env:COVERAGE_CORE = 'sysmon'
uv run --no-project -- .\.venv\Scripts\python.exe -m pytest -n 4 --dist=loadfile tests\unit\memory\test_attack_analytics.py tests\unit\memory\test_attack_analytics_lock_retry.py tests\unit\memory\test_attack_analytics_eval_identity.py tests\unit\memory\test_attack_analytics_metadata.py tests\unit\memory\test_sqlite_cancellation.py --cov=pyrit.memory.attack_analytics --cov-report=term-missing -q --durations=8

Result: 226 passed, with 93% focused module coverage. Two additional regression cases were subsequently added without changing production code.

Final new regression suite:

uv run --no-project -- .\.venv\Scripts\python.exe -m pytest tests\unit\memory\test_attack_analytics_lock_retry.py -q --durations=5

Result: 41 passed.

Original deadline bound, consistent-report snapshot, and ODBC timeout regressions:

uv run --no-project -- .\.venv\Scripts\python.exe -m pytest tests\unit\memory\test_attack_analytics.py -k 'deadline_bounds_file_database_lock_wait or report_is_consistent_and_later_result_pages_are_fresh or odbc_timeout_is_applied_before_each_cursor' -q --durations=5

Result: 5 passed, 119 deselected, with original assertions unchanged.

All applicable commit-time hooks passed, including Ruff, repository-wide production typing, async naming, and documentation structure validation. No hooks were skipped.

Full-repository tests, live Azure SQL, macOS/Ubuntu execution, and combined validation with #3059 were not run. Local coverage is not a production latency claim or a full-repository coverage gate.

JupyText was not run: this changes Markdown documentation, not paired Python/notebook examples.

Retry plain SQLITE_BUSY reads and report setup asynchronously within the existing request deadline instead of using native SQLite busy sleeps. Preserve the read transaction, non-busy errors, and SQL Server behavior. Drain connection-setting restoration under repeated cancellation and discard connections when reset fails.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

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.

1 participant