fix(arrow-flight): adaptively re-split oversized FlightData based on actual encoded size - #11048
Open
DevMattG wants to merge 5 commits into
Open
fix(arrow-flight): adaptively re-split oversized FlightData based on actual encoded size#11048DevMattG wants to merge 5 commits into
DevMattG wants to merge 5 commits into
Conversation
… size `split_batch_for_grpc_response` only estimates a RecordBatch's encoded size from `get_buffer_memory_size()`, which ignores IPC framing/padding overhead and can badly undercount for variable-length data (dictionaries, strings). This let `FlightDataEncoder` silently emit `FlightData` messages larger than `max_flight_data_size` (see apache#3478). `FlightDataEncoder::encode_batch` now delegates to a new `encode_piece_adaptive`, which encodes each heuristic-split piece, queues any dictionaries unconditionally (they're keyed by id and correct to send regardless of how the batch body ends up sliced), and if the *actual* encoded size still exceeds the limit, re-splits, recursively if necessary. To avoid paying for that fallback on every batch whose estimated size lands right at the limit, the first-pass heuristic now targets 90% of `max_flight_data_size` (`SPLIT_HEURISTIC_MARGIN_DIVISOR`), leaving headroom for the framing overhead the estimate doesn't account for.
DevMattG
marked this pull request as ready for review
September 10, 2026 14:28
github-actions
Bot
added
arrow
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
2 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.
Which issue does this PR close?
Rationale for this change
FlightDataEncodersplits record batches based on a cheap, buffer-memory-size heuristic before IPC encoding. That heuristic can be significantly wrong -- it ignores IPC framing/padding overhead, and undercounts batches with dictionaries or highly skewed row sizes -- so pieces produced by the initial split can still end up well overmax_flight_data_sizeonce actually encoded.What changes are included in this PR?
max_flight_data_sizeinFlightDataEncoder::encode_piece_adaptive. If it's still too large, the piece is re-split using the real size ratio and the sub-pieces are recursively re-checked, so oversized pieces get corrected regardless of why the initial estimate was wrong.split_batch_for_grpc_response's batch-count calculation to round up (div_ceil) instead of using floor division, which could previously produce one too few batches.Are these changes tested?
Yes. Added/extended unit tests in
arrow-flight/src/encode.rscovering:DictionaryHandling::Resendand the defaultDictionaryHandling::Hydrate.max_flight_data_size(can't be split further, must still be emitted).Are there any user-facing changes?
No breaking API changes.