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/23 09:52:08 UTC
[GitHub] [incubator-doris] caiconghui opened a new pull request #3386: User ThreadPoolManager to create threadPool and add some prothemus metric about pool
caiconghui opened a new pull request #3386:
URL: https://github.com/apache/incubator-doris/pull/3386
This PR to to resolve the problem that the usage of CachedPool or FixedThreadPool which not limit thread num or blocked task num may cause jvm process crashed due to too much thread or oom, we must construct threadpool explicitly, and know the runtime state about 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: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on a change in pull request #3386: User ThreadPoolManager to create threadPool and add some prothemus metric about pool
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3386:
URL: https://github.com/apache/incubator-doris/pull/3386#discussion_r413859899
##########
File path: fe/src/main/java/org/apache/doris/common/ThreadPoolManager.java
##########
@@ -0,0 +1,148 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.common;
+
+import com.google.common.collect.Maps;
+import com.google.common.util.concurrent.ThreadFactoryBuilder;
+import org.apache.doris.metric.GaugeMetric;
+import org.apache.doris.metric.MetricLabel;
+import org.apache.doris.metric.MetricRepo;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import java.util.Map;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.RejectedExecutionHandler;
+import java.util.concurrent.SynchronousQueue;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+
+
+public class ThreadPoolManager {
+
+ private static Map<String, ThreadPoolExecutor> nameToThreadPool = Maps.newConcurrentMap();
+
+ private static String[] poolMerticTypes = {"pool_size", "active_thread_num", "task_in_queue"};
+
+ public static void registerAllThreadPoolMetric() {
+ for (Map.Entry<String, ThreadPoolExecutor> entry : nameToThreadPool.entrySet()) {
+ registerThreadPoolMetric(entry.getKey(), entry.getValue());
+ }
+ }
+
+ public static void registerThreadPoolMetric(String poolName, ThreadPoolExecutor threadPool) {
+ for (String poolMetricType : poolMerticTypes) {
+ GaugeMetric<Integer> gauge = new GaugeMetric<Integer>("thread_pool", "thread_pool statistics") {
+ @Override
+ public Integer getValue() {
+ String metricType = this.getLabels().get(1).getValue();
+ switch (metricType) {
+ case "pool_size":
+ return threadPool.getPoolSize();
+ case "active_thread_num":
+ return threadPool.getActiveCount();
+ case "task_in_queue":
+ return threadPool.getQueue().size();
+ default:
+ return 0;
+ }
+ }
+ };
+ gauge.addLabel(new MetricLabel("name", poolName))
+ .addLabel(new MetricLabel("type", poolMetricType));
+ MetricRepo.addMetric(gauge);
+ }
+ }
+
+ public static ThreadPoolExecutor newDaemonCacheThreadPool(int maxNumThread, String poolName) {
+ return newDaemonThreadPool(0, maxNumThread, 60L, TimeUnit.SECONDS, new SynchronousQueue(), new LogDiscardPolicy(poolName), poolName);
Review comment:
Better define this `60` as a static field in frontend of the class
##########
File path: fe/src/main/java/org/apache/doris/mysql/nio/NMysqlServer.java
##########
@@ -47,7 +46,7 @@
private AcceptingChannel<StreamConnection> server;
// default task service.
- private ExecutorService taskService = Executors.newCachedThreadPool((new ThreadFactoryBuilder().setDaemon(false).setNameFormat("doris-mysql-nio TASK").build()));
+ private ExecutorService taskService = ThreadPoolManager.newDaemonCacheThreadPool(Config.max_mysql_service_task_threads_num, "doris-mysql-nio-pool");
Review comment:
The origin `taskService` is not a daemon thread, does not matter?
----------------------------------------------------------------
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: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] caiconghui commented on a change in pull request #3386: User ThreadPoolManager to create threadPool and add some prothemus metric about pool
Posted by GitBox <gi...@apache.org>.
caiconghui commented on a change in pull request #3386:
URL: https://github.com/apache/incubator-doris/pull/3386#discussion_r414693391
##########
File path: fe/src/main/java/org/apache/doris/mysql/nio/NMysqlServer.java
##########
@@ -47,7 +46,7 @@
private AcceptingChannel<StreamConnection> server;
// default task service.
- private ExecutorService taskService = Executors.newCachedThreadPool((new ThreadFactoryBuilder().setDaemon(false).setNameFormat("doris-mysql-nio TASK").build()));
+ private ExecutorService taskService = ThreadPoolManager.newDaemonCacheThreadPool(Config.max_mysql_service_task_threads_num, "doris-mysql-nio-pool");
Review comment:
It doesn't matter, because thread which submit task is not a daemon thread. for the whole fe jvm process, only when there are no non-daemon threads will cause jvm process exit
----------------------------------------------------------------
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: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] caiconghui commented on a change in pull request #3386: User ThreadPoolManager to create threadPool and add some prothemus metric about pool
Posted by GitBox <gi...@apache.org>.
caiconghui commented on a change in pull request #3386:
URL: https://github.com/apache/incubator-doris/pull/3386#discussion_r414693391
##########
File path: fe/src/main/java/org/apache/doris/mysql/nio/NMysqlServer.java
##########
@@ -47,7 +46,7 @@
private AcceptingChannel<StreamConnection> server;
// default task service.
- private ExecutorService taskService = Executors.newCachedThreadPool((new ThreadFactoryBuilder().setDaemon(false).setNameFormat("doris-mysql-nio TASK").build()));
+ private ExecutorService taskService = ThreadPoolManager.newDaemonCacheThreadPool(Config.max_mysql_service_task_threads_num, "doris-mysql-nio-pool");
Review comment:
It doesn't matter, because thread which submit task is not a daemon thread. for the whole fe jvm process, only then there are no non-daemon threads will cause jvm process exit
----------------------------------------------------------------
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: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] caiconghui commented on issue #3386: User ThreadPoolManager to create threadPool and add some prothemus metric about pool
Posted by GitBox <gi...@apache.org>.
caiconghui commented on issue #3386:
URL: https://github.com/apache/incubator-doris/pull/3386#issuecomment-618304586
Ref 3387
----------------------------------------------------------------
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: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] caiconghui commented on a change in pull request #3386: User ThreadPoolManager to create threadPool and add some prothemus metric about pool
Posted by GitBox <gi...@apache.org>.
caiconghui commented on a change in pull request #3386:
URL: https://github.com/apache/incubator-doris/pull/3386#discussion_r414693391
##########
File path: fe/src/main/java/org/apache/doris/mysql/nio/NMysqlServer.java
##########
@@ -47,7 +46,7 @@
private AcceptingChannel<StreamConnection> server;
// default task service.
- private ExecutorService taskService = Executors.newCachedThreadPool((new ThreadFactoryBuilder().setDaemon(false).setNameFormat("doris-mysql-nio TASK").build()));
+ private ExecutorService taskService = ThreadPoolManager.newDaemonCacheThreadPool(Config.max_mysql_service_task_threads_num, "doris-mysql-nio-pool");
Review comment:
It doesn't matter, because nio threads "doris-mysql-nio" are not daemon. for the whole fe jvm process, only when there are no non-daemon threads will cause jvm process exit
----------------------------------------------------------------
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: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] caiconghui edited a comment on issue #3386: User ThreadPoolManager to create threadPool and add some prothemus metric about pool
Posted by GitBox <gi...@apache.org>.
caiconghui edited a comment on issue #3386:
URL: https://github.com/apache/incubator-doris/pull/3386#issuecomment-618304586
Ref #3387
----------------------------------------------------------------
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: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] caiconghui commented on a change in pull request #3386: User ThreadPoolManager to create threadPool and add some prothemus metric about pool
Posted by GitBox <gi...@apache.org>.
caiconghui commented on a change in pull request #3386:
URL: https://github.com/apache/incubator-doris/pull/3386#discussion_r414693391
##########
File path: fe/src/main/java/org/apache/doris/mysql/nio/NMysqlServer.java
##########
@@ -47,7 +46,7 @@
private AcceptingChannel<StreamConnection> server;
// default task service.
- private ExecutorService taskService = Executors.newCachedThreadPool((new ThreadFactoryBuilder().setDaemon(false).setNameFormat("doris-mysql-nio TASK").build()));
+ private ExecutorService taskService = ThreadPoolManager.newDaemonCacheThreadPool(Config.max_mysql_service_task_threads_num, "doris-mysql-nio-pool");
Review comment:
It doesn't matter, for thread which submit task is not a daemon thread. for the whole fe jvm process, only then there are no non-daemon threads will cause jvm process exist
----------------------------------------------------------------
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: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on a change in pull request #3386: User ThreadPoolManager to create threadPool and add some prothemus metric about pool
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3386:
URL: https://github.com/apache/incubator-doris/pull/3386#discussion_r413846053
##########
File path: fe/src/main/java/org/apache/doris/common/ThreadPoolManager.java
##########
@@ -0,0 +1,148 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.common;
+
+import com.google.common.collect.Maps;
+import com.google.common.util.concurrent.ThreadFactoryBuilder;
+import org.apache.doris.metric.GaugeMetric;
+import org.apache.doris.metric.MetricLabel;
+import org.apache.doris.metric.MetricRepo;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import java.util.Map;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.RejectedExecutionHandler;
+import java.util.concurrent.SynchronousQueue;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+
+
+public class ThreadPoolManager {
Review comment:
Could add a comment to explain the usage of this class?
----------------------------------------------------------------
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: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org