Skip to content

Query failing with exceed local memory does not free memory correctly#17984

Merged
highker merged 1 commit intoprestodb:masterfrom
swapsmagic:memory_pool_leak
Jul 5, 2022
Merged

Query failing with exceed local memory does not free memory correctly#17984
highker merged 1 commit intoprestodb:masterfrom
swapsmagic:memory_pool_leak

Conversation

@swapsmagic
Copy link
Copy Markdown
Contributor

@swapsmagic swapsmagic commented Jul 5, 2022

Query with HashBuildOperator, if fails with exceed local memory doesn't free memory for the operator.
This leads to general pool keeping the memory for it and makes the amount of memory not usable by other
running queries.
Reason for the leak, we reserve the memory and then do memory constraint check. And if query fails due to memory
constraint, we don't free the reserved memory. We changing the order so first do memory constraint check to address it.

Test plan - Unit test, HiveQueryRunner test locally

== RELEASE NOTES ==

General Changes
* Fix broadcast join memory leak caused by exceeding local memory query failure

@swapsmagic swapsmagic requested a review from a team as a code owner July 5, 2022 17:43
@swapsmagic swapsmagic requested review from presto-oss and removed request for a team July 5, 2022 17:43
@swapsmagic swapsmagic force-pushed the memory_pool_leak branch 2 times, most recently from dd8dc8d to a51934f Compare July 5, 2022 17:50
Query with HashBuildOperator, if fails with exceed local memory doesn't free memory for the operator.
This leads to general pool keeping the memory for it and makes the amount of memory not usable by other
running queries.
Reason for the leak, we reserve the memory and then do memory constraint check. And if query fails due to memory
constraint, we don't free the reserved memory. We changing the order so first do memory constraint check to address it.
@tdcmeehan tdcmeehan self-requested a review July 5, 2022 18:11
@swapsmagic swapsmagic changed the title Query failing with exceed local memory doesn't free memory correctly Query failing with exceed local memory does not free memory correctly Jul 5, 2022
@highker highker self-assigned this Jul 5, 2022
@highker highker merged commit d42d354 into prestodb:master Jul 5, 2022
@highker highker mentioned this pull request Jul 6, 2022
7 tasks
@swapsmagic swapsmagic deleted the memory_pool_leak branch July 12, 2022 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants