Skip to content

Conversation

@xiaoxmeng
Copy link
Contributor

@xiaoxmeng xiaoxmeng commented Nov 14, 2023

If a task from a query completes before the other tasks get scheduled on the same
worker, then the query context might have been destroyed before creating the other
tasks from the same query. The query context manager maintains a query context
cache but it only holds the weak pointer. So that when the next task from the same
query comes, it will create a new query context with the same query id and creates
a root memory pool with the same name from the memory manager. If there is a
background memory arbitration process, then the old memory pool instance might
still be held by the memory arbitration process and run into duplicate root memory pool
issue.
This PR fixes this issue by appending a monotonically increasing id to root memory
pool name to ensure the instance name is unique.

@xiaoxmeng xiaoxmeng marked this pull request as ready for review November 14, 2023 23:02
@xiaoxmeng xiaoxmeng requested a review from a team as a code owner November 14, 2023 23:02
Copy link
Contributor

@mbasmanova mbasmanova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the fix.

If a task from a query completes before the other tasks get scheduled on the same
worker, then the query context might have been destroyed before creating the other
tasks from the same query. The query context manager maintains a query context
cache but it only holds the weak pointer. So that when the next task from the same
query comes, it will create a new query context with the same query id and creates
a root memory pool with the same name from the memory manager. If there is a
background memory arbitration process, then the old memory pool instance might
still be held by the memory arbitration process and run into duplicate root memory pool
issue.
This PR fixes this issue by appending a monotonically increasing id to root memory
pool name to ensure the instance name is unique.
@xiaoxmeng xiaoxmeng merged commit b2fc43c into prestodb:master Nov 15, 2023
@wanglinsong wanglinsong mentioned this pull request Dec 8, 2023
26 tasks
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.

2 participants