You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by se...@apache.org on 2018/05/24 21:38:07 UTC

[2/2] hive git commit: HIVE-19631 : reduce epic locking in AbstractService (Sergey Shelukhin, reviewed by Thejas M Nair)

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 <se...@apache.org>
Authored: Thu May 24 14:32:00 2018 -0700
Committer: sergey <se...@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) {