HIVE-19631 : reduce epic locking in AbstractService (Sergey Shelukhin, reviewed by Thejas M Nair)
Project: http://git-wip-us.apache.org/repos/asf/hive/repo Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/fa30fe4b Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/fa30fe4b Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/fa30fe4b Branch: refs/heads/branch-3 Commit: fa30fe4b1ca63c7f0579b9d33e0a00588c53b38d Parents: eaeb200 Author: sergey <ser...@apache.org> Authored: Thu May 24 14:32:00 2018 -0700 Committer: sergey <ser...@apache.org> Committed: Thu May 24 14:32:18 2018 -0700 ---------------------------------------------------------------------- .../apache/hive/service/AbstractService.java | 23 ++++++++++++++------ .../org/apache/hive/service/cli/CLIService.java | 5 ++++- .../apache/hive/service/server/HiveServer2.java | 12 +++++----- 3 files changed, 27 insertions(+), 13 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hive/blob/fa30fe4b/service/src/java/org/apache/hive/service/AbstractService.java ---------------------------------------------------------------------- diff --git a/service/src/java/org/apache/hive/service/AbstractService.java b/service/src/java/org/apache/hive/service/AbstractService.java index 37a9f4c..2ddb069 100644 --- a/service/src/java/org/apache/hive/service/AbstractService.java +++ b/service/src/java/org/apache/hive/service/AbstractService.java @@ -50,7 +50,7 @@ public abstract class AbstractService implements Service { /** * The configuration. Will be null until the service is initialized. */ - protected HiveConf hiveConf; + private HiveConf hiveConf; /** * List of state change listeners; it is final to ensure @@ -69,6 +69,7 @@ public abstract class AbstractService implements Service { this.name = name; } + // This probably doesn't need to be sync, but nobody calls this, so it doesn't matter. @Override public synchronized STATE getServiceState() { return state; @@ -84,11 +85,15 @@ public abstract class AbstractService implements Service { @Override public synchronized void init(HiveConf hiveConf) { ensureCurrentState(STATE.NOTINITED); - this.hiveConf = hiveConf; + setHiveConf(hiveConf); changeState(STATE.INITED); LOG.info("Service:" + getName() + " is inited."); } + protected final void setHiveConf(HiveConf hiveConf) { + this.hiveConf = hiveConf; + } + /** * {@inheritDoc} * @@ -126,13 +131,17 @@ public abstract class AbstractService implements Service { } @Override - public synchronized void register(ServiceStateChangeListener l) { - listeners.add(l); + public void register(ServiceStateChangeListener l) { + synchronized (listeners) { + listeners.add(l); + } } @Override - public synchronized void unregister(ServiceStateChangeListener l) { - listeners.remove(l); + public void unregister(ServiceStateChangeListener l) { + synchronized (listeners) { + listeners.remove(l); + } } @Override @@ -141,7 +150,7 @@ public abstract class AbstractService implements Service { } @Override - public synchronized HiveConf getHiveConf() { + public HiveConf getHiveConf() { return hiveConf; } http://git-wip-us.apache.org/repos/asf/hive/blob/fa30fe4b/service/src/java/org/apache/hive/service/cli/CLIService.java ---------------------------------------------------------------------- diff --git a/service/src/java/org/apache/hive/service/cli/CLIService.java b/service/src/java/org/apache/hive/service/cli/CLIService.java index c9914ba..3e26197 100644 --- a/service/src/java/org/apache/hive/service/cli/CLIService.java +++ b/service/src/java/org/apache/hive/service/cli/CLIService.java @@ -80,7 +80,7 @@ public class CLIService extends CompositeService implements ICLIService { @Override public synchronized void init(HiveConf hiveConf) { - this.hiveConf = hiveConf; + setHiveConf(hiveConf); sessionManager = new SessionManager(hiveServer2); defaultFetchRows = hiveConf.getIntVar(ConfVars.HIVE_SERVER2_THRIFT_RESULTSET_DEFAULT_FETCH_SIZE); addService(sessionManager); @@ -132,6 +132,7 @@ public class CLIService extends CompositeService implements ICLIService { } private void setupBlockedUdfs() { + HiveConf hiveConf = getHiveConf(); FunctionRegistry.setupPermissionsForBuiltinUDFs( hiveConf.getVar(ConfVars.HIVE_SERVER2_BUILTIN_UDF_WHITELIST), hiveConf.getVar(ConfVars.HIVE_SERVER2_BUILTIN_UDF_BLACKLIST)); @@ -563,8 +564,10 @@ public class CLIService extends CompositeService implements ICLIService { } // obtain delegation token for the give user from metastore + // TODO: why is this synchronized? public synchronized String getDelegationTokenFromMetaStore(String owner) throws HiveSQLException, UnsupportedOperationException, LoginException, IOException { + HiveConf hiveConf = getHiveConf(); if (!hiveConf.getBoolVar(HiveConf.ConfVars.METASTORE_USE_THRIFT_SASL) || !hiveConf.getBoolVar(HiveConf.ConfVars.HIVE_SERVER2_ENABLE_DOAS)) { throw new UnsupportedOperationException( http://git-wip-us.apache.org/repos/asf/hive/blob/fa30fe4b/service/src/java/org/apache/hive/service/server/HiveServer2.java ---------------------------------------------------------------------- diff --git a/service/src/java/org/apache/hive/service/server/HiveServer2.java b/service/src/java/org/apache/hive/service/server/HiveServer2.java index cf3edbf..f9116ff 100644 --- a/service/src/java/org/apache/hive/service/server/HiveServer2.java +++ b/service/src/java/org/apache/hive/service/server/HiveServer2.java @@ -770,7 +770,7 @@ public class HiveServer2 extends CompositeService { LOG.info("Started/Reconnected tez sessions."); // resolve futures used for testing - if (HiveConf.getBoolVar(hiveServer2.hiveConf, ConfVars.HIVE_IN_TEST)) { + if (HiveConf.getBoolVar(hiveServer2.getHiveConf(), ConfVars.HIVE_IN_TEST)) { hiveServer2.isLeaderTestFuture.set(true); hiveServer2.resetNotLeaderTestFuture(); } @@ -785,7 +785,7 @@ public class HiveServer2 extends CompositeService { LOG.info("Stopped/Disconnected tez sessions."); // resolve futures used for testing - if (HiveConf.getBoolVar(hiveServer2.hiveConf, ConfVars.HIVE_IN_TEST)) { + if (HiveConf.getBoolVar(hiveServer2.getHiveConf(), ConfVars.HIVE_IN_TEST)) { hiveServer2.notLeaderTestFuture.set(true); hiveServer2.resetIsLeaderTestFuture(); } @@ -800,14 +800,15 @@ public class HiveServer2 extends CompositeService { try { resourcePlan = sessionHive.getActiveResourcePlan(); } catch (HiveException e) { - if (!HiveConf.getBoolVar(hiveConf, ConfVars.HIVE_IN_TEST_SSL)) { + if (!HiveConf.getBoolVar(getHiveConf(), ConfVars.HIVE_IN_TEST_SSL)) { throw new RuntimeException(e); } else { resourcePlan = null; // Ignore errors in SSL tests where the connection is misconfigured. } } - if (resourcePlan == null && HiveConf.getBoolVar(hiveConf, ConfVars.HIVE_IN_TEST)) { + if (resourcePlan == null && HiveConf.getBoolVar( + getHiveConf(), ConfVars.HIVE_IN_TEST)) { LOG.info("Creating a default resource plan for test"); resourcePlan = createTestResourcePlan(); } @@ -823,6 +824,7 @@ public class HiveServer2 extends CompositeService { // will be invoked anyway in TezTask. Doing it early to initialize triggers for non-pool tez session. LOG.info("Initializing tez session pool manager"); tezSessionPoolManager = TezSessionPoolManager.getInstance(); + HiveConf hiveConf = getHiveConf(); if (hiveConf.getBoolVar(ConfVars.HIVE_SERVER2_TEZ_INITIALIZE_DEFAULT_SESSIONS)) { tezSessionPoolManager.setupPool(hiveConf); } else { @@ -840,7 +842,7 @@ public class HiveServer2 extends CompositeService { // Initialize workload management. LOG.info("Initializing workload management"); try { - wm = WorkloadManager.create(wmQueue, hiveConf, resourcePlan); + wm = WorkloadManager.create(wmQueue, getHiveConf(), resourcePlan); wm.start(); LOG.info("Workload manager initialized."); } catch (Exception e) {