delivery: price the byte-cut from honest sampled bytes, not entry nbytes - #81
Merged
Conversation
Production entry tables over-report pa.Table.nbytes ~150x. Root cause is dictionary-encoded columns from the parquet read path: every chunk of a sliced dictionary column counts the entire shared dictionary (measured 41KB/row against ~300B honest; multi-chunk entries multiply it per chunk), and combine_chunks() does not correct it — unified dictionaries keep unreferenced entries. On 2026-08-26 the adaptive byte-cut divided by the inflated number and sliced team-2 into ~2,850-row flushes with a frozen growth gate; flush_adaptive has been disabled fleet-wide since. _estimate_row_bytes samples <=1,024 head rows, casts to a recursively dictionary-free schema (struct/list/large_list/fixed_size_list/ list_view/map all rewritten — nested dictionaries inflate identically), and prices per-row from the decoded copy. Everything that sizes work now shares those units: - take-loop admission and the within-entry split (rows x per_row); - the bytes TRIGGER, via a cached per-destination estimate (QE: raw buf.bytes against an honest target fired at ~target/150 of real content — trickle flushes that could never satisfy the fill gate); - the AIMD's evidence, threaded to _flush as est_bytes (the fill gate must judge growth in the units the cut used, or a learned-down target can never grow back). The estimator is total: any pyarrow failure degrades to 0 = unpriceable, byte-cut inert, 60K rows cap backstops — never a poll cycle fatal (the escape handler exits the pod). Sampling falls back past zero-row/unpriceable head entries. Failures log once per (destination, exception type). Buffer gauges and caps deliberately stay on raw nbytes: their prod values are tuned against inflated units; re-rating them is a separate coordinated change with charts. Adversarially reviewed x3 (initial HOLD, verification pass, QE breaker); all required findings fixed, QE reproductions kept in-tree (tests/unit/test_honest_bytes_qe.py). 864 unit tests green. Re-enabling flush_adaptive in charts is the follow-up once this deploys.
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.
Problem
Production entry tables over-report
pa.Table.nbytesby ~150×. Root cause (found during this work — the earlier "zero-copy slices" theory was wrong on current pyarrow): dictionary-encoded columns from the parquet read path. Every chunk of a sliced dictionary column counts the entire shared dictionary — measured 41KB/row against ~300B honest, matching production's implied ~47KB/row — andcombine_chunks()does not correct it (unified dictionaries keep unreferenced entries).On 2026-08-26 the #80 adaptive byte-cut divided by the inflated number and sliced team-2 into ~2,850-row crumb flushes with a frozen AIMD growth gate.
flush_adaptive: falsehas been the fleet-wide stopgap since (codified in charts#14747). The same inflated units are why team-2's 32GiB buffer cap admits only 1-2 entries (the cap-flap sawtooth).Change
_estimate_row_bytes: sample ≤1,024 head rows, cast to a recursively dictionary-free schema (struct / list / large_list / fixed_size_list / list_view / map all rewritten — nested dictionaries inflate identically), price per-row from the decoded copy. Every consumer that sizes work now shares those units:rows × per_row);buf.bytesagainst an honest target fired at ~target/150 of content — trickle flushes that could never satisfy the fill gate, freezing a halved target forever);_flushasest_bytesso the fill gate judges growth in the units the cut used.Totality: any pyarrow failure degrades to 0 = unpriceable → byte-cut inert, 60K rows-cap backstop (the flag-off behavior) — never a poll-cycle fatal. Sampling falls back past zero-row/unpriceable head entries. One warning per (destination, exception type).
Deliberately unchanged: buffer gauges/caps stay on raw nbytes — prod cap values are tuned against inflated units; re-rating is a coordinated follow-up with charts (the 32GiB comment there already says so).
Review trail
Three adversarial passes: initial review (HOLD: estimator totality, nested dictionaries), verification pass (caught the vacuous AIMD test — replaced with a mutation-verified no-spurious-growth discriminator; found fixed_size_list), independent QE breaker (found the bytes-trigger unit mismatch + zero-row-head disarm; reproductions kept in-tree as
tests/unit/test_honest_bytes_qe.py, including quantified sampling-skew design limits). All required findings fixed. 864 unit tests green.Rollout
Deploy is inert:
flush_adaptiveis false in prod charts, and with the flag off the estimator is never called (behavior byte-identical to today's stopgap). The follow-up charts PR flipsdelivery.flushAdaptive: truefor prod-us and watches team-2's flush sizes, target evolution, and the cap-flap panel.