Fix the race between memory release and task reclaim #8086
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for meta-velox canceled.
|
|
@xiaoxmeng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@xiaoxmeng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
mbasmanova
approved these changes
Dec 18, 2023
| @@ -2590,7 +2590,15 @@ uint64_t Task::MemoryReclaimer::reclaim( | |||
| if (task->isCancelled()) { | |||
| return 0; | |||
| } | |||
| return memory::MemoryReclaimer::reclaim(pool, targetBytes, maxWaitMs, stats); | |||
| // Before reclaiming from its operators, first to check if there is any free | |||
| // capacity in the root after stopping this task. | |||
There was a problem hiding this comment.
Is this needed because memory could have been freed while we waited for the task to pause?
There was a problem hiding this comment.
Yes, and we didn't check the free memory after that.
|
@xiaoxmeng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
There is race between memory release and task reclaim that cause a unnecessary local OOM in Meta internal shadowing test: T1. memory arbitration triggers from a query T2. memory arbitrator find the memory grow request exceeds its the query memory capacity limit so starts to reclaim from this query T3. one or more operators from this query releases a large chunk of memory which has sufficient space to satisfy the memory arbitration request T4. the memory arbitration wait for the task to complete and then reclaim from each of its operator T5. however all the operators have no memory to reclaim (T3 has freed all the reclaimable memory) T6. memory arbitrator found nothing has been reclaimed from this query and fail the memory arbitration request. However, there is free memory after T3. This PR fixes this issue by shrink the query memory pool after the task reclaim pauses the task. Verify with table writer which is the case found in Meta shadowing test. Also improve the memory arbitration logging a bit.
|
@xiaoxmeng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@xiaoxmeng merged this pull request in fa6de06. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There is race between memory release and task reclaim that cause a
unnecessary local OOM in Meta internal shadowing test:
T1. memory arbitration triggers from a query
T2. memory arbitrator find the memory grow request exceeds its the query
memory capacity limit so starts to reclaim from this query
T3. one or more operators from this query releases a large chunk of memory
which has sufficient space to satisfy the memory arbitration request
T4. the memory arbitration wait for the task to complete and then reclaim
from each of its operator
T5. however all the operators have no memory to reclaim (T3 has freed all
the reclaimable memory)
T6. memory arbitrator found nothing has been reclaimed from this query and
fail the memory arbitration request.
However, there is free memory after T3. This PR fixes this issue by shrink the
query memory pool after the task reclaim pauses the task. Verify with table
writer which is the case found in Meta shadowing test.
Also improve the memory arbitration logging a bit.