fix(data-imports): stop reporting transient S3 blips as compaction failures - #74630
Merged
trunk-io[bot] merged 1 commit intoJul 29, 2026
Merged
Conversation
…ilures Applies the existing `is_transient_object_store_error` classifier (already used in `common/extract.py`'s pre-write maintenance path) to the two post-run compaction/vacuum call sites that were missing it: `_post_run_operations` in `pipeline_v2/pipeline.py`, and both branches of `run_post_load_operations` in `common/load.py`. All three sites already swallow the exception and let the sync complete either way; the only change is skipping `capture_exception` when the error is a recognized transient S3 blip. Branch: posthog-code/fix-vacuum-post-run-transient-s3-noise Generated-By: PostHog Code Task-Id: 2e3670ef-30aa-4461-995e-ddadf81ee399
|
😎 Merged successfully - details. |
Contributor
|
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 |
2 tasks
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Contained fix that only changes error-tracking noise classification for an already-caught, non-fatal exception path (no behavior/retry change), with regression tests covering both branches; author is on the owning team.
- Author wrote 0% of the modified lines and has 11 merged PRs in these paths (familiarity MODERATE).
- 👍 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 | ✓ | 33L, 2F substantive, 141L/4F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1c-medium (141L, 4F, single-area, fix) |
| stamphog 2.0.0b3 | .stamphog/policy.yml @ 545937f · reviewed head c196c13 |
This was referenced Jul 29, 2026
Merged
Contributor
|
/trunk merge |
trunk-io
Bot
deleted the
posthog-code/fix-vacuum-post-run-transient-s3-noise
branch
July 29, 2026 13:03
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
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
An error tracking issue surfaced an
OSError: Generic S3 error ... 503 Service Unavailable ... SlowDown ... Please reduce your request rate, raised fromDeltaTableHelper.vacuum_table()(viacompact_table()), called from_post_run_operations()at the end of every v2-pipeline sync. The same fingerprint fires across many unrelated source types and schemas within the same short window, which points at S3 throttling a burst of concurrent post-sync vacuum/compact operations against the shared data-warehouse bucket, not a bug tied to any one source.Tracing the call site:
_post_run_operationsalready wrapscompact_table()in a baretry/except Exception: capture_exception(e)— so the sync itself is unaffected either way, this is purely error-tracking noise. The pipeline layer already has a classifier for exactly this failure mode,is_transient_object_store_error()indelta_table_helper.py, and the pre-write maintenance path (common/extract.py'srun_pre_write_defensive_compact) already uses it to log a warning instead of capturing. The post-run path inpipeline_v2/pipeline.py, and the equivalent post-load paths incommon/load.py(CDC maintenance and non-CDC compact), never picked it up, so the exact same transient S3 blip is treated inconsistently depending on which call site hits it.Changes
pipeline_v2/pipeline.py's_post_run_operations: checkis_transient_object_store_error(e)before capturing acompact_table()failure; log a warning instead when it's a recognized transient S3 blip.common/load.py'srun_post_load_operations: same treatment for both the CDCrun_maintenance()branch and the non-CDCcompact_table()branch.No retry policy, timeout, or
NonRetryableErrorsclassification changed — this only makes an already-non-fatal exception path consistent with the classifier the pipeline already has, at the two call sites that were missing it.How did you test this code?
pipeline_v2/test/test_pipeline.pywith a parameterizedtest_compaction_error_handling(transient S3 blip → warning, no capture; genuineOSError→ captured) — the exact regression this PR fixes.common/test/test_load.py'sTestRunPostLoadDeltaMaintenancesimilarly for both the CDC maintenance path and the non-CDC compact path.test_pipeline.py(pipeline_v2),test_load.py(common), andtest_delta_table_helper.py(core) suites: 82 passed, 4 pre-existing failures inTestGetDeltaTableUnrecoverableErrorsthat need a live object-storage service unavailable in this sandbox — confirmed identical on unmodifiedmaster.ruff check/ruff format --checkclean on all 4 changed files.uv run mypy --cache-fine-grained .clean (17074 source files).Docs update
N/A — internal pipeline implementation detail, no user-facing or API surface change.
🤖 Agent context
Autonomy: Fully autonomous
Triaged from a live PostHog error-tracking issue (webhook-delivered) using Claude Code. Pulled the stack trace, exception samples, and
warehouse_sources_*properties via the PostHog MCP tools to confirm the failure is cross-source (Postgres, MySQL, GoogleAds, GitHub) and lives entirely in shared pipeline code. Searched open PRs for duplicates: found #69306 addressing the same root cause with a similar design, but it targetspipelines/pipeline/delta_table_helper.py, a path removed by a since-refactor (nowpipelines/core/), so it can't land as-is; also checked #74541 (a different bug in the same file) for overlap — none. Invoked/writing-testsbefore adding test coverage.