You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2021/03/09 09:24:56 UTC
[GitHub] [hive] klcopp opened a new pull request #2052: HIVE-24860: DbTxnManager$HeartBeaterThread thread leak
klcopp opened a new pull request #2052:
URL: https://github.com/apache/hive/pull/2052
### What changes were proposed in this pull request?
Shut down thread pool HeartbeatExecutorService upon session close.
### Why are the changes needed?
If the HeartBeaterExecutorService isn't shut down, UDF classes belonging to the session's UDFClassLoader (which was the session thread's context classloader at HeartBeaterExecutorService creation) could pile up and cause Metaspace OOM.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Manually
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on pull request #2052: HIVE-24860: Shut down DbTxnManager heartbeatExecutorService at session close
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on pull request #2052:
URL: https://github.com/apache/hive/pull/2052#issuecomment-793679004
> > @klcopp, with [HIVE-24179](https://issues.apache.org/jira/browse/HIVE-24179) heartbeatExecutorService was made to be a global to avoid multiple shutdown hooks for the same threadpool. cc @zabetak
>
> Thanks for catching this @deniskuzZ!
>
> Strange thing that the service is static, but not initialized in a static way.
> The first user might set the pool size to a small one, and then later ones can not change it... Is there any particular reason behind this?
I don't think we should allow to change this config at session level. Idea was to have a single threadpool for txn heartbeating job. Btw initialization is also under the static method.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on pull request #2052: HIVE-24860: Shut down DbTxnManager heartbeatExecutorService at session close
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on pull request #2052:
URL: https://github.com/apache/hive/pull/2052#issuecomment-793658340
@klcopp, with HIVE-24179 heartbeatExecutorService was made to be a global to avoid multiple shutdown hooks for the same threadpool.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] zabetak edited a comment on pull request #2052: HIVE-24860: Shut down DbTxnManager heartbeatExecutorService at session close
Posted by GitBox <gi...@apache.org>.
zabetak edited a comment on pull request #2052:
URL: https://github.com/apache/hive/pull/2052#issuecomment-793695672
> heartbeatExecutorService was made to be a global to avoid ...
Small clarification: the service was already global (static), this didn't change in HIVE-24179.
The service seems to be directly related with the lifecycle of the DbTxnManager so it could be a good idea to make it instance based. This PR addresses another problem so we could continue this discussion (instance vs class variable) if necessary in a separate JIRA.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] github-actions[bot] commented on pull request #2052: HIVE-24860: Shut down DbTxnManager heartbeatExecutorService at session close
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #2052:
URL: https://github.com/apache/hive/pull/2052#issuecomment-855489155
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Feel free to reach out on the dev@hive.apache.org list if the patch is in need of reviews.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] klcopp commented on pull request #2052: HIVE-24860: Shut down DbTxnManager heartbeatExecutorService at session close
Posted by GitBox <gi...@apache.org>.
klcopp commented on pull request #2052:
URL: https://github.com/apache/hive/pull/2052#issuecomment-793719805
Thanks for chiming in, @zabetak and @deniskuzZ ! I will leave the heartbeatExecutorService static as shutdown needs to be part of the shutdownhook.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] github-actions[bot] closed pull request #2052: HIVE-24860: Shut down DbTxnManager heartbeatExecutorService at session close
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #2052:
URL: https://github.com/apache/hive/pull/2052
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on pull request #2052: HIVE-24860: Shut down DbTxnManager heartbeatExecutorService at session close
Posted by GitBox <gi...@apache.org>.
pvary commented on pull request #2052:
URL: https://github.com/apache/hive/pull/2052#issuecomment-793690150
> I don't think we should allow to change this config at session level. Idea was to have a single threadpool for txn heartbeating job. Btw initialization is also under the static method.
Yeah, `DbTxnManager.initHeartbeatExecutorService()` is static-like - would it hurt to create the ExecutorService in a static block - using the global config, and use this service everywhere?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on pull request #2052: HIVE-24860: Shut down DbTxnManager heartbeatExecutorService at session close
Posted by GitBox <gi...@apache.org>.
pvary commented on pull request #2052:
URL: https://github.com/apache/hive/pull/2052#issuecomment-793669781
> @klcopp, with [HIVE-24179](https://issues.apache.org/jira/browse/HIVE-24179) heartbeatExecutorService was made to be a global to avoid multiple shutdown hooks for the same threadpool. cc @zabetak
Thanks for catching this @deniskuzZ!
Strange thing that the service is static, but not initialized in a static way.
The first user might set the pool size to a small one, and then later ones can not change it... Is there any particular reason behind this?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ edited a comment on pull request #2052: HIVE-24860: Shut down DbTxnManager heartbeatExecutorService at session close
Posted by GitBox <gi...@apache.org>.
deniskuzZ edited a comment on pull request #2052:
URL: https://github.com/apache/hive/pull/2052#issuecomment-793658340
@klcopp, with HIVE-24179 heartbeatExecutorService was made to be a global to avoid multiple shutdown hooks for the same threadpool. cc @zabetak
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] zabetak commented on pull request #2052: HIVE-24860: Shut down DbTxnManager heartbeatExecutorService at session close
Posted by GitBox <gi...@apache.org>.
zabetak commented on pull request #2052:
URL: https://github.com/apache/hive/pull/2052#issuecomment-793695672
> heartbeatExecutorService was made to be a global to avoid ...
Small clarification: the service was already global (static), this didn't change in HIVE-24179.
The service seems to be directly related with the lifecycle of the DbTxnManager so it could be a good idea to make it instance based. This PR addresses another problem so we could continue this discussion (instance vs class variable) if necessary in a separate JIRA.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org