feat(warehouse): deltalite real-write path (phase 2, flag-gated) - #75884
Merged
Conversation
Adds an optional deltalite write path for the incremental merge: when the
DATA_WAREHOUSE_DELTALITE_WRITE_ENABLED env switch is on and the per-schema
data-warehouse-deltalite-write flag matches, deltalite performs the real merge
(DeltaLiteTable.upsert) instead of the delta-rs MERGE.
Safety by construction: any failure — env/flag off, import error, deltalite
error/conflict/refusal — falls back to the delta-rs MERGE, so switching a schema
to deltalite can only change which engine writes, never whether the sync
succeeds. Off by default; nothing changes until the env + flag are set.
- settings: DATA_WAREHOUSE_DELTALITE_WRITE_ENABLED (master switch, default False)
- deltalite_shadow: extract _is_deltalite_flag_enabled; add is_deltalite_write_enabled
- metric: warehouse_load_deltalite_write_total{outcome=written|fallback}
- the forward shadow is skipped when deltalite did the real write (redundant)
Follows the phase-1 shadow, which validated deltalite is byte-for-byte
equivalent to MERGE across the fleet on real prod S3.
|
😎 This pull request was merged. |
👀 Auto-assigned reviewersThese soft owners were skipped because they only have minor changes here. Nothing blocks merge, so self-assign if you'd like a look:
Soft owners come from each directory's |
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
Contributor
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
Contributor
🤖 CI report
|
| File | Patch | Uncovered changed lines |
|---|---|---|
products/warehouse_sources/backend/temporal/data_imports/pipelines/core/delta_table_helper.py |
94.7% | 613–614 |
🤖 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 30625569483 -n patch-coverage), or the coverage-data block at the end of this comment.
Per-product line coverage (touched products)
| Product | Coverage | Lines |
|---|---|---|
platform_features |
██░░░░░░░░░░░░░░░░░░ 12.1% |
7 / 58 |
batch_exports |
████████░░░░░░░░░░░░ 39.4% |
8,782 / 22,290 |
demo |
███████████░░░░░░░░░ 56.3% |
1,497 / 2,661 |
warehouse_sources_queue |
████████████░░░░░░░░ 59.2% |
148 / 250 |
data_tools |
██████████████░░░░░░ 70.0% |
63 / 90 |
tasks |
██████████████░░░░░░ 70.1% |
32,784 / 46,772 |
ai_gateway |
███████████████░░░░░ 75.0% |
9 / 12 |
signals |
████████████████░░░░ 81.2% |
24,750 / 30,477 |
cdp |
████████████████░░░░ 82.1% |
3,285 / 3,999 |
data_modeling |
█████████████████░░░ 85.6% |
7,688 / 8,979 |
notebooks |
█████████████████░░░ 86.0% |
7,794 / 9,060 |
wizard |
█████████████████░░░ 86.4% |
1,060 / 1,227 |
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.4% |
12,192 / 13,798 |
business_knowledge |
██████████████████░░ 89.0% |
4,391 / 4,936 |
engineering_analytics |
██████████████████░░ 89.3% |
6,529 / 7,309 |
conversations |
██████████████████░░ 89.3% |
17,600 / 19,701 |
dashboards |
██████████████████░░ 89.4% |
5,983 / 6,693 |
visual_review |
██████████████████░░ 89.5% |
5,870 / 6,558 |
alerts |
██████████████████░░ 90.2% |
4,458 / 4,942 |
links |
██████████████████░░ 90.6% |
183 / 202 |
streamlit_apps |
██████████████████░░ 90.7% |
2,630 / 2,901 |
mcp_analytics |
██████████████████░░ 90.9% |
3,191 / 3,511 |
error_tracking |
██████████████████░░ 91.0% |
10,926 / 12,008 |
slack_app |
██████████████████░░ 91.1% |
9,664 / 10,610 |
marketing_analytics |
██████████████████░░ 91.2% |
12,092 / 13,265 |
stamphog |
██████████████████░░ 91.3% |
4,505 / 4,936 |
mcp_store |
██████████████████░░ 91.9% |
4,257 / 4,634 |
product_analytics |
███████████████████░ 92.5% |
5,849 / 6,321 |
managed_migrations |
███████████████████░ 92.6% |
1,556 / 1,681 |
early_access_features |
███████████████████░ 92.6% |
1,287 / 1,390 |
notifications |
███████████████████░ 92.6% |
1,017 / 1,098 |
ai_observability |
███████████████████░ 92.8% |
15,618 / 16,821 |
surveys |
███████████████████░ 93.1% |
5,771 / 6,197 |
posthog_ai |
███████████████████░ 93.2% |
1,326 / 1,422 |
web_analytics |
███████████████████░ 93.3% |
14,911 / 15,976 |
approvals |
███████████████████░ 93.3% |
3,437 / 3,682 |
reminders |
███████████████████░ 93.4% |
468 / 501 |
legal_documents |
███████████████████░ 93.8% |
1,628 / 1,736 |
workflows |
███████████████████░ 94.1% |
7,241 / 7,698 |
endpoints |
███████████████████░ 94.2% |
8,655 / 9,192 |
tracing |
███████████████████░ 94.5% |
2,671 / 2,827 |
review_hog |
███████████████████░ 94.6% |
8,101 / 8,563 |
skills |
███████████████████░ 94.6% |
3,158 / 3,337 |
messaging |
███████████████████░ 94.7% |
2,885 / 3,048 |
experiments |
███████████████████░ 95.4% |
25,938 / 27,177 |
logs |
███████████████████░ 95.5% |
10,440 / 10,937 |
growth |
███████████████████░ 96.1% |
3,245 / 3,376 |
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,081 / 16,679 |
user_interviews |
███████████████████░ 96.5% |
2,638 / 2,734 |
access_control |
███████████████████░ 96.9% |
870 / 898 |
customer_analytics |
███████████████████░ 97.1% |
10,444 / 10,758 |
warehouse_sources |
███████████████████░ 97.3% |
359,265 / 369,255 |
data_catalog |
████████████████████ 97.7% |
2,588 / 2,648 |
analytics_platform |
████████████████████ 98.0% |
2,153 / 2,197 |
metrics |
████████████████████ 98.2% |
2,491 / 2,536 |
pulse |
████████████████████ 98.4% |
2,017 / 2,049 |
live_debugger |
████████████████████ 99.2% |
613 / 618 |
field_notes |
████████████████████ 99.4% |
158 / 159 |
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.
Phase 2 is now gated solely by the data-warehouse-deltalite-write flag — no DATA_WAREHOUSE_DELTALITE_WRITE_ENABLED env var — so it can be ramped or killed from the flag UI without a deploy. The per-merge flag eval is a couple of indexed lookups; negligible fleet-wide.
The shadow verification did its job: it proved deltalite is byte-for-byte equivalent to the delta-rs MERGE across the fleet on real prod S3, and surfaced the null-composite-key duplicate bug in MERGE (fixed in #75601). With phase 2 (deltalite doing the real write) in this PR, running the shadow too is redundant. Removes deltalite_shadow.py (compare engine, seed/upsert/cleanup, sampling + caps), the DATA_WAREHOUSE_DELTALITE_SHADOW_* settings, the shadow metrics, and the write_to_deltalake shadow hook. The per-schema flag helper the write path needs is kept in a focused new module, deltalite_write.py.
… tests The write-path fallback warning + metric were inside the same try/except as the flag eval, so a flag miss or a flags-service blip logged 'deltalite write failed' — which broke test_incremental_merge_dedupes_duplicate_source_rows (it saw a second awarning). Split the flag gate out of the write attempt: a flag off / eval error now falls back to the delta-rs MERGE silently, and the warning + 'fallback' metric are reserved for a genuine deltalite write failure. Also add an autouse fixture so the write-path tests' patch.dict(sys.modules) doesn't evict the lazily-imported metrics module (which re-registered its Prometheus metrics on the next test).
estefaniarabadan
approved these changes
Jul 31, 2026
Addresses review: (1) move the lazy imports inside their try/except so an import failure falls back to the MERGE as the docstring promises, and (2) scope the fallback to the upsert *commit* only. A failure after deltalite commits (e.g. update_incremental) no longer returns False — that would re-run the delta-rs MERGE on top of deltalite's already-committed write. Post-commit bookkeeping is now best-effort. Adds a test for the post-commit path.
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
Addresses review: after the upsert commits, the log + metric increment were still outside a try, so if either raised the exception would propagate, deltalite_wrote would stay unset, and the sync would fail/retry (or re-run the MERGE) on top of an already-committed write. Wrap the whole post-commit block (handle refresh, log, metric) best-effort and always return True. Parametrise the post-commit test over refresh- and log-failure.
helper._logger is typed FilteringBoundLogger, so mypy rejected ._logger.ainfo.side_effect. Set the side effect on the mock logger before constructing the helper (same MagicMock-logger pattern the other tests use).
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
2 participants
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
Phase 1 (the deltalite shadow) ran across the whole warehouse fleet on real prod S3 and proved deltalite is byte-for-byte equivalent to the delta-rs
MERGE— the only divergences were deltalite being more correct (collapsing the null-composite-key duplicates that MERGE accumulates, since fixed by #75601). deltalite's real upsert + conditional-put commit path is therefore already well-exercised.This PR does two things: (1) lets deltalite perform the real incremental merge behind a flag (phase 2 canary), and (2) removes the now-redundant phase-1 shadow.
Changes
Phase 2 — real writes. When the per-schema
data-warehouse-deltalite-writeflag matches,write_to_deltalakeroutes the incremental merge todeltalite.DeltaLiteTable.upsertinstead of the delta-rsMERGE.data-warehouse-deltalite-writeflag, so it can be ramped or killed from the flag UI without a deploy. (Per-merge flag eval is a couple of indexed lookups; negligible fleet-wide.)metrics:warehouse_load_deltalite_write_total{outcome=written|fallback}.Important
Fallback by construction.
_write_via_deltalitereturnsFalse— and the delta-rsMERGEruns as today — on anything: flag off, import failure, deltalite error, commit conflict, unsupported table. Enabling a schema can only change which engine writes, never whether the sync succeeds. Nothing changes until the flag is on.Removes the phase-1 shadow. It did its job (proved equivalence + found the MERGE null-key bug), and running it alongside phase 2 is redundant. Deletes
deltalite_shadow.py(compare engine, seed/upsert/cleanup, sampling + caps), theDATA_WAREHOUSE_DELTALITE_SHADOW_*settings, the shadow metrics, and thewrite_to_deltalakeshadow hook. The per-schema flag helper the write path needs is kept in a focused new module,deltalite_write.py. Net −712 lines.Rollout (after merge — no deploy needed to enable)
data-warehouse-deltalite-writeon for team 2 only (dogfood), excluding Sentry.warehouse_load_deltalite_write_total{outcome}+ thedeltalite write:logs, confirm team-2 table contents, ramp.Note
On an already-bloated table, deltalite's first write collapses the accumulated duplicates — correct, but the row count drops (customer-visible). Team-2 dogfood first is deliberate.
Follow-ups (separate, out of scope here): remove the now-dead
DATA_WAREHOUSE_DELTALITE_SHADOW_ENABLEDenv var from charts, and archive thedata-warehouse-deltalite-shadowflag.How did you test this code?
I'm an agent.
12 passedlocally —TestDeltaliteWritePath(mocked deltalite: skips w/o PKs, falls back when flag off, routes toupsertwhen enabled, falls back when deltalite raises), the newtest_deltalite_write.pyflag tests (fail-closed, write-flag key + person properties, source_type resolution), andTestNullSafeMergePredicate. ruff clean; the two suites' only other failures are the pre-existing environmentalobjectstorageones.Rollback
Set the
data-warehouse-deltalite-writeflag off — merges return to the delta-rsMERGE. No deploy required.