Fix flaky test_sort_fetch_memory_calculation test
#7534
Conversation
| ); | ||
|
|
||
| let result = collect(sort_exec.clone(), task_ctx).await?; | ||
| let result = collect(sort_exec.clone(), task_ctx.clone()).await?; |
There was a problem hiding this comment.
task_ctx is simply moved into task_ctx. Before the stream future is resolved, it could be dropped early which makes DiskManager dropped too. That is why the spill file existed before proceeding into read_spill_as_stream but was deleted intermittently before reading.
|
cc @alamb |
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Thanks @viirya -- I agree this solves the problem for this test. Thank you 🙏
However, it does seem like a real problem that the ExternalSorter can have its temporary files deleted out from under it. I filed #7546 to track that.
See also #7538 (comment) for more discussion
alamb
changed the title
test_sort_fetch_memory_calculation test
|
Thanks @alamb. Yea, I noticed the issue. I think for DataFusion itself usage, e.g., through DataFrame API, the TaskContext is kept in the session so normally the temporary dirs won't be deleted too early. Only problematic cases are when you construct physical query plan (like we did internally) like this test. I think most users are not in this cases. So I didn't put a fix immediately for this but propose a quick fix like this. And yes, I saw #7538 was proposed last night. I will take a look too. |
2 participants
Which issue does this PR close?
Closes #7537,
Closes #7523
Rationale for this change
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?