Skip to content

fix(data-imports): retry delta vacuum on commit conflict - #76085

Open
Gilbert09 wants to merge 1 commit into
masterfrom
posthog-code/fix-delta-vacuum-commit-conflict-retry
Open

fix(data-imports): retry delta vacuum on commit conflict#76085
Gilbert09 wants to merge 1 commit into
masterfrom
posthog-code/fix-delta-vacuum-commit-conflict-retry

Conversation

@Gilbert09

Copy link
Copy Markdown
Member

Problem

An error tracking issue surfaced a CommitFailedError during delta maintenance, on the _vacuum frame in maintenance.py:

Failed to commit transaction: Commit failed: a concurrent transaction deleted data this operation read.
Help: This transaction's query must be rerun to exclude the removed data.

The call path: run_scheduled -> run_maintenance -> compact_if_fragmented -> _vacuum -> delta-rs's table.vacuum().

vacuum() commits a REMOVE of tombstoned files, so it's subject to delta-rs's conflict checker exactly like merge and optimize.compact — delta-rs doesn't retry this itself (see the comment on DELTA_MERGE_CONFLICT_RETRIES), because resolving it safely means re-reading the table and re-running the operation. _compact already routes through execute_with_conflict_retry for this reason, but _vacuum called table.vacuum() directly, so a concurrent maintenance pass touching the same table (e.g. a second Temporal activity attempt still finishing a compact after the first attempt's heartbeat timed out) made the CommitFailedError propagate straight out instead of retrying with a refreshed table.

This is best-effort maintenance code — the exception was already caught by run_scheduled's try/except and never re-raised, so no sync was ever at risk. This only affects whether a self-healing race gets reported as a defect.

Changes

  • Route _vacuum's table.vacuum() call through execute_with_conflict_retry, matching _compact.
  • Generalize execute_with_conflict_retry's return type (it was typed dict, matching optimize.compact()'s stats, but vacuum() returns a list of removed file paths).

How did you test this code?

  • Added TestVacuum.test_retries_on_commit_conflict_then_succeeds in test_maintenance.py, mirroring the existing compact regression test — asserts _vacuum retries once with a refreshed table on CommitFailedError instead of raising immediately.
  • Ran the full delta/test/ suite (51 tests) and mypy --cache-fine-grained repo-wide — both clean.

Docs update

N/A — internal maintenance-code fix, 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 and warehouse_sources_* event properties via the PostHog MCP tools to confirm the failing frame and reproduction context (an Intercom incremental sync), then traced the call path into delta/maintenance.py and compared _vacuum against _compact's existing conflict-retry handling.

Searched open PRs (by exception type, message phrase, module path, and the maintainer's own open PRs) before starting. Found related-but-distinct open/merged work on the same maintenance code (#75240, #73454, #74541, #74594) — all address different DeltaError signatures or a different re-fetch bug, none add commit-conflict retry to vacuum.

vacuum() commits a REMOVE of tombstoned files, so it's subject to delta-rs's conflict checker exactly like merge and optimize.compact — but unlike those two, it wasn't routed through execute_with_conflict_retry, so a CommitFailedError from a concurrent maintenance pass propagated straight out instead of retrying with a refreshed table.

Generated-By: PostHog Code
Task-Id: 20c39f3e-36bf-4381-af67-fb39807aaf94
@trunk-io

trunk-io Bot commented Jul 31, 2026

Copy link
Copy Markdown

Merging to master in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

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

@github-actions

Copy link
Copy Markdown
Contributor

Hey @Gilbert09! 👋

It looks like your git author email on this PR isn't your @posthog.com address (owerstom@gmail.com). Since you're on the PostHog team, it's worth pointing your local git author email at your @posthog.com address. Why it matters:

  • Consistent work identity in git history — internal tooling that attributes commits to team members keys off your @posthog.com address.
  • Keeps team contributions easy to tell apart from external community ones when scanning history.

You can fix it for this repo with:

git config user.email "you@posthog.com"

Or set it globally with git config --global user.email "you@posthog.com". No need to redo this PR — just a nudge for next time. 🙂

@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team July 31, 2026 19:49
@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

🤖 CI report

Backend coverage — all changed backend lines covered

🧪 Backend test coverage

Patch coverage — changed backend lines (products + core): ████████████████████ 100.0% (11 / 11)

All changed backend lines are covered ✅

Per-product line coverage (touched products)
Product Coverage Lines
demo ███████████░░░░░░░░░ 56.3% 1,497 / 2,661
tasks ██████████████░░░░░░ 70.3% 33,203 / 47,255
signals ████████████████░░░░ 81.4% 25,289 / 31,055
cdp ████████████████░░░░ 82.1% 3,285 / 3,999
data_modeling █████████████████░░░ 85.7% 7,716 / 9,008
notebooks █████████████████░░░ 86.0% 7,794 / 9,060
actions █████████████████░░░ 86.6% 717 / 828
cohorts █████████████████░░░ 87.5% 6,393 / 7,309
exports ██████████████████░░ 87.9% 7,078 / 8,054
product_tours ██████████████████░░ 87.9% 1,303 / 1,482
data_warehouse ██████████████████░░ 88.4% 12,264 / 13,870
engineering_analytics ██████████████████░░ 89.3% 6,529 / 7,309
conversations ██████████████████░░ 89.5% 17,667 / 19,746
dashboards ██████████████████░░ 89.5% 5,989 / 6,693
alerts ██████████████████░░ 90.3% 4,482 / 4,966
streamlit_apps ██████████████████░░ 90.7% 2,630 / 2,901
mcp_analytics ██████████████████░░ 90.9% 3,191 / 3,511
error_tracking ██████████████████░░ 91.0% 10,928 / 12,010
slack_app ██████████████████░░ 91.1% 9,664 / 10,610
marketing_analytics ██████████████████░░ 91.2% 12,092 / 13,265
stamphog ██████████████████░░ 91.3% 4,505 / 4,936
product_analytics ███████████████████░ 92.5% 5,849 / 6,321
early_access_features ███████████████████░ 92.6% 1,287 / 1,390
ai_observability ███████████████████░ 92.8% 15,618 / 16,821
surveys ███████████████████░ 93.2% 5,853 / 6,281
posthog_ai ███████████████████░ 93.2% 1,326 / 1,422
approvals ███████████████████░ 93.3% 3,437 / 3,682
web_analytics ███████████████████░ 93.4% 15,047 / 16,112
reminders ███████████████████░ 93.4% 468 / 501
workflows ███████████████████░ 94.1% 7,251 / 7,707
endpoints ███████████████████░ 94.2% 8,655 / 9,192
review_hog ███████████████████░ 94.6% 8,101 / 8,563
skills ███████████████████░ 94.6% 3,158 / 3,337
logs ███████████████████░ 95.4% 10,487 / 10,987
experiments ███████████████████░ 95.5% 26,047 / 27,287
annotations ███████████████████░ 96.2% 732 / 761
revenue_analytics ███████████████████░ 96.3% 1,887 / 1,960
feature_flags ███████████████████░ 96.4% 17,488 / 18,144
replay_vision ███████████████████░ 96.4% 16,286 / 16,891
user_interviews ███████████████████░ 96.5% 2,638 / 2,734
customer_analytics ███████████████████░ 97.1% 10,463 / 10,777
warehouse_sources ███████████████████░ 97.3% 359,901 / 369,906
data_catalog ████████████████████ 97.7% 2,588 / 2,648
pulse ████████████████████ 98.4% 2,017 / 2,049

Report-only. Patch coverage = changed backend lines covered vs origin/master. Sorted lowest first.
Known gaps: lines covered only by Temporal tests show as uncovered; core line numbers may drift if master changed the same file.

@Gilbert09 Gilbert09 added the stamphog Request AI approval (no full review) label Jul 31, 2026 — with PostHog

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Contained internal retry-logic fix for delta-lake vacuum maintenance, author is on the owning team, change is well-tested and generalizes an existing helper already used for compact — no risky territory touched.

  • 👍 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 25L, 2F substantive, 48L/3F incl. docs/generated/snapshots — within ceiling
tier T1-agent / T1b-small (48L, 3F, single-area, fix)
stamphog 2.0.0b4 .stamphog/policy.yml @ 34c5044 · reviewed head ccc808c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stamphog Request AI approval (no full review)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant