fix(clickhouse): connect to the persons queue inside the sweep op - #92072
Open
eli-r-ph wants to merge 1 commit into
Open
fix(clickhouse): connect to the persons queue inside the sweep op#92072eli-r-ph wants to merge 1 commit into
eli-r-ph wants to merge 1 commit into
Conversation
A dry run of the deletion sweep failed at Postgres resource init on a network path a dry run never uses, and a resource-init failure fires no failure hook, so the run's dictionaries survived on every host. The op now receives the connection URL and connects after the dry-run return, inside the step, with a ten-second connect timeout on the shared Postgres resources.
|
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 |
Contributor
🤖 CI report
|
eli-r-ph
marked this pull request as ready for review
September 1, 2026 00:12
Contributor
|
Reviews (1): Last reviewed commit: "fix(clickhouse): connect to the persons ..." | Re-trigger Greptile |
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
1 participant
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
The first prod dry run of
clickhouse_deletion_sweep_jobfailed on a Postgres connection a dry run never uses. Thepersons_databaseresource opens its connection at resource init, sopersist_deleted_personsdialed the persons writer before its dry-run check ran, and the closed network path timed out after two silent minutes. A resource-init failure emits noSTEP_FAILURE, sodrop_assets_on_failurenever fired and the run's two dictionaries (about 18 GiB on each of 24 hosts) survived until dropped by hand.Changes
persist_deleted_personsreceives the connection URL (newPostgresURLresource,persons_database_url) and connects inside the op body, after the dry-run return, closing the connection in afinally.PostgresResourcegainsconnect_timeout=10, so a blackholed route fails in seconds with a clear error instead of hanging for the OS TCP timeout. This is the one part of the diff that touches other jobs; the rest is confined to the sweep.How did you test this code?
psycopg2.connectpatched to raise never calls it.hogli review(the local Greptile CLI fails to install in this environment).🤖 Agent context
Authored with Claude Code in an interactive session with Eli, from the failed prod-eu run's debug log (
RESOURCE_INIT_FAILUREonpersons_database, psycopg2 connection timeout to the persons writer). Skills invoked:/writing-tests,/writing-code-comments,/writing-pr-descriptions. Companion charts PR PostHog/charts#14920 attaches the persons client security group to the Dagster node class, which is the network half of the fix. Kept the existing eagerpersons_databaseresource for its other consumers; only the sweep op moved to the lazy URL resource.