Skip to content

fix(data-imports): stop reporting duckgres sink maintenance timeouts as errors - #73565

Merged
Gilbert09 merged 1 commit into
masterfrom
posthog-code/duckgres-maintenance-timeout-noise
Jul 28, 2026
Merged

fix(data-imports): stop reporting duckgres sink maintenance timeouts as errors#73565
Gilbert09 merged 1 commit into
masterfrom
posthog-code/duckgres-maintenance-timeout-noise

Conversation

@Gilbert09

Copy link
Copy Markdown
Member

Problem

Error tracking surfaced a QueryCanceled: canceling statement due to statement timeout from products/warehouse_sources/backend/temporal/data_imports/pipelines/pipeline_v3/duckgres/consumer.py, raised inside _run_maintenance's call to DuckgresBatchQueue.supersede_replaced_runs in jobs_db.py.

_run_maintenance runs every ~30s to sweep obsolete runs and refresh backlog gauges. Its three queries (supersede_replaced_runs, get_backlog_stats, count_orgs_at_budget) are already wrapped in a 30s statement_timeout (ELIGIBILITY_QUERY_STATEMENT_TIMEOUT_MS) specifically so a slow/loaded queue DB fails fast instead of stacking up long-running scans — the code comment right above the call already says a timeout here "must not wedge or crash the poll loop — skip this tick and retry on the next one."

The poll loop does correctly skip and retry: the except Exception around the whole maintenance block swallows the error so nothing crashes. But it also unconditionally calls capture_exception(e) for every exception in that block, including this expected, self-healing timeout. So the very safeguard designed to make timeouts benign was reporting each one to error tracking as if it were a defect.

Changes

  • Added is_eligibility_query_timeout next to ELIGIBILITY_QUERY_STATEMENT_TIMEOUT_MS in jobs_db.py — a small classifier for psycopg.errors.QueryCanceled, the expected shape of that bound tripping.
  • consumer.py's _run_maintenance now branches on it: a QueryCanceled logs a warning and skips the tick as before, without capture_exception. Any other, genuinely unexpected error in that block still gets captured, same as today.

This only changes what gets reported to error tracking — the swallow-and-retry behavior for the whole block is unchanged.

How did you test this code?

Extended the existing test_fetch_swallows_maintenance_query_timeout_and_skips_planner (which already asserted the timeout is swallowed and the planner/fetch are skipped) to also assert capture_exception is not called. Added a sibling test_fetch_reports_unexpected_maintenance_query_error asserting a genuinely unexpected error (e.g. a RuntimeError) still gets captured — locking in that the fix is scoped to the specific timeout case, not a blanket suppression.

Ran:

  • uv run pytest products/warehouse_sources/backend/temporal/data_imports/pipelines/pipeline_v3/duckgres/test_consumer.py -k "test_fetch_swallows_maintenance_query_timeout_and_skips_planner or test_fetch_reports_unexpected_maintenance_query_error" — 2 passed
  • uv run mypy --cache-fine-grained on the two changed source files — clean
  • ruff check / ruff format — clean
  • hogli ci:preflight --fix — 0 failures

The rest of the duckgres test suite (DB-backed integration tests) requires a running Postgres instance not available in this sandbox; those were not run, but this change doesn't touch any DB-facing query logic.

Docs update

N/A — internal error-classification change, no user-facing behavior change.

🤖 Agent context

Autonomy: Fully autonomous

Triaged a webhook-delivered PostHog error tracking issue using the PostHog MCP error-tracking tools (query-error-tracking-issue, query-error-tracking-issue-events) to pull the stack trace and sync context, then read consumer.py and jobs_db.py directly to confirm the timeout is already an expected, bounded condition per the existing code comments and a pre-existing test (test_fetch_swallows_maintenance_query_timeout_and_skips_planner) that only asserted the swallow, not the error-tracking report. Used an Explore-style subagent to check for a shared "transient DB error" classifier elsewhere in the pipeline (repartition_table.py's _is_transient_infra_error, delta_table_helper.py's object-store classifier, sources/postgres/postgres.py's QueryCanceled handling) — none covered this specific queue-DB maintenance path, so I added a small local classifier next to the timeout constant it's paired with. Invoked /writing-tests before extending the test. Checked for duplicate open PRs (exception type, message phrase, module path, and the maintainer's own open PRs) — found related-but-non-overlapping prior work (e.g. #73386, which classifies a different error type in a different file) rather than a duplicate.


Created with PostHog Code

…as errors

The eligibility-CTE maintenance queries (`supersede_replaced_runs`, `get_backlog_stats`, `count_orgs_at_budget`) are bounded by a 30s statement timeout specifically so a slow/loaded queue DB fails fast and the poll loop retries on the next tick — the surrounding code already documents this as expected. But the catch-all in `_run_maintenance` reported every exception, including that expected timeout, to error tracking as a defect.

Added `is_eligibility_query_timeout` next to the timeout constant in `jobs_db.py` and branched on it in `consumer.py`: a `QueryCanceled` from these queries now logs a warning and skips the tick as before, without `capture_exception`; any other, genuinely unexpected error still gets captured.

Generated-By: PostHog Code
Task-Id: 74c8471e-3e63-4af7-850f-5b69730bc85b
@github-actions

Copy link
Copy Markdown
Contributor

Hey @Gilbert09! 👋

It looks like your git author email on this PR isn't your @posthog.com address (owerstom@gmail.com). Since you're on the PostHog team, it's worth pointing your local git author email at your @posthog.com address. Why it matters:

  • Consistent work identity in git history — internal tooling that attributes commits to team members keys off your @posthog.com address.
  • Keeps team contributions easy to tell apart from external community ones when scanning history.

You can fix it for this repo with:

git config user.email "you@posthog.com"

Or set it globally with git config --global user.email "you@posthog.com". No need to redo this PR — just a nudge for next time. 🙂

@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team July 24, 2026 15:39
@Gilbert09 Gilbert09 added the stamphog Request AI approval (no full review) label Jul 24, 2026 — with PostHog

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Small, well-tested change that only reclassifies an already-swallowed, expected timeout so it stops polluting error tracking — no control-flow, data, API, or infra impact, and author is on the owning team.

  • 👍 on the PR from hex-security-app[bot].
Gate mechanics and policy version
Gate Result
prerequisites all clear
deny-list no deny categories matched
size 18L, 2F substantive, 62L/3F incl. docs/generated/snapshots — within ceiling
tier T1-agent / T1b-small (62L, 3F, single-area, fix)
stamphog 2.0.0b3 .stamphog/policy.yml @ 1920144 · reviewed head 3a9e27e

@Gilbert09
Gilbert09 merged commit 6718751 into master Jul 28, 2026
300 of 365 checks passed
@Gilbert09
Gilbert09 deleted the posthog-code/duckgres-maintenance-timeout-noise branch July 28, 2026 14:40
@deployment-status-posthog

deployment-status-posthog Bot commented Jul 28, 2026

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-07-28 15:22 UTC Run
prod-us ✅ Deployed 2026-07-28 15:45 UTC Run
prod-eu ✅ Deployed 2026-07-28 15:43 UTC Run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stamphog Request AI approval (no full review)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant