fix(data-imports): retry transient DB connection drops on two import reads - #75737
Draft
posthog[bot] wants to merge 2 commits into
Draft
fix(data-imports): retry transient DB connection drops on two import reads#75737posthog[bot] wants to merge 2 commits into
posthog[bot] wants to merge 2 commits into
Conversation
…reads get_dot_notated_table_name and update_last_synced_at read PostHog's own Postgres through database_sync_to_async_pool with no retry, so a pooler blip surfaced as an uncaught OperationalError. Wrap both in the existing aretry_on_db_connection_drop helper, and extend the DjangoOperationalError -> PostHogInternalDatabaseError re-raise from should_produce_table to get_dot_notated_table_name so a persistent failure escapes clear of the substrings source classification keys on. Generated-By: PostHog Code Task-Id: 9b03d3e4-5b15-40d7-b569-de22408ef1fe
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
…ion-drop Generated-By: PostHog Code Task-Id: 9b03d3e4-5b15-40d7-b569-de22408ef1fe
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
OperationalErrors out of the data warehouse import path, straight into error tracking for the data imports team.database_sync_to_async_poolwith no retry and no error classification:CDPProducer.get_dot_notated_table_name()andupdate_last_synced_at().OperationalErrorfrom our DB stringifies the same way a customer's misconfigured source host does, so a source'sget_non_retryable_errorscan match it and permanently stop a healthy sync.should_produce_table()catchesOperationalErrorand re-raisesPostHogInternalDatabaseErrorfor exactly this reason.get_dot_notated_table_name()is its prerequisite and never got it.Changes
cdp_producer.get_dot_notated_table_namearetry_on_db_connection_drop+ re-raise asPostHogInternalDatabaseErrorpipeline_sync.update_last_synced_ataretry_on_db_connection_dropcdp_producer.should_produce_tableposthog/temporal/common/utils.py(evict the stale connection, retry once), same asrepartition_controller.py:80. Nothing hand-rolled.PostHogInternalDatabaseError, clear of the substrings source classification keys on.Note
update_last_synced_atretries a small read-then-save, which is idempotent, so the retry is safe there.How did you test this code?
Added two tests in
test_cdp_producer.py, mirroring the existingshould_produce_tableconnection-failure test:test_get_dot_notated_table_name_retries_once_on_connection_drop– the first ORM read raises, the second succeeds, and the resolved name still comes back. Catches removal of the retry wrapper, which no existing test covers.test_get_dot_notated_table_name_database_failure_stays_retryable– parameterized overOperationalErrorandInterfaceError; asserts a persistent failure surfaces asPostHogInternalDatabaseErrorwhose message matches none ofPostgresSource().get_non_retryable_errors(). This is the misclassification regression.What I could and could not run
ruff check/ruff format– cleantach check --dependencies --interfaces– cleanmypy --cache-fine-grainedon both changed modules – cleandjango_db(transaction=True)) and no database was available in this environment, so I did not execute them. No manual testing was done.Automatic notifications
Docs update
N/A, internal retry and error-classification change with no user-facing behavior change.
🤖 Agent context
Autonomy: Fully autonomous
I (Claude) worked from an inbox report on the error burst. I checked for competing work first: #72062 added the sync
retry_on_db_connection_drophelper and applied it tomark_initial_sync_complete, and #73386 broadens transient classification in pre-write maintenance. Neither touches these two call sites, so this is additive on top of the same pattern.I considered also wrapping
should_produce_table's_check()in the retry, but kept the change to the two unguarded reads the report identified rather than widening scope. I invoked/writing-testsand/writing-code-comments.Created with PostHog Desktop from this inbox report.