Add integration test for erroring when memory limits are hit #4406
Conversation
alamb
force-pushed
the
alamb/memory_limit_tests
branch
from
50b3509
to
2617ef3
Compare
| "Cannot spill AggregationState".to_owned(), | ||
| )) | ||
| Err(DataFusionError::ResourcesExhausted(format!( | ||
| "Cannot spill {}", |
There was a problem hiding this comment.
Improved error message here
| if in_mem_batches.len() == 0 { | ||
| return Ok(0); | ||
| } | ||
|
|
||
| debug!( |
There was a problem hiding this comment.
I just moved the debug message down as it was confusing that spill didn't actually happen if in_mem_batches was empty
| // use a random number generator to pick a random sized output | ||
| let mut rng = StdRng::seed_from_u64(42); |
There was a problem hiding this comment.
This piece of code was copied in several places so I refactored it into stagger_batch
alamb
force-pushed
the
alamb/memory_limit_tests
branch
from
2617ef3
to
4be318c
Compare
|
cc @crepererum and @milenkovicm |
| .await | ||
| } | ||
|
|
||
| /// 100K memory limit |
There was a problem hiding this comment.
100k? How does relate to 50 and 0.95?
There was a problem hiding this comment.
Whoops -- the 100k is left over from a previous revision -- I will update the comment
|
Benchmark runs are scheduled for baseline = 66c95e7 and contender = 49166ea. 49166ea is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Draft as it is waiting on:ResourceExhaustedfor memory limit inAggregateStream#4405Which issue does this PR close?
Resolves #4404
Rationale for this change
I would like end to end coverage that when run with a memory limit, queries will error (rather than exceed the memory limit)
What changes are included in this PR?
memory_limitteststagger_batchinto a functionAre these changes tested?
Are there any user-facing changes?