Add tests for record batch size splitting logic in FlightClient #3481
Conversation
| ]) | ||
| .unwrap(); | ||
|
|
||
| verify_encoded_split(batch, 112).await; |
There was a problem hiding this comment.
This is pretty good -- only 112 bytes above desired max (I think that is mostly the various alignment and padding overhead)
| .unwrap(); | ||
|
|
||
| // 5k over limit (which is 2x larger than limit of 5k) -- not great :( | ||
| verify_encoded_split(batch, 5800).await; |
There was a problem hiding this comment.
This is pretty bad -- over 2x larger than the largest message size limit
arrow-flight/src/encode.rs
Outdated
| } | ||
| } | ||
|
|
||
| // ensure that the specified overage is exactly the maxmium than necessary |
There was a problem hiding this comment.
This comment is a little funky
Good idea -- added |
|
Benchmark runs are scheduled for baseline = b4d5705 and contender = 7805a81. 7805a81 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 #3478
Rationale for this change
There is ongoing drama downstream in IOx related to maximum message sizes.
This PR adds some tests for important cases to document the current behavior (and hopefully make fixing #3478 easier)
What changes are included in this PR?
Tests that encode RecordBatches and check how far off the encoded size is from the requested "flight_max_data_size"
Are there any user-facing changes?
No