fix(data-warehouse): survive pyarrow string_view in v3 cdc delete enrichment - #71003
Conversation
|
Reviews (1): Last reviewed commit: "fix(data-warehouse): survive pyarrow str..." | Re-trigger Greptile |
9e5704d to
dcd2974
Compare
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 |
dcd2974 to
d6e7c57
Compare
|
Thanks for the context. I agree the version-bump failure itself is pre-existing behavior being repaired, but the unfiltered |
…ichment pyarrow 21+ (shipped via #70476) materializes string columns as string_view, whose equal/greater_equal compute kernels are unimplemented. The v3 CDC delete-enrichment step in processor.py pushed an `in` predicate onto the existing delta table filtered by a string primary key, which raised ArrowNotImplementedError and failed every CDC batch carrying deletes on a string key. V2 was unaffected (its load path does no such filtered pyarrow read). Prefer delta filter pushdown as before; on ArrowNotImplementedError fall back to reading the table and filtering in pyarrow after casting the key to string, which has working kernels. Numeric/timestamp keys keep the fast pushdown path. Generated-By: PostHog Code Task-Id: 999e969c-5031-4b5f-a5b2-96f37dd4eb8a
d6e7c57 to
4053562
Compare
| return existing_delta_table.to_pyarrow_table(filters=[(first_pk, "in", first_components)]) | ||
| except pa.lib.ArrowNotImplementedError: | ||
| logger.warning("cdc_delete_enrichment_pushdown_fallback", primary_key=first_pk) | ||
| existing = existing_delta_table.to_pyarrow_table() |
There was a problem hiding this comment.
When a string primary key triggers the expected Arrow exception, this loads every row and column of the tenant's Delta table before applying the DELETE key filter. A tenant can grow a wide CDC table and submit a small DELETE batch to force this allocation; up to 16 schema groups run concurrently per loader, so the resulting OOM can restart the shared worker and interrupt other imports.
Prompt To Fix With AI
Keep the string_view fallback memory-bounded. Scan the Delta dataset in record batches (projecting only columns required by delete enrichment where possible), cast the primary-key column to string per batch, filter each batch against the requested keys, and concatenate only matching batches. Do not materialize the unfiltered table with `to_pyarrow_table()`.Severity: medium | Confidence: 96% | React with 👍 if useful or 👎 if not
🤖 CI report
|
| File | Patch | Uncovered changed lines |
|---|---|---|
products/warehouse_sources/backend/temporal/data_imports/pipelines/pipeline_v3/load/processor.py |
90.0% | 538 |
🤖 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 29408577235 -n patch-coverage), or the coverage-data block at the end of this comment.
Per-product line coverage (touched products)
| Product | Coverage | Lines |
|---|---|---|
warehouse_sources |
███████████████████░ 96.1% |
217,548 / 226,264 |
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.
Problem
#70476 bumped
deltalaketo 1.6.1 andpyarrowto 23.0.1. pyarrow 21+ materializes string columns as Arrowstring_view, and theequal/greater_equalcompute kernels have no(string_view, string_view)implementation.The v3 CDC delete-enrichment step reads the existing delta table filtered by a primary key to backfill columns on standalone DELETE rows. It does that with a pushed-down
inpredicate:posthog/products/warehouse_sources/backend/temporal/data_imports/pipelines/pipeline_v3/load/processor.py
Lines 503 to 505 in 7a4d1d1
When the primary key is a string, that pushdown now runs the missing kernel and raises
ArrowNotImplementedError. It is deterministic, so all three attempts fail and the whole run fails. Since the deploy this has been firing continuously on the v3 CDC path (workflow_type=cdc-extraction), while v2 is unaffected because its load path does no equivalent filtered pyarrow read.Stack trace on every failure:
Changes
_read_existing_rows_by_first_pkwraps the read. It keeps the delta filter pushdown as the fast path (file pruning, cheap, still correct for numeric and timestamp keys). OnArrowNotImplementedErrorit falls back to reading the table and filtering in pyarrow after casting the key column tostring, which has working kernels. The composite-PK narrowing and SCD2 filtering downstream are unchanged.Note
The fallback reads the existing table without file pruning, so it is heavier than the pushdown. It only runs for CDC batches that carry deletes on a string key, and only while pyarrow lacks the string_view kernels. Numeric and timestamp keys keep the fast path. If a later pyarrow adds the kernel, the fast path just starts serving string keys again and the fallback stops triggering.
This is a forward-fix that keeps the engine upgrade rather than reverting #70476.
How did you test this code?
Reproduced the exact failure locally against the now-pinned
pyarrow==23.0.1/deltalake==1.6.1: a delta table written withstring_viewcolumns makesto_pyarrow_table(filters=[(pk, "in", vals)])raise the samegreater_equal (string_view, string_view)error seen in production. Confirmed the fallback returns the correct rows.Added
TestReadExistingRowsByFirstPkintest_processor.py. It writes a real local-filesystem delta table withstring_viewcolumns across two files (so file pruning is exercised like production) and asserts the helper returns exactly the rows whose PK is in the requested set, across subset / single / superset-with-miss / no-match cases. This is the regression no existing test caught: the existingtest_processor.pycovers partitioning and write-type mapping, andtest_delta_table_helper.pycovers merge behavior, neither the string_view filtered read. On the current pinned pyarrow the subset/single/superset cases exercise the fallback path (the fix); the test stays correct if a future pyarrow makes the fast path serve string keys.I (Claude, agent-assisted) could not run the S3/Postgres-backed e2e suite locally (no Docker in this environment). Automated tests I actually ran:
test_processor.py::TestReadExistingRowsByFirstPk— 4 passedtest_processor.py— 17 passed, no regressionsruff check+ruff format— cleanhogli ci:preflight— 0 failuresAutomatic notifications
Docs update
No user-facing docs affected.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Produced with PostHog Code (Claude). Follow-up to a production investigation that traced a v3 CDC failure surge to #70476: the error appeared the moment the new loader image rolled, was zero for the ~41h before, and is exclusively on the CDC delete-enrichment
to_pyarrow_tableread. I verified no other PR deployed that day touched the failing path and that #70476's own code diff was cosmetic, so the library bump is the cause.Skills invoked:
investigating-warehouse-sources-load(production RCA across Grafana logs/metrics and the federated queue Postgres) and/writing-tests(test value gate).Decisions: I tried three narrower fixes first and rejected them empirically against a reproduced string_view table — casting inside the filter expression, forcing the dataset schema to
string, andas_large_types=Trueall still hit the same kernel error because statistics-based file pruning compares string_view before the cast applies. Only reading without a value predicate and filtering in pyarrow avoids the missing kernel. Chose a try-fast-then-fallback shape so numeric/timestamp keys keep pruning and the workaround self-disables if pyarrow later fills the gap. A full revert of #70476 was the alternative; this keeps the engine upgrade.Created with PostHog Code