Skip to content

fix: honor max_row_group_bytes in the parallel Parquet writer - #25041

Open
Satyr09 wants to merge 2 commits into
apache:mainfrom
Satyr09:daipayan/parallel-max-row-group-bytes-barrier-corrected
Open

fix: honor max_row_group_bytes in the parallel Parquet writer#25041
Satyr09 wants to merge 2 commits into
apache:mainfrom
Satyr09:daipayan/parallel-max-row-group-bytes-barrier-corrected

Conversation

@Satyr09

@Satyr09 Satyr09 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Closes #22982.

Rationale for this change

max_row_group_bytes, added in #22649, is currently ignored when parallel writing is enabled (the default). This PR lets users apply the byte target without switching to the single-threaded writer.

What changes are included in this PR?

Column workers report their estimated encoded sizes. When a byte target is set, the writer waits for all columns to finish each input slice before deciding how many rows to write next. This follows the single-threaded writer's splitting policy and produces the same row-group boundaries for the same batches and settings.

Columns still encode in parallel, but waiting between slices can reduce throughput. This synchronization is only used when the byte target is set. The docs explain the tradeoff.

What is the testing strategy for this PR?

Added SQL and writer tests for row/byte limits, readback, matching serial boundaries, nested data, compression, empty batches, errors, cancellation, and encryption.

I also ran DataFusion's existing SQL benchmark_runner with COPY workloads on a four-CPU Linux runner. Each case wrote 131,072 rows; the table shows median times in ms from two rounds, excluding warmups. Results and workloads/setup are available on my fork.

Input Original parallel This PR parallel Serial
Four integer columns 20.82 16.65 32.00
Four wide string columns 276.86 292.99 643.81
String widths rotate between columns 99.77 111.78 226.39
64 Boolean columns 21.37 24.57 39.41

All three modes had the byte target set, but the original parallel writer ignores it. This PR matched the serial writer's row-group layouts in all four cases, and all retained outputs passed readback checks. Timings varied between rounds, so these are preliminary results.

With the option unset, three of the four controls were within 0.5% of the original median; the wide-string control was noisy.

One issue still to investigate: peak tracked memory for rotating strings rose from 14.43 MiB to 40.22 MiB in both rounds. The original writer produced one group and this version produced six, but the cause of the increase has not been isolated. These figures are DataFusion memory reservations, not process RSS.

Are there any user-facing changes?

Yes. The byte target now works with parallel writing. It remains an estimate that can overshoot, not a hard byte or memory limit. The default stays None, with only the row-count limit applied.

Synchronize coherent per-column progress and preserve serial row-group boundaries. Cover nested root rows, fractional sizes, failure propagation, cancellation, and encrypted group ordinals. Local Parquet tests and focused Clippy pass; full validation is moving to Linux CI as requested.
@github-actions github-actions Bot added documentation Improvements or additions to documentation sqllogictest SQL Logic Tests (.slt) common Related to common crate datasource Changes to the datasource crate labels Sep 7, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.57426% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.72%. Comparing base (5bf6aef) to head (28c48da).
⚠️ Report is 26 commits behind head on main.

Files with missing lines Patch % Lines
datafusion/datasource-parquet/src/sink.rs 92.57% 9 Missing and 6 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #25041      +/-   ##
==========================================
+ Coverage   81.64%   81.72%   +0.08%     
==========================================
  Files        1125     1127       +2     
  Lines      413843   416419    +2576     
  Branches   413843   416419    +2576     
==========================================
+ Hits       337868   340312    +2444     
- Misses      56056    56111      +55     
- Partials    19919    19996      +77     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Satyr09
Satyr09 marked this pull request as ready for review September 7, 2026 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

common Related to common crate datasource Changes to the datasource crate documentation Improvements or additions to documentation sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Honour max_row_group_bytes in the parallel Parquet writer

2 participants