From 4d36bf734a67c083ca1cbb1837d9aaccb8191626 Mon Sep 17 00:00:00 2001 From: zhuixun <492369601@qq.com> Date: Mon, 15 Mar 2021 10:06:32 +0800 Subject: [PATCH] [Refactor] Make sure the singleton thread is safe (#5428) The following class org.apache.doris.clone.ColocateTableBalancer org.apache.doris.common.proc.ProcService org.apache.doris.rpc.BackendServiceProxy org.apache.doris.common.util.ProfileManager org.apache.doris.qe.HelpModule org.apache.doris.common.publish.ClusterStatePublisher is not safe in multiple thread environment. This PR is to implement a secure singleton mode. The class org.apache.doris.common.publish.ClusterStatePublisher singleton method is not used. --- .../org/apache/doris/clone/ColocateTableBalancer.java | 8 ++++++-- .../java/org/apache/doris/common/proc/ProcService.java | 8 ++++++-- .../doris/common/publish/ClusterStatePublisher.java | 8 ++++++-- .../java/org/apache/doris/common/util/ProfileManager.java | 8 ++++++-- .../src/main/java/org/apache/doris/qe/HelpModule.java | 6 +++++- .../java/org/apache/doris/rpc/BackendServiceProxy.java | 8 ++++++-- .../main/java/org/apache/doris/service/ExecuteEnv.java | 2 +- 7 files changed, 36 insertions(+), 12 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/clone/ColocateTableBalancer.java b/fe/fe-core/src/main/java/org/apache/doris/clone/ColocateTableBalancer.java index 5f7956d9de..d317772b9b 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/clone/ColocateTableBalancer.java +++ b/fe/fe-core/src/main/java/org/apache/doris/clone/ColocateTableBalancer.java @@ -62,10 +62,14 @@ public class ColocateTableBalancer extends MasterDaemon { super("colocate group clone checker", intervalMs); } - private static ColocateTableBalancer INSTANCE = null; + private static volatile ColocateTableBalancer INSTANCE = null; public static ColocateTableBalancer getInstance() { if (INSTANCE == null) { - INSTANCE = new ColocateTableBalancer(CHECK_INTERVAL_MS); + synchronized (ColocateTableBalancer.class) { + if (INSTANCE == null) { + INSTANCE = new ColocateTableBalancer(CHECK_INTERVAL_MS); + } + } } return INSTANCE; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/common/proc/ProcService.java b/fe/fe-core/src/main/java/org/apache/doris/common/proc/ProcService.java index 6b0e60f92f..c93b137917 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/common/proc/ProcService.java +++ b/fe/fe-core/src/main/java/org/apache/doris/common/proc/ProcService.java @@ -28,7 +28,7 @@ import org.apache.logging.log4j.Logger; // proc service entry public final class ProcService { private static final Logger LOG = LogManager.getLogger(ProcService.class); - private static ProcService INSTANCE; + private static volatile ProcService INSTANCE; private BaseProcDir root; @@ -154,7 +154,11 @@ public final class ProcService { public static ProcService getInstance() { if (INSTANCE == null) { - INSTANCE = new ProcService(); + synchronized (ProcService.class) { + if (INSTANCE == null) { + INSTANCE = new ProcService(); + } + } } return INSTANCE; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/common/publish/ClusterStatePublisher.java b/fe/fe-core/src/main/java/org/apache/doris/common/publish/ClusterStatePublisher.java index 698c59c419..ff7e225937 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/common/publish/ClusterStatePublisher.java +++ b/fe/fe-core/src/main/java/org/apache/doris/common/publish/ClusterStatePublisher.java @@ -39,7 +39,7 @@ import java.util.concurrent.ExecutorService; // This class intend to publish the state of cluster to backends. public class ClusterStatePublisher { private static final Logger LOG = LogManager.getLogger(ClusterStatePublisher.class); - private static ClusterStatePublisher INSTANCE; + private static volatile ClusterStatePublisher INSTANCE; private ExecutorService executor = ThreadPoolManager.newDaemonFixedThreadPool(5, 256, "cluster-state-publisher", true); @@ -52,7 +52,11 @@ public class ClusterStatePublisher { public static ClusterStatePublisher getInstance() { if (INSTANCE == null) { - INSTANCE = new ClusterStatePublisher(Catalog.getCurrentSystemInfo()); + synchronized (ClusterStatePublisher.class) { + if (INSTANCE == null) { + INSTANCE = new ClusterStatePublisher(Catalog.getCurrentSystemInfo()); + } + } } return INSTANCE; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/common/util/ProfileManager.java b/fe/fe-core/src/main/java/org/apache/doris/common/util/ProfileManager.java index decb1646f2..e084a19cd3 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/common/util/ProfileManager.java +++ b/fe/fe-core/src/main/java/org/apache/doris/common/util/ProfileManager.java @@ -47,7 +47,7 @@ import java.util.concurrent.locks.ReentrantReadWriteLock.WriteLock; */ public class ProfileManager { private static final Logger LOG = LogManager.getLogger(ProfileManager.class); - private static ProfileManager INSTANCE = null; + private static volatile ProfileManager INSTANCE = null; private static final int ARRAY_SIZE = 100; // private static final int TOTAL_LEN = 1000 * ARRAY_SIZE ; public static final String QUERY_ID = "Query ID"; @@ -80,7 +80,11 @@ public class ProfileManager { public static ProfileManager getInstance() { if (INSTANCE == null) { - INSTANCE = new ProfileManager(); + synchronized (ProfileManager.class) { + if (INSTANCE == null) { + INSTANCE = new ProfileManager(); + } + } } return INSTANCE; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/qe/HelpModule.java b/fe/fe-core/src/main/java/org/apache/doris/qe/HelpModule.java index b1eff9d1ec..e90909008d 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/qe/HelpModule.java +++ b/fe/fe-core/src/main/java/org/apache/doris/qe/HelpModule.java @@ -301,7 +301,11 @@ public class HelpModule { // whether need reload ZipFile public static HelpModule getInstance() { if (instance == null) { - instance = new HelpModule(); + synchronized (HelpModule.class) { + if (instance == null) { + instance = new HelpModule(); + } + } } try { diff --git a/fe/fe-core/src/main/java/org/apache/doris/rpc/BackendServiceProxy.java b/fe/fe-core/src/main/java/org/apache/doris/rpc/BackendServiceProxy.java index 12394f6329..5d2c3afe69 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/rpc/BackendServiceProxy.java +++ b/fe/fe-core/src/main/java/org/apache/doris/rpc/BackendServiceProxy.java @@ -59,7 +59,7 @@ public class BackendServiceProxy { // TODO(zc): use TNetworkAddress, private Map serviceMap; - private static BackendServiceProxy INSTANCE; + private static volatile BackendServiceProxy INSTANCE; static { int javaRuntimeVersion = JdkUtils.getJavaVersionAsInteger(System.getProperty("java.version")); @@ -76,7 +76,11 @@ public class BackendServiceProxy { public static BackendServiceProxy getInstance() { if (INSTANCE == null) { - INSTANCE = new BackendServiceProxy(); + synchronized (BackendServiceProxy.class) { + if (INSTANCE == null) { + INSTANCE = new BackendServiceProxy(); + } + } } return INSTANCE; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/service/ExecuteEnv.java b/fe/fe-core/src/main/java/org/apache/doris/service/ExecuteEnv.java index 544cb7d7a5..a7ac522b5b 100755 --- a/fe/fe-core/src/main/java/org/apache/doris/service/ExecuteEnv.java +++ b/fe/fe-core/src/main/java/org/apache/doris/service/ExecuteEnv.java @@ -23,7 +23,7 @@ import org.apache.doris.qe.MultiLoadMgr; // Execute environment, used to save other module, need to singleton public class ExecuteEnv { - private volatile static ExecuteEnv INSTANCE; + private static volatile ExecuteEnv INSTANCE; private MultiLoadMgr multiLoadMgr; private ConnectScheduler scheduler;