fix(parquet): keep DELTA_BYTE_ARRAY dedup for values larger than the page size limit - #10505
Open
adriangb wants to merge 3 commits into
Open
fix(parquet): keep DELTA_BYTE_ARRAY dedup for values larger than the page size limit#10505adriangb wants to merge 3 commits into
adriangb wants to merge 3 commits into
Conversation
adriangb
marked this pull request as ready for review
August 1, 2026 16:49
Contributor
Author
|
run benchmark arrow_writer |
Contributor
Author
|
run benchmark writer_overhead |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix-delta-byte-array-page-split-10489 (d053bf0) to cd17899 (merge-base) diff Run configurationrun benchmark arrow_writerBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writer File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix-delta-byte-array-page-split-10489 (d053bf0) to cd17899 (merge-base) diff Run configurationrun benchmark writer_overheadBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench writer_overhead File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing fix-delta-byte-array-page-split-10489 (d053bf0) to cd17899 (merge-base) diff Run configurationrun benchmark writer_overheadCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing fix-delta-byte-array-page-split-10489 (d053bf0) to cd17899 (merge-base) diff Run configurationrun benchmark arrow_writerCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
adriangb
force-pushed
the
fix-delta-byte-array-page-split-10489
branch
from
August 3, 2026 13:58
d63a6d7 to
5b904cc
Compare
alamb
pushed a commit
that referenced
this pull request
Aug 3, 2026
…er benchmark (#10512) # Which issue does this PR close? - Part of #10511 # Rationale for this change No existing benchmark writes `DELTA_BYTE_ARRAY` through the writer: the `arrow_writer` property matrix varies writer version, compression, bloom filters, and CDC, all on the default encoding, and `encoding.rs` benches encoders in isolation. That gap matters for the large-value write path specifically: the byte-budget sub-batching in `write_batch_internal` (#9972) measures raw payload bytes, and page-boundary behavior interacts with the encoding's cross-value state (#10489, #10505). None of that is visible to any current benchmark. #10511 tracks whether making the byte budget encoded-size-aware is worthwhile; these benchmarks are the measurement for that question. # What changes are included in this PR? Two new batches and one new bench group in `parquet/benches/arrow_writer.rs`: - `large_string_shared_prefix`: 128 rows x 2 MiB, long common prefix with a short distinct suffix (the case `DELTA_BYTE_ARRAY` exists for) - `large_string_distinct`: same shape, values differing from byte 0 (the adversarial case, prefix length ~0) Values are sized so one value alone exceeds the default 1 MiB page limit, the regime of #10489. Each batch runs under `plain` and `delta_byte_array` properties (dictionary disabled), so the delta-vs-plain gap on identical data separates inherent encoding cost from writer overhead. Results on an Apple M-series laptop, current `main`: | group | `plain` | `delta_byte_array` | | --- | --- | --- | | `large_string_shared_prefix` | 59.0 ms (4.2 GiB/s) | 61.4 ms (4.1 GiB/s) | | `large_string_distinct` | 37.2 ms (6.7 GiB/s) | 44.7 ms (5.6 GiB/s) | Note these numbers shift when #10505 lands, in both directions: shared-prefix delta gets slower in CPU time because the encoder starts doing real prefix comparisons instead of degenerating to per-page `PLAIN` (while the output shrinks ~128x), and distinct delta gets faster from halving the page count. That sensitivity is the point of having the benchmark. # Are these changes tested? The change is itself a benchmark; it compiles and runs under `cargo bench -p parquet --bench arrow_writer`. # Are there any user-facing changes? No. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…page limit A `BYTE_ARRAY` value larger than `data_page_size_limit` exceeds the limit on its own, so the post-write `should_add_data_page` check cuts a page after every single value. Parquet requires at least one value per data page, so that value is unsplittable and the limit is simply unsatisfiable there. For `DELTA_BYTE_ARRAY` this is destructive rather than merely wasteful: each page boundary discards the encoder's previous value, so every value gets `prefix_length = 0` and the encoding degenerates to exactly `PLAIN`. Columns of large values that share long prefixes stop being deduplicated (apache#10489). Exempt a page's mandatory first value from the byte limit when that value alone already exceeds it, so the limit applies to what follows — the bytes we can still place on another page. The exemption is gated on `ColumnValueEncoder::compresses_against_previous_value`, so only `DELTA_BYTE_ARRAY` opts in; `PLAIN` and `DELTA_LENGTH_BYTE_ARRAY` cost the same wherever a value lands and keep their existing one-value page bound. Pages therefore stay bounded by the value size rather than growing with `write_batch_size`, preserving the fix from apache#9972. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The first-value exemption triggers on a page-opening mini-batch holding exactly one value. A null in a chunk changes the level:value ratio, the byte-budget chunker rounds up to two-level mini-batches, and pages that open with a two-value mini-batch miss the exemption: they are cut after two values with the first stored in full. The mini-batch pairing the null with a value has one value, so the page it opens does get the exemption and accumulates the remaining suffixes. Pin that layout ([2, 2, 2, 2, 9] for 16 identical values with a null at index 8) so the limitation is a documented decision rather than an accident, and note it on set_page_size_floor. If the trigger is later keyed on values written to the page (0 -> 1) instead of mini-batch shape, the test fails with fewer, larger pages and should be updated to pin the improved layout. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
adriangb
force-pushed
the
fix-delta-byte-array-page-split-10489
branch
from
August 3, 2026 16:03
5b904cc to
c284081
Compare
Contributor
Author
|
run benchmark arrow_writer writer_overhead |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix-delta-byte-array-page-split-10489 (c284081) to f8a57f8 (merge-base) diff Run configurationrun benchmark arrow_writerBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writer File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix-delta-byte-array-page-split-10489 (c284081) to f8a57f8 (merge-base) diff Run configurationrun benchmark writer_overheadBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench writer_overhead File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing fix-delta-byte-array-page-split-10489 (c284081) to f8a57f8 (merge-base) diff Run configurationrun benchmark writer_overheadCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing fix-delta-byte-array-page-split-10489 (c284081) to f8a57f8 (merge-base) diff Run configurationrun benchmark arrow_writerCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
Contributor
Author
|
run benchmark arrow_writer writer_overhead |
1 similar comment
Contributor
Author
|
run benchmark arrow_writer writer_overhead |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix-delta-byte-array-page-split-10489 (c284081) to f8a57f8 (merge-base) diff Run configurationrun benchmark writer_overheadBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench writer_overhead File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix-delta-byte-array-page-split-10489 (c284081) to f8a57f8 (merge-base) diff Run configurationrun benchmark arrow_writerBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writer File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix-delta-byte-array-page-split-10489 (c284081) to f8a57f8 (merge-base) diff Run configurationrun benchmark arrow_writerBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writer File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix-delta-byte-array-page-split-10489 (c284081) to f8a57f8 (merge-base) diff Run configurationrun benchmark writer_overheadBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench writer_overhead File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing fix-delta-byte-array-page-split-10489 (c284081) to f8a57f8 (merge-base) diff Run configurationrun benchmark writer_overheadCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing fix-delta-byte-array-page-split-10489 (c284081) to f8a57f8 (merge-base) diff Run configurationrun benchmark writer_overheadCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing fix-delta-byte-array-page-split-10489 (c284081) to f8a57f8 (merge-base) diff Run configurationrun benchmark arrow_writerCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing fix-delta-byte-array-page-split-10489 (c284081) to f8a57f8 (merge-base) diff Run configurationrun benchmark arrow_writerCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
Keeping large values on the same page means the encoder finally has a previous value to dedup against — and the prefix scan that had been returning 0 on every value now runs over the whole value. It was written a byte at a time, so it became the dominant cost: profiling `large_string_shared_prefix/delta_byte_array` (128 x 2 MiB values sharing a 2 MiB prefix) puts 76% of total runtime in that one loop, which is what made the benchmark ~38% slower than before the page-limit fix. Compare a block at a time instead. On the same benchmark: base (before the page fix) 66.7 ms page fix only 90.3 ms +35.5% page fix + this 26.5 ms -60.3% `large_string_distinct` (no shared prefix) is unchanged at -40.6%, and the PARQUET_2_0 string paths that use DELTA_BYTE_ARRAY are flat. The same scan exists in the non-Arrow `DeltaByteArrayEncoder`, so both now share one helper. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
run benchmark arrow_writer |
2 similar comments
Contributor
Author
|
run benchmark arrow_writer |
Contributor
Author
|
run benchmark arrow_writer |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix-delta-byte-array-page-split-10489 (6190b5a) to f8a57f8 (merge-base) diff Run configurationrun benchmark arrow_writerBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writer File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix-delta-byte-array-page-split-10489 (6190b5a) to f8a57f8 (merge-base) diff Run configurationrun benchmark arrow_writerBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writer File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix-delta-byte-array-page-split-10489 (6190b5a) to f8a57f8 (merge-base) diff Run configurationrun benchmark arrow_writerBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writer File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing fix-delta-byte-array-page-split-10489 (6190b5a) to f8a57f8 (merge-base) diff Run configurationrun benchmark arrow_writerCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing fix-delta-byte-array-page-split-10489 (6190b5a) to f8a57f8 (merge-base) diff Run configurationrun benchmark arrow_writerCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing fix-delta-byte-array-page-split-10489 (6190b5a) to f8a57f8 (merge-base) diff Run configurationrun benchmark arrow_writerCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
Contributor
Author
Local benchmark results (Apple M4 Pro, aarch64)Since the CI runner has been noisy on this PR, I ran the same benchmarks locally as base ( Real changes — delta far outside that benchmark's own noise floor:
The
Everything else is flat. Of the remaining 36 benchmarks, 33 land within ±2%, and the three that don't ( The regressions CI reported do not reproduce. Specifically:
Worth noting that Profile: where the time actually wentOn After the change that loop is gone from the profile, and the benchmark is 2.5× faster than it was before either commit. Why 32-byte blocksIt is the widest block that both aarch64 and x86-64 still expand inline: at 64 bytes x86-64 (baseline SSE2) drops to an out-of-line |
Contributor
Author
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
3 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.
DELTA_BYTE_ARRAYstops deduplicating in parquet 59 for values larger than the page size limit #10489The cause is the page flush, not the mini-batch splitting
The issue attributes the regression to #9972's byte-budget sub-batching. It's actually the page flush; the sub-batching only exposes it.
should_add_data_pagefires whenestimated_data_page_size() >= data_page_size_limit. ForDELTA_BYTE_ARRAYthat estimate is the real encoded size, and the first value on a page is stored in full — so a single 8 MiB value against a 1 MiB limit puts the page over the limit by itself and triggers a flush. Flushing clears the encoder'sprevious, so the next value getsprefix_length = 0, and so on. The encoding degenerates to exactlyPLAIN.Before #9972 a 1024-row mini-batch meant the check simply didn't run until 1024 values had been written, so the dedup survived. That was an accident of
write_batch_size, not a designed property — the same column at 2000 rows already lost the prefix at the 1024-value boundary.This matters for choosing a fix: making the byte budget encoding-aware (suggestion 2 in the issue) does not fix the reported bug. However precise the budget, the first value alone still exceeds the limit and still triggers the flush.
The fix
Parquet requires at least one value per data page, so a value larger than the limit cannot be split out — the limit is unsatisfiable for it. Counting those bytes against the limit is what forces the pathological one-value-per-page cut.
Record that first value's encoded size in
PageMetrics::page_size_floorand apply the limit to what follows it, i.e. the bytes that can still go on another page.The exemption is gated on a new
ColumnValueEncoder::compresses_against_previous_value(defaultfalse,trueonly forDELTA_BYTE_ARRAYin both the generic and arrow encoders).PLAINandDELTA_LENGTH_BYTE_ARRAYcost the same wherever a value lands, so there is nothing to preserve by keeping values together and they keep their existing tighter one-value page bound. All three regression tests from #9972 pass unmodified.Why not "don't split when the split cannot help"
Suggestion 1 in the issue restores the dedup, but by removing the bound that #9972 added, for exactly the workload it was added for. Measured with
Some(1) => chunk_sizeinbyte_budget_chunker.rs, 2048 rows × 128 KiB distinct values against a 64 KiB page limit — the same regime as a 10 MB value against the 1 MiB default, scaled to fit in RAM:Page size under suggestion 1 is
write_batch_size × value_size. At 1024 × 10 MB that is a 10 GB page, which is the failure #9972 fixed.Results
The reporter's table, reproduced verbatim (10 identical 8 MiB values, raw input 80 MiB):
data_page_size_limitValues that merely share a long prefix behave the same. Values sharing no prefix are unaffected in file size and stay bounded at up to two values per page — the exempt one plus the one that trips the budget.
Tests
test_column_writer_delta_byte_array_dedups_large_shared_prefix_values— 16 identical 64 KiB values, 16 KiB page limit. Fails onmainwith 1 MiB across 16 pages (byte for byte whatPLAINproduces); passes here at ~one value's worth.test_column_writer_delta_byte_array_bounds_pages_without_shared_prefix— the same column with values differing from byte 0, asserting pages stay bounded by two values. Guards against fixing this by dropping the bound.test_large_string_delta_byte_array_shared_prefix— theArrowWriterpath from the report, as an exact page-layout assertion.Notes
One known limitation, pinned by
test_column_writer_delta_byte_array_nullable_shared_prefix_partial_dedup: the exemption triggers when a page opens with a mini-batch holding exactly one value. That is guaranteed for chunks without nulls (the byte-budget chunker gives an over-limit value its own mini-batch), but a null in a chunk makes the chunker convert values to levels by ratio and round up, so pages can open with a two-value mini-batch, miss the exemption, and store their first value in full. Dedup on such columns is partial rather than absent, and never worse thanmain, which stores every value in full. The clean fix is for the granular path to cut windows after an exact value count instead of ratio-scaling; that is independent of this change and tracked in #10538. The pinning test documents the layout to expect once it lands.A follow-up worth considering separately: the byte budget in
count_within_budget_*still measures raw payload length, soDELTA_BYTE_ARRAYcolumns sub-batch more eagerly than the encoded size warrants. That is a throughput question rather than a correctness one, and it is not what caused this regression.🤖 Generated with Claude Code