Improve arrow flight batch splitting and naming #3444
Conversation
| max_batch_size: usize, | ||
| /// The maximum approximate target message size in bytes | ||
| /// (see details on [`Self::with_max_flight_data_size`]). | ||
| max_flight_data_size: usize, |
There was a problem hiding this comment.
I renamed this to max_flight_data_size and tried to clarify the intent more
| max_flight_data_size_bytes: usize, | ||
| expected_sizes: Vec<usize>, | ||
| ) { | ||
| let array: UInt64Array = (0..num_input_rows).collect(); |
There was a problem hiding this comment.
It occurs to me that this splitting won't work for dictionaries, not really sure what we can do about that though... 🤔
There was a problem hiding this comment.
🤔 I guess we hope for small dictionaries
|
Benchmark runs are scheduled for baseline = 4a3b7e9 and contender = e256e3d. e256e3d is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
re #3371
Rationale for this change
I wrote some tests for this code in https://github.com/influxdata/influxdb_iox/pull/6484 (actually covers a bug I fixed when upstreaming to arrow-rs)
Also, it would be nice to clarify that the units are in bytes and not some other unit (like rows, for example) as suggested by @carols10cents on https://github.com/influxdata/influxdb_iox/pull/6460
What changes are included in this PR?
max_message_sizetomax_message_size_bytesAre there any user-facing changes?
No : Note that this code has not been released yet, so there are no external API changes.