You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by "ASF GitHub Bot (Jira)" <ji...@apache.org> on 2021/11/15 17:11:00 UTC
[jira] [Commented] (PHOENIX-6592) PhoenixStatsCacheLoader uses non-deamon threads
[ https://issues.apache.org/jira/browse/PHOENIX-6592?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17443963#comment-17443963 ]
ASF GitHub Bot commented on PHOENIX-6592:
-----------------------------------------
virajjasani commented on a change in pull request #1351:
URL: https://github.com/apache/phoenix/pull/1351#discussion_r749522713
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/query/PhoenixStatsCacheLoader.java
##########
@@ -43,15 +46,20 @@
public PhoenixStatsCacheLoader(PhoenixStatsLoader statsLoader, Configuration config) {
this.statsLoader = statsLoader;
- if (this.executor == null) {
+ if (executor == null) {
synchronized (PhoenixStatsCacheLoader.class) {
- if (this.executor == null) {
+ if (executor == null) {
// The size of the thread pool used for refreshing cached table stats
final int statsCacheThreadPoolSize = config.getInt(
QueryServices.STATS_CACHE_THREAD_POOL_SIZE,
QueryServicesOptions.DEFAULT_STATS_CACHE_THREAD_POOL_SIZE);
-
- this.executor = Executors.newFixedThreadPool(statsCacheThreadPoolSize);
+ final ThreadFactory threadFactory =
+ new ThreadFactoryBuilder()
+ .setDaemon(true)
+ .setNameFormat("PHOENIX-STATS-CACHE-LOADER" + "-thread-%s")
Review comment:
nit: why not `"PHOENIX-STATS-CACHE-LOADER-thread-%s"` directly?
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/query/PhoenixStatsCacheLoader.java
##########
@@ -43,15 +46,20 @@
public PhoenixStatsCacheLoader(PhoenixStatsLoader statsLoader, Configuration config) {
this.statsLoader = statsLoader;
- if (this.executor == null) {
+ if (executor == null) {
synchronized (PhoenixStatsCacheLoader.class) {
- if (this.executor == null) {
+ if (executor == null) {
// The size of the thread pool used for refreshing cached table stats
final int statsCacheThreadPoolSize = config.getInt(
QueryServices.STATS_CACHE_THREAD_POOL_SIZE,
QueryServicesOptions.DEFAULT_STATS_CACHE_THREAD_POOL_SIZE);
-
- this.executor = Executors.newFixedThreadPool(statsCacheThreadPoolSize);
+ final ThreadFactory threadFactory =
+ new ThreadFactoryBuilder()
+ .setDaemon(true)
Review comment:
This is the main fix right? Looks good.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
> PhoenixStatsCacheLoader uses non-deamon threads
> -----------------------------------------------
>
> Key: PHOENIX-6592
> URL: https://issues.apache.org/jira/browse/PHOENIX-6592
> Project: Phoenix
> Issue Type: Bug
> Components: core
> Reporter: Josh Elser
> Assignee: Istvan Toth
> Priority: Major
>
> org.apache.phoenix.query.PhoenixStatsCacheLoader starts non deamon threads which can prevent the Phoenix client JVM from shutting down in a timely manner.
--
This message was sent by Atlassian Jira
(v8.20.1#820001)