You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2020/04/07 09:05:27 UTC
[GitHub] [incubator-doris] vagetablechicken opened a new issue #3273:
fragment mem tracker doesn't be unregistered from parent
vagetablechicken opened a new issue #3273: fragment mem tracker doesn't be unregistered from parent
URL: https://github.com/apache/incubator-doris/issues/3273
**Describe the bug**
MemTracker ctor will do
https://github.com/apache/incubator-doris/blob/83b5455be57d00ef89f9bf6688e05503ab9f3c92/be/src/runtime/mem_tracker.cpp#L65
So before the mem tracker destroyed, we need to do unregister.
https://github.com/apache/incubator-doris/blob/4449316d856092fa1df41b55a8f687d35ccf90a7/be/src/runtime/mem_tracker.h#L96
Just focus on the top level, we have exec_env_mem_tracker(it has no parent).
And if one plan fragment has prepared, we get three mem_trackers
- query_mem_tracker(owned by runtime_state)
- instance_mem_tracker(owned by runtime_state)
- fragment_mem_tracker(owned by plan_fragment_executor)
query_mem_tracker & instance_mem_tracker are unregistered in here
https://github.com/apache/incubator-doris/blob/a340bc7a00bdbdf93f307c95e33545ed7a2b9245/be/src/runtime/runtime_state.cpp#L155-L172
**But fragment_mem_tracker doesn't be unregistered.**
So exec_env_mem_tracker will have lots of dangling pointers(child trackers), never delete.
It's unnecessary to do bottom level unregistered. But I think the top level is needed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] vagetablechicken commented on issue #3273:
fragment mem tracker doesn't be unregistered from parent
Posted by GitBox <gi...@apache.org>.
vagetablechicken commented on issue #3273: fragment mem tracker doesn't be unregistered from parent
URL: https://github.com/apache/incubator-doris/issues/3273#issuecomment-610717223
> @vagetablechicken
> I'm not sure what is your mean by "the top level is needed". Could you please explain more carefully?
OK. We can regard mem trackers as nodes.
_There are many trees, cause we may create a mem tracker as a no-parent node. But we don't need to pay attention on them._
Focus on one plan fragment,
the root mem tracker is exec_env_mem_tracker(level 0), and
the query_mem_tracker & fragment_mem_tracker are the root node's children(level 1). These can be called top-level nodes.
And the low-level nodes, such as scan node's mem_tracker, sink node's mem_tracker...(level >=2)
Low-level mem trackers will be destroyed when plan finished. And the top-level mem trackers will be destroyed too.
For example, the fragment_mem_tracker, we don't need to worry about dangling pointers(child trackers) in it, because it will be destroyed when plan fragment finished,
But the fragment_mem_tracker's pointer in exec_env_mem_tracker is never delete. If we use exec_env_mem_tracker and visit its children, we may access the deallocated memory.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] vagetablechicken commented on issue #3273:
fragment mem tracker doesn't be unregistered from parent
Posted by GitBox <gi...@apache.org>.
vagetablechicken commented on issue #3273: fragment mem tracker doesn't be unregistered from parent
URL: https://github.com/apache/incubator-doris/issues/3273#issuecomment-610819596
So, I will just add `_mem_tracker->unregister_from_parent()` in `PlanFragmentExecutor::close()`, to unregister the fragment mem tracker from exec_env_mem_tracker.
And we need add recursive depth for LogUsage(), ref impala
https://github.com/apache/impala/blob/d877dbc572e95f086b37142e1d41231d5b6b7f9f/be/src/runtime/mem-tracker.cc#L300
We should be careful about any access to child trackers.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] imay commented on issue #3273: fragment mem
tracker doesn't be unregistered from parent
Posted by GitBox <gi...@apache.org>.
imay commented on issue #3273: fragment mem tracker doesn't be unregistered from parent
URL: https://github.com/apache/incubator-doris/issues/3273#issuecomment-610726619
@vagetablechicken
Thanks for you careful explain, I see now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] vagetablechicken edited a comment on issue #3273:
fragment mem tracker doesn't be unregistered from parent
Posted by GitBox <gi...@apache.org>.
vagetablechicken edited a comment on issue #3273: fragment mem tracker doesn't be unregistered from parent
URL: https://github.com/apache/incubator-doris/issues/3273#issuecomment-610819596
So, I will just add `_mem_tracker->unregister_from_parent()` in `PlanFragmentExecutor::close()`, to unregister the fragment mem tracker from exec_env_mem_tracker.
We should be careful about any access to child trackers. So we need add recursive depth for LogUsage(), ref impala
https://github.com/apache/impala/blob/d877dbc572e95f086b37142e1d41231d5b6b7f9f/be/src/runtime/mem-tracker.cc#L300
It will be safe to call `exec_env_mem_tracker.LogUsage(1)` anytime. But if we want to log the low-level mem tracker, we must ensure the children are still alive.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] imay closed issue #3273: fragment mem tracker
doesn't be unregistered from parent
Posted by GitBox <gi...@apache.org>.
imay closed issue #3273: fragment mem tracker doesn't be unregistered from parent
URL: https://github.com/apache/incubator-doris/issues/3273
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] imay commented on issue #3273: fragment mem
tracker doesn't be unregistered from parent
Posted by GitBox <gi...@apache.org>.
imay commented on issue #3273: fragment mem tracker doesn't be unregistered from parent
URL: https://github.com/apache/incubator-doris/issues/3273#issuecomment-610833875
> So, I will just add `_mem_tracker->unregister_from_parent()` in `PlanFragmentExecutor::close()`, to unregister the fragment mem tracker from exec_env_mem_tracker.
Looking forward to your PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] imay commented on issue #3273: fragment mem
tracker doesn't be unregistered from parent
Posted by GitBox <gi...@apache.org>.
imay commented on issue #3273: fragment mem tracker doesn't be unregistered from parent
URL: https://github.com/apache/incubator-doris/issues/3273#issuecomment-610274100
@vagetablechicken
I'm not sure what is your mean by "the top level is needed". Could you please explain more carefully?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org