-
Notifications
You must be signed in to change notification settings - Fork 29k
[DNM][SPARK-54934][CORE] Make UnsafeInMemorySorter free memory thread-safety #53710
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
JIRA Issue Information=== Improvement SPARK-54934 === This comment was automatically generated by GitHub Actions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Ngone51
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this!
The log indicates that two spills were called by the same thread 152. So it doesn't make sense to me that the issue is due to the concurrent threads.
|
Could you share the complete logs or do you have a reproduciable example? |
|
@Ngone51 Thank you for your reply. I mistakenly thought that the spilling operation was not releasing memory correctly. After adding some logs, I think this might be because the Window operator cannot trigger the memory spill of the subsequent ObjectHashAggregate sorter. The SparkPlan like this: |
|
I will try adding the |
What changes were proposed in this pull request?
Make UnsafeInMemorySorter free memory thread-safety
Why are the changes needed?
I encountered a
SparkOutOfMemoryError. Logs indicate that memory is held byUnsafeExternalSorter, butUnsafeExternalSorteris supposed to trigger spilling and release memory.UnsafeInMemorySorter.freeMemoryis not thread-safe, whileUnsafeExternalSorter.spillmay be called concurrently by multiple threads, which could lead to a memory leak. Related logs:Does this PR introduce any user-facing change?
No
How was this patch tested?
add unit test
Was this patch authored or co-authored using generative AI tooling?
No