mbutrovich commented on code in PR #21484:
URL: https://github.com/apache/datafusion/pull/21484#discussion_r3054154449
##########
datafusion/core/tests/memory_limit/mod.rs:
##########
@@ -213,6 +213,7 @@ async fn sort_merge_join_spill() {
.with_config(config)
.with_disk_manager_builder(DiskManagerBuilder::default())
.with_scenario(Scenario::AccessLogStreaming)
Review Comment:
Claude's assessment:
> The test's original intent was to verify SMJ fails with OOM when memory is
too tight even for spilling. But at 1000 bytes with this data, spilling works
fine — there's no "too tight to spill" threshold we can easily hit because the
spill path writes to disk before the reservation grows.
>
> The reservation system works like: try_grow fails → spill to disk → no
reservation needed. As long as the disk manager works, there's no minimum
memory floor that would cause OOM. Even at 1 byte, the first try_grow would
fail and spill immediately.
>
> So the test as-is can't be made to fail with OOM under SMJ spilling — it
was only "working" because of the Dictionary bug.
I'm not sure if the test is worth keeping, but maybe that's a different PR.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
