fix(postgres): tolerate sources that reject SET statement_timeout - #70311
Conversation
The row-serving setup connection in postgres_source runs a session-level SET statement_timeout before streaming. Some Postgres-compatible engines and poolers reject it with FeatureNotSupported, which escaped the setup error handling and matched no non-retryable substring, so Temporal retried an import that could never succeed. Wrap the SET as best-effort, mirroring the metadata-probe sites in _get_table and _get_columns_for_tables: fall back to the source's default timeout on FeatureNotSupported, while still re-raising genuine connection drop/limit errors so the setup retry recovers on a fresh connection. Generated-By: PostHog Code Task-Id: b35a78a8-a943-46f1-908a-38022bf8acf9
|
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(postgres): tolerate sources that rej..." | Re-trigger Greptile |
🤖 CI report
|
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Contained, well-tested fix: wraps one previously-unguarded SET statement_timeout call in the same best-effort error handling already used at three other sites in this file, reusing existing helper functions; the bulk of the diff is unrelated regenerated test snapshots. Not risky territory (data-warehouse batch import setup, not auth/billing/schema/API/ingestion-path), author is on the owning team, and the added regression test demonstrates the fix.
- Author wrote 1% of the modified lines and has 38 merged PRs in these paths (familiarity MODERATE).
- 👍 on the PR from greptile-apps[bot], hex-security-app[bot].
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 19L, 1F substantive, 14086L/4F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (14086L, 4F, two-areas, fix) |
| stamphog 2.0.0b3 | .stamphog/policy.yml @ be35044 · reviewed head 23556ab |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
1 participant
Problem
Error tracking surfaced a Postgres data-warehouse import failing repeatedly with:
Error tracking issue: https://us.posthog.com/project/2/error_tracking/019f57ed-b58f-7421-badd-25948364f78d
The real stack originates in the Postgres source's row-serving setup (
postgres_source→source_for_pipeline), where we run a session-levelSET statement_timeouton the setup connection before streaming rows. Some Postgres-compatible engines and connection poolers speak the wire protocol but rejectSET statement_timeout, raisingFeatureNotSupported(SQLSTATE0A000).That SET was the one unguarded
SET statement_timeoutcall left in this file. The three other sites (the metadata probes in_get_tableand_get_columns_for_tables, and the streaming-connection setup) already treat it as best-effort and fall back to the source's default. The row-serving setup site did not, so the error escaped the setup error handling, matched noget_non_retryable_errorssubstring, and Temporal retried an import that can never succeed.The
statement_timeoutwe set here is a safety net, not a correctness requirement. Crashing the whole sync because a source won't accept it is strictly worse than running without it.Changes
Wrap the row-serving
SET statement_timeoutin the same best-effort guard the metadata sites use:FeatureNotSupported(or any other non-droppsycopg.Error), log at debug and fall back to the source's default timeout instead of failing the sync.This is a fixable-bug fix (missing graceful-skip), not a
NonRetryableErrorschange. The connection is autocommit, so no rollback is needed.How did you test this code?
I'm an agent. I added a regression test (
TestSetupStatementTimeoutUnsupported) that drivespostgres_sourcethrough setup with a mock connection whose setup cursor rejectsSET statement_timeoutwithFeatureNotSupported, and asserts the import completes and streams to exhaustion instead of raising. I verified it fails against the unfixed code (raisesFeatureNotSupportedat theSET statement_timeoutsite) and passes with the fix.The 3 errors are pre-existing environment failures — those tests need a live Postgres connection that isn't available in this sandbox (DNS "Name or service not known"), unrelated to this change.
ruff checkandruff formatare clean.🤖 Agent context
Autonomy: Fully autonomous
Authored by an agent triaging a live error-tracking issue for the Postgres warehouse source. I confirmed the crash originates in the source's own setup code (
postgres_sourceat theSET statement_timeoutcall, intopsycopgexecute), not in serialized log context. I classified it as a fixable bug rather than aNonRetryableErrorsaddition because the other threeSET statement_timeoutsites in the same file already fall back gracefully — this site simply missed the guard. Invoked the/writing-testsskill before adding the regression test. Checked open PRs (including #66527, which handles a differentFeatureNotSupported— a customer view expression matched onnot supported for type— and #70293, a setup commit-nesting fix) and confirmed neither addresses this error.Created with PostHog Code