Skip to content

fix(data-imports): reuse resolved delta table in compact/vacuum maintenance - #74541

Merged
gantoine merged 4 commits into
masterfrom
posthog-code/fix-delta-compact-vacuum-refetch
Jul 30, 2026
Merged

fix(data-imports): reuse resolved delta table in compact/vacuum maintenance#74541
gantoine merged 4 commits into
masterfrom
posthog-code/fix-delta-compact-vacuum-refetch

Conversation

@Gilbert09

Copy link
Copy Markdown
Member

Problem

An error tracking issue surfaced Exception: Deltatable not found, raised from DeltaTableHelper.vacuum_table() inside delta_table_helper.py, itself called from compact_table(), itself called from the post-load orchestrator in common/load.py.

Tracing the call path: compact_table() fetches the delta table via get_delta_table(), checks it's not None, successfully runs optimize.compact() on it, and then calls vacuum_table() — which calls get_delta_table() again instead of reusing the table reference it already validated. get_delta_table() caches its result only opportunistically (a small, best-effort cache), so a concurrent sync of a different table running in the same worker process can evict this table's cached entry in the window between the two calls. The redundant re-fetch then comes back None, and vacuum_table() raises "Deltatable not found" immediately after a compact that just succeeded on the very same table.

The same redundant-refetch pattern showed up in two other maintenance methods that already hold a validated table reference: vacuum_if_stale() (calls vacuum_table() instead of vacuuming the table it already fetched) and compact_if_fragmented() (calls get_file_uris() and compact_table(), both of which re-fetch).

Changes

  • Extracted _compact(table) / _vacuum(table) helpers that operate on an already-resolved DeltaTable, with no re-fetch and no None check.
  • compact_table() and vacuum_table() now fetch once and delegate to the helpers.
  • vacuum_if_stale() and compact_if_fragmented() now reuse the table they already fetched at the top of the method, instead of calling back into vacuum_table() / compact_table() / get_file_uris().

This isn't a retry/classification change — it doesn't touch get_delta_table()'s caching behavior or any retryable-error handling. It only removes redundant re-fetches inside call chains that already hold a live, validated table reference.

How did you test this code?

  • Added TestCompactTable::test_does_not_refetch_table_for_the_vacuum_step, asserting compact_table() calls get_delta_table() exactly once — locks in that the vacuum step reuses the resolved table instead of racing a second fetch.
  • Updated TestCompactIfFragmented and TestVacuumIfStale cases to assert against the new _compact/_vacuum internals instead of the now-unused compact_table/vacuum_table/get_file_uris call sites.
  • Ran test_delta_table_helper.py (67 passed; 4 pre-existing failures in TestGetDeltaTableUnrecoverableErrors are environment-only — they need a running object-storage service not available in this sandbox, and fail identically on master).
  • ruff check / ruff format --check clean on both changed files.
  • Full-repo uv run mypy --cache-fine-grained . clean (17065 source files).

Docs update

N/A — internal pipeline implementation detail, no user-facing or API surface change.

Automatic notifications

  • Publish to changelog?
  • Alert Sales and Marketing teams?

🤖 Agent context

Autonomy: Fully autonomous

