VARIANT: shred allowlist via prefix/keys, keep the full document - #120
Open
fuziontech wants to merge 11 commits into
Open
VARIANT: shred allowlist via prefix/keys, keep the full document#120fuziontech wants to merge 11 commits into
fuziontech wants to merge 11 commits into
Conversation
2026-08-12 incident: enabling dual-write on prod-us events-nrt
crash-looped all 16 consumers. One tenant's payload carried a JSON
integer above INT64_MAX; try_cast(... AS VARIANT) accepts it (it is a
valid UINT64 variant), but DuckDB's shredded Parquet write then
overflows converting UINT64 -> INT64 and fails the whole INSERT. Offsets
never advanced, so the same batch failed forever.
Retry the failed flush once with the VARIANT projection dropped: the
records land string-only with a NULL companion, matching how
ensure_variant_columns already degrades a wrong-typed companion. If the
retry also fails the error propagates — the projection wasn't the cause.
Values are never rewritten; regex-normalizing big ints in the JSON text
corrupts any string containing a long digit run.
Adds variant_write_fallback_total + errors_total{type="variant_write"}.
Tests use a new ducklake_conn fixture — a REAL local DuckLake catalog
writing Parquet. The existing in-memory `conn` fixture cannot reproduce
this class of bug at all (no Parquet, no shredding), which is exactly
why the pre-incident test suite passed. Shredding needs a few hundred
rows to trip data inlining.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review found the batch-level retry was the wrong layer. It absorbed retryable failures (commit contention permanently NULLed companions and blinded the contention alert), dropped every source's companion rather than the offender's, skipped the reset_caches() the outer retry drives, leaked an abandoned Parquet file per flush, and could not help at all when a small write is INLINED into the catalog — verified: that path succeeds, then detonates later in ducklake_flush_inlined_data, where no retry can reach it. Prevent instead: _variant_projection nulls the companion per row for JSON carrying a 19+ digit integer in value position. Anchoring on the delimiters keeps long digit runs inside string values (session ids) from false-positiving — verified against a real catalog. Only offending rows lose VARIANT; neighbours, other sources, and every string column are untouched, and nothing unshreddable ever reaches the column, inlined or not. The retry survives as a narrow backstop for value shapes the pattern misses: only the out-of-range conversion signature is absorbed, counters increment after the string-only write actually succeeds (they inflated 3x per flush during outages), and the log says a nonzero counter means a guard bug, not routine degradation. Tests: promote the local-DuckLake fixture to tests/integration/conftest.py with its pytest.skip guard (the copy here would hard-error CI offline), add an inlining-enabled variant, and cover per-row precision, the inlined path, second-source isolation, contention NOT being absorbed, and the backstop still firing. The old re-raise test failed both INSERTs so it passed against unfixed main; it now fails only the projecting INSERT. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…JSON Re-review of the reworked guard found it would crash-loop prod again, for a new reason, and was wrong in both directions on top of that. Verified each claim against a real DuckLake catalog: - regexp_matches() only binds VARCHAR. arrow_converter types each key from its first non-null sample, so an all-numeric batch for the configured source yields int64 — BinderException, backstop doesn't match it, partition wedges. main handled this fine. - The pattern required a leading :/,/[ delimiter, so a bare top-level number never matched. Under default data inlining that commits poison into catalog state and breaks ducklake_flush_inlined_data forever, where no write-time retry can reach it. - "19+ digits" is not the hazard. Measured: only (INT64_MAX, UINT64_MAX] fails to shred. Nanosecond timestamps and snowflake ids — 19 digits, ubiquitous — shred fine but were having their whole row's companion nulled, silently, with no metric. Replace the SQL regex with a two-stage Arrow pass: a vectorized prefilter shaped to the window's decimal form (19 digits leading 9, or 20 leading 1 — so ns timestamps don't trip it), then orjson round-trip on just the flagged rows, rewriting only ints in the dangerous window as strings. Precision comes from the parser, never from a pattern. Result lands in a hidden per-source column read only inside the VARIANT cast, so the source column still writes byte for byte, and the SQL projection goes back to main's simple form — no type fragility. Degradation is now "one value is typed as a string" rather than "the row loses its whole companion", and it is visible: variant_values_coerced_total. ~10ms per 8k-row flush when nothing matches, ~22ms when something does. Tests pin the boundary in both directions (ns timestamps, snowflakes, INT64_MAX, >UINT64_MAX, negatives all keep numeric typing), cover the bare-number/inlined path and non-string sources, and assert the fallback counter fires. The old tests encoded a false premise — that a 30-digit INT128 value explodes — which would have blocked any correct narrowing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Dual-write still writes every JSON key into the VARIANT companion. MILLPOND_VARIANT_KEY_PREFIX (default $) and MILLPOND_VARIANT_KEYS only limit which object keys the parquet writer shreds, via SET on the PostHog duckdb fork. Stock 1.5.2 rejects the settings and we warn — that is the 2026-08 OOM path. Empty prefix + empty keys is the explicit opt-out and is logged as such.
pyproject/uv.lock stay on the official 1.5.5 wheel so CI and local 3.13 can still resolve from PyPI. The Dockerfile overwrites that wheel (and the CLI) with PostHog/duckdb v1.5.5-posthog.2 so variant_shred_key_prefix exists, and asserts source_id=2a514c18f7. DuckLake canary moves 415a9ebd → d8a1881e (the 1.5.5 channel). Two integration tests follow 1.5.5 catalog/VARIANT typing.
Renaming to duckdb.whl strips the PEP 427 version; uv 0.12 rejects it. Keep curl only for the CLI zip.
Reject illegal characters with a RuntimeError that names the env var. Do not log a success-shaped INFO after a rejected SET on stock 1.5.5.
Keep #119's pyducklake lockstep pin and the 1.5.5-strict VARIANT DOUBLE assertions. Keep the PostHog fork overwrite, and --reinstall the 1.5.5-posthog.2 wheel so uv does not leave the official source_id (d8cdaa33fd) in the image. Runtime stage now asserts both the fork source_id and ducklake SHA d8a1881e.
uv 0.12 keys the wheel cache by version, so --reinstall of the 1.5.5 fork URL can silently put the official PyPI bits back. millpond's image pin is uv 0.7 (which did download), but keep the flag so a future uv bump cannot regress the source_id assert.
Same uv 1.5.5-version collision as viaduck. Unpack the release wheel into site-packages and assert source_id 697fa6fb44. CLI comes from the matching v1.5.5-posthog.5 zip.
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.
Summary
Dual-write still writes every JSON key into the VARIANT companion.
MILLPOND_VARIANT_KEY_PREFIX(default$) andMILLPOND_VARIANT_KEYSonly limit which object keys the parquet writer shreds, viaSETon the PostHog duckdb fork.Stock 1.5.2 rejects the settings; we warn and continue (that is the 2026-08 OOM path). Empty prefix + empty keys is the explicit opt-out.
Depends on PostHog/duckdb#6. Wheel/Dockerfile pin to the fork is a follow-up once
v1.5.5-posthog.2artifacts exist.Test plan
just lint/just fmt-check/just test/just test-integrationjust test-e2e(running)