xudong963 opened a new pull request, #12028: URL: https://github.com/apache/arrow-rs/pull/12028
# Which issue does this PR close? - Closes #12027. # Rationale for this change Since #10364 the Arrow writer hands `FixedLenByteArray` values to the column writer as `split_to` slices of one `Bytes` buffer per batch. The column writer keeps its running min/max as clones of the written values, which for byte-array types is a refcount bump, and when the column chunk is closed `truncate_statistics` only rebuilt min/max for types that may be truncated. Decimal and Float16 are exempt from truncation, so their chunk statistics went into `ColumnChunkMetaData` still pointing into the batch buffer. Every closed row group of such a column kept one or two batch-sized buffers (128 KiB each with 8192-row batches) alive for as long as the `ParquetMetaData` returned by `close()` lived, while `memory_size()` reported 16 bytes per value. See the issue for the full chain and the production impact (about 1 GiB of RSS per 10 GB written through DataFusion, which retains the metadata of every written file). # What changes are included in this PR? - `truncate_statistics` now always rebuilds `BYTE_ARRAY` / `FIXED_LEN_BYTE_ARRAY` min/max from a copy of their bytes. `can_truncate_value` now only decides the truncation length passed to `truncate_min_value` / `truncate_max_value` (`None` for Decimal and Float16), so those types are copied but never shortened. - The rebuilt statistics keep every other field of the incoming statistics (null, distinct and NaN counts, the deprecated and backwards-compatible flags) through a crate-private `ValueStatistics::with_min_max`; exactness becomes `was exact && !truncated`. Previously the rebuilt statistics were created from scratch with `ValueStatistics::new(.., sort_order().is_signed())`. That was equivalent for the unsigned types that reached this path, but Decimal and Float16 have a signed sort order, and reusing it would have set `is_min_max_deprecated` and dropped `min_value` / `max_value` from the footer. The thrift output is unchanged for all types. - Regression test: writes FLBA decimals sliced out of one `Bytes` buffer and asserts that the column chunk statistics do not point into it and that the deprecated / backwards-compatible flags are unchanged. It fails on `main` and passes with this change. It would be useful to backport this to the 59.x and 60.x patch lines, since the aliasing exists since 59.0.0. # Are these changes tested? Yes. `cargo test -p parquet --lib` passes (1487 tests, including the existing truncation / Decimal / Float16 statistics tests) and `cargo clippy -p parquet --all-targets --all-features -- -D warnings` is clean. The allocator-counting reproducer from #12027, rebuilt against this branch (64 batches x 8192 `Decimal128(38, 8)` rows written through `ArrowWriter`, only the `ParquetMetaData` kept after `close()`): | row groups | `metadata.memory_size()` | live heap after `close()`, parquet 60.1.0 | live heap after `close()`, this PR | |---|---|---|---| | 512 (1024 rows each) | 341 KB | 8.73 MB | 366 KB | | 16 (4 batches each) | 12 KB | 4.21 MB | 12.7 KB | # Are there any user-facing changes? No API changes. Column chunk statistics for Decimal and Float16 columns no longer retain the buffers the values were written from. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