Triaged from a live PostHog error-tracking issue (webhook-delivered) using Claude Code. Pulled the full stack trace and event properties via the PostHog MCP tools, traced the exact redundant-fetch call chain in delta_table_helper.py, and confirmed via git log/gh pr list that no open PR already covers this call path (a related but distinct open PR, #74378, fixes a different redundant get_delta_table() call in pipeline.py's cleanup path — left untouched). Implemented and tested a scoped fix at the root cause rather than widening error classification or touching retry/backoff policy.

@trunk-io

trunk-io Bot commented Jul 29, 2026

Copy link
Copy Markdown

😎 Merged manually by @gantoine - details.

@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 29, 2026 08:08
@Gilbert09 Gilbert09 added the stamphog Request AI approval (no full review) label Jul 29, 2026 — with PostHog
stamphog[bot]
stamphog Bot previously approved these changes Jul 29, 2026

@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 bugfix in the warehouse-sources pipeline (removes redundant delta-table re-fetches causing spurious "Deltatable not found" errors), by an author on the owning team with solid test coverage; no risky-territory surface touched.

  • Author wrote 35% of the modified lines and has 40 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 37L, 1F substantive, 85L/2F incl. docs/generated/snapshots — within ceiling
tier T1-agent / T1b-small (85L, 2F, single-area, fix)
stamphog 2.0.0b3 .stamphog/policy.yml @ 61c8216 · reviewed head e971b03

@github-actions

github-actions Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

🤖 CI report

Playwright — all passed

All tests passed.

View test results →

⚠️ Backend snapshots — 13 updated (13 modified, 0 added, 0 deleted)

Query snapshots: Backend query snapshots updated

Changes: 13 snapshots (13 modified, 0 added, 0 deleted)

What this means:

  • Query snapshots have been automatically updated to match current output
  • These changes reflect modifications to database queries or schema

Next steps:

  • Review the query changes to ensure they're intentional
  • If unexpected, investigate what caused the query to change

Review snapshot changes →

⚠️ Backend coverage — 87.0% of changed backend lines covered — 5 uncovered

🧪 Backend test coverage

Patch coverage — changed backend lines (products + core): █████████████████░░░ 87.0% (35 / 40)

File Patch Uncovered changed lines
products/warehouse_sources/backend/temporal/data_imports/pipelines/core/delta_table_helper.py 70.6% 944–946, 948, 953

🤖 Agents: add a test covering the lines above, or note why under "How did you test this code?". Machine-readable gap list: the patch-coverage artifact on this run (gh run download 30554192908 -n patch-coverage), or the coverage-data block at the end of this comment.

Per-product line coverage (touched products)
Product Coverage Lines
demo ███████████░░░░░░░░░ 56.2% 1,497 / 2,663
tasks ██████████████░░░░░░ 70.0% 32,556 / 46,509
signals ████████████████░░░░ 81.1% 24,208 / 29,836
cdp ████████████████░░░░ 82.1% 3,285 / 3,999
data_modeling █████████████████░░░ 85.3% 7,359 / 8,626
notebooks █████████████████░░░ 86.0% 7,794 / 9,060
actions █████████████████░░░ 86.6% 717 / 828
cohorts ██████████████████░░ 87.8% 6,181 / 7,040
product_tours ██████████████████░░ 87.9% 1,303 / 1,482
exports ██████████████████░░ 88.2% 7,046 / 7,986
data_warehouse ██████████████████░░ 88.3% 12,036 / 13,635
conversations ██████████████████░░ 89.3% 17,545 / 19,644
engineering_analytics ██████████████████░░ 89.4% 6,441 / 7,202
dashboards ██████████████████░░ 89.5% 5,989 / 6,693
alerts ██████████████████░░ 90.2% 4,458 / 4,942
mcp_analytics ██████████████████░░ 90.2% 2,910 / 3,225
streamlit_apps ██████████████████░░ 90.7% 2,630 / 2,901
error_tracking ██████████████████░░ 91.0% 10,925 / 12,004
slack_app ██████████████████░░ 91.1% 9,554 / 10,492
stamphog ██████████████████░░ 91.1% 4,056 / 4,450
marketing_analytics ██████████████████░░ 91.2% 12,092 / 13,265
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.1% 5,771 / 6,197
web_analytics ███████████████████░ 93.2% 14,826 / 15,906
posthog_ai ███████████████████░ 93.2% 1,326 / 1,422
approvals ███████████████████░ 93.3% 3,437 / 3,682
reminders ███████████████████░ 93.4% 468 / 501
workflows ███████████████████░ 93.8% 6,943 / 7,399
endpoints ███████████████████░ 94.2% 8,655 / 9,192
skills ███████████████████░ 94.6% 3,158 / 3,337
review_hog ███████████████████░ 94.7% 7,015 / 7,407
experiments ███████████████████░ 95.4% 25,835 / 27,067
logs ███████████████████░ 95.5% 10,435 / 10,928
annotations ███████████████████░ 96.2% 732 / 761
revenue_analytics ███████████████████░ 96.3% 1,887 / 1,960
feature_flags ███████████████████░ 96.4% 17,385 / 18,037
replay_vision ███████████████████░ 96.4% 15,957 / 16,550
user_interviews ███████████████████░ 96.5% 2,638 / 2,734
customer_analytics ███████████████████░ 97.0% 10,218 / 10,529
warehouse_sources ███████████████████░ 97.3% 355,757 / 365,801
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 force-pushed the posthog-code/fix-delta-compact-vacuum-refetch branch from e971b03 to 205a164 Compare July 29, 2026 11:05
@stamphog
stamphog Bot dismissed their stale review July 29, 2026 11:06

New commits pushed (delta classified non_linear_history) — stamphog approval dismissed; re-review running automatically.

@stamphog

stamphog Bot commented Jul 29, 2026

Copy link
Copy Markdown

Note

🤖 stamphog reviewed 205a164ca42191b54f060cd57ac082e2f6158560 — verdict: WAIT

@hex-security-app[bot] still has a review in flight (👀) after 5 minutes — not approving over an unfinished review. The stamphog label has been kept; the review re-runs on the next push, or remove and re-apply the label once the reviewer finishes.

  • 👍 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 37L, 1F substantive, 85L/2F incl. docs/generated/snapshots — within ceiling
tier T1-agent / T1b-small (85L, 2F, single-area, fix)
stamphog 2.0.0b3 .stamphog/policy.yml @ 552f985 · reviewed head 205a164

stamphog[bot]
stamphog Bot previously approved these changes Jul 29, 2026

@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.

Small, well-tested bug fix that removes redundant re-fetches in delta table compact/vacuum maintenance (root-caused a real "Deltatable not found" race); no data model, API, auth, or CI surface touched, and the author is on the owning team.

  • 👍 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 37L, 1F substantive, 85L/2F incl. docs/generated/snapshots — within ceiling
tier T1-agent / T1b-small (85L, 2F, single-area, fix)
stamphog 2.0.0b3 .stamphog/policy.yml @ c244b10 · reviewed head 3345186

…enance

Compaction and vacuum used to re-fetch the delta table via `get_delta_table()` after already resolving it moments earlier in the same call, even though `get_delta_table()`'s cache is only opportunistic. A concurrent sync of a different table can evict this table's cache entry mid-flight, so the redundant re-fetch could come back `None` and raise "Deltatable not found" right after a successful compact. This reuses the table reference already in hand across `compact_table`, `vacuum_if_stale`, and `compact_if_fragmented` instead of re-deriving it.

Generated-By: PostHog Code
Task-Id: c87d168c-c8b4-4ce2-9b72-d24d57864a60
@Gilbert09
Gilbert09 force-pushed the posthog-code/fix-delta-compact-vacuum-refetch branch from 3345186 to f330cef Compare July 30, 2026 10:28
@stamphog
stamphog Bot dismissed their stale review July 30, 2026 10:33

New commits pushed (delta classified non_linear_history) — stamphog approval dismissed; re-review running automatically.

@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.

Small, well-tested internal refactor of Delta table compact/vacuum maintenance to reuse an already-resolved table instead of re-fetching it — fixes a real race but stays within internal data-import pipeline code, no schema/API/auth/dependency surface touched.

  • Author wrote 38% of the modified lines and has 8 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 39L, 1F substantive, 87L/2F incl. docs/generated/snapshots — within ceiling
tier T1-agent / T1b-small (87L, 2F, single-area, fix)
stamphog 2.0.0b3 .stamphog/policy.yml @ 0e33e42 · reviewed head f330cef

@tests-posthog
tests-posthog Bot requested a review from a team as a code owner July 30, 2026 10:47
@stamphog

stamphog Bot commented Jul 30, 2026

Copy link
Copy Markdown

Retaining stamphog approval — delta since last review classified as trivial_paths.

@talyn-app

talyn-app Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

/trunk merge

Generated-By: PostHog Code
Task-Id: ebf836c6-bf3a-4466-9cdb-c53802eb4e20
@talyn-app
talyn-app Bot enabled auto-merge (squash) July 30, 2026 14:56
@gantoine
gantoine disabled auto-merge July 30, 2026 16:48
@gantoine
gantoine merged commit 755609d into master Jul 30, 2026
233 checks passed
@gantoine
gantoine deleted the posthog-code/fix-delta-compact-vacuum-refetch branch July 30, 2026 16:48
@deployment-status-posthog

deployment-status-posthog Bot commented Jul 30, 2026

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-07-30 17:47 UTC Run
prod-us ✅ Deployed 2026-07-30 18:06 UTC Run
prod-eu ✅ Deployed 2026-07-30 18:05 UTC Run

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

Labels

no-snapshot-update stamphog Request AI approval (no full review)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants