fix(snowflake): stop the query timebomb cancelling long syncs - #70271
Conversation
The connector reuses `network_timeout` as a client-side query timebomb that cancels the running query once it elapses (ProgrammingError 000604/57014), so the finite `network_timeout` we set capped every query's total wall-clock at 300s and killed legitimate long-running syncs of large tables. Move the bound to `socket_timeout`, which limits each individual socket connect/read (catching a stalled/half-open connection, the original intent) without arming the query timebomb. Snowflake executes queries via bounded polling round-trips, so a long query never trips a per-socket timeout. Generated-By: PostHog Code Task-Id: 482cff6c-b059-47e6-a815-3faeffe79d51
|
Hey @Gilbert09! 👋 It looks like your git author email on this PR isn't your
You can fix it for this repo with: git config user.email "you@posthog.com"Or set it globally with |
|
Reviews (1): Last reviewed commit: "fix(snowflake): bound stalls with socket..." | Re-trigger Greptile |
The connector reuses `network_timeout` as a client-side query timebomb that cancels the running query once it elapses (ProgrammingError 000604/57014), so the 300s `network_timeout` we set to bound the per-request retry budget also capped every query's total wall-clock and killed legitimate long-running syncs of large tables. Keep `network_timeout` for the retry budget, but pass an explicit, much larger per-query `timeout` to the full-table data scans (the COUNT(*) estimate and the streaming SELECT). That overrides the timebomb for those queries while leaving the retry budget intact, and defers the real ceiling to Temporal's 6h import activity timeout. Generated-By: PostHog Code Task-Id: 482cff6c-b059-47e6-a815-3faeffe79d51
|
There was a problem hiding this comment.
Small, focused fix to a warehouse-sync timeout bug by an owning-team author with tests added; the one substantive reviewer concern (unbounded retry budget) was addressed in the current diff and acknowledged by Greptile's 👍, and this isn't schema/API/auth/billing/CI risky territory.
- Author wrote 67% of the modified lines and has 6 merged PRs in these paths (familiarity MODERATE).
- Gilbert09 reviewed the current head.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 21L, 1F substantive, 30L/2F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1b-small (30L, 2F, single-area, fix) |
| stamphog 2.0.0b3 | .stamphog/policy.yml @ 72f9c2a · reviewed head 5f97442 |
Problem
Snowflake imports were failing with a
ProgrammingError:Surfaced by error tracking: issue
019f5495. The raising frame isget_rowsinsnowflake.py, atstreaming_cursor.execute(query, params)— the streaming data query for a table sync.Root cause is our own config, not the customer's. We set
network_timeout=300on the connection to bound the per-request retry budget (so a stalled/half-open connection fails fast instead of hanging the worker thread). But the connector reusesnetwork_timeoutfor a second, unrelated thing: a client-side query timebomb that arms a timer oncursor.execute()and cancels the running query once the timeout elapses (cursor.py:real_timeout = timeout or network_timeout, then cancel →ProgrammingError000604/57014).So the 300s
network_timeoutcapped every query's total wall-clock at 300s. The import sync activity has a 6hstart_to_close_timeout, and a large table's streamingSELECT ... ORDER BY(or theCOUNT(*)estimate) can easily take longer than 5 minutes to execute — at which point the timebomb cancels it. Retrying can't help because it's a fixed function of table size, so this is a bug in our code, not a transient or upstream error.Changes
Keep
network_timeoutfor the retry budget, but stop it from cancelling long queries.The retry budget is applied per HTTP request, so it never caps a query's total wall-clock — only the timebomb does. The connector lets you override the timebomb per query via
cursor.execute(timeout=...). So the full-table data scans (theCOUNT(*)estimate and the streamingSELECT) now pass an explicit, much larger per-query timeout, which overrides the timebomb for those queries while leaving the retry budget intact. That value mirrors the import activity's 6hstart_to_close_timeout— Temporal's activity timeout is the real ceiling; the connector defers to it rather than pre-empting a sync that's still making progress.Short metadata queries (schema/PK discovery) keep the default 300s timebomb — they're fast and run in the 10-minute discovery activity, so an early bound there is fine.
How did you test this code?
Automated tests only (I'm an agent — no manual Snowflake run):
TestBuildPipeline::test_builds_source_response_and_streamsto assert the streaming scan runs with a per-querytimeoutabovenetwork_timeout— the regression guard: reusing the 300snetwork_timeoutas the query cap is exactly what caused the incident.TestGetRowsToSync::test_returns_countto assert theCOUNT(*)probe carries the same long timeout (otherwise a large table trips the timebomb and the probe spuriously logs a 604).TestConnect::test_bounds_network_timeoutstill guards the retry-budget bound.pytest .../snowflake/tests/test_snowflake.py— 97 passed.🤖 Agent context
Autonomy: Fully autonomous
Triaged from an error-tracking webhook by Claude (PostHog Code). Confirmed the issue and frame via the error-tracking MCP tools, then read the installed
snowflake-connector-pythonsource to establish thatnetwork_timeoutdoes double duty: per-request retry budget (network.py) and a per-query cancellation timebomb (cursor.py). Initially considered swapping tosocket_timeout, but that drops the aggregate retry-budget protection (a flaky connection could then spin until the 6h Temporal activity timeout). The per-queryexecute(timeout=...)override keeps the retry budget and only lifts the cap where queries are legitimately long. Invoked the/writing-testsskill before touching tests.Created with PostHog Code