hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bus...@apache.org
Subject [hbase] branch branch-1 updated: HBASE-24805 HBaseTestingUtility.getConnection should be threadsafe
Date Tue, 04 Aug 2020 16:16:55 GMT
This is an automated email from the ASF dual-hosted git repository.

busbey pushed a commit to branch branch-1
in repository https://gitbox.apache.org/repos/asf/hbase.git


The following commit(s) were added to refs/heads/branch-1 by this push:
     new af18670  HBASE-24805 HBaseTestingUtility.getConnection should be threadsafe
af18670 is described below

commit af18670665ee84ded074ee11ae010ff203376d66
Author: Sean Busbey <busbey@apache.org>
AuthorDate: Fri Jul 31 02:34:24 2020 -0500

    HBASE-24805 HBaseTestingUtility.getConnection should be threadsafe
    
    * refactor how we use connection to rely on the access method
    * refactor initialization and cleanup of the shared connection
    * incompatibly change HCTU's Configuration member variable to be final so it can be safely
accessed from multiple threads.
    
    Closes #2188
    
    adapted for jdk7
    
    Signed-off-by: Viraj Jasani <vjasani@apache.org>
    (cherry picked from commit 86ebbdd8a2df89de37c2c3bd50e64292eaf28b11)
    (cherry picked from commit 0806349adab338330428c900588234d7f6fcfcc2)
---
 .../hadoop/hbase/HBaseCommonTestingUtility.java    |  2 +-
 .../apache/hadoop/hbase/HBaseTestingUtility.java   | 56 +++++++++++++++-------
 2 files changed, 39 insertions(+), 19 deletions(-)

diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/HBaseCommonTestingUtility.java
b/hbase-common/src/test/java/org/apache/hadoop/hbase/HBaseCommonTestingUtility.java
index 19a9ac2..1871d11 100644
--- a/hbase-common/src/test/java/org/apache/hadoop/hbase/HBaseCommonTestingUtility.java
+++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/HBaseCommonTestingUtility.java
@@ -40,7 +40,7 @@ import org.apache.hadoop.fs.Path;
 public class HBaseCommonTestingUtility {
   protected static final Log LOG = LogFactory.getLog(HBaseCommonTestingUtility.class);
 
-  protected Configuration conf;
+  protected final Configuration conf;
 
   public HBaseCommonTestingUtility() {
     this(null);
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java
index 90ed49c..3335b49 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java
@@ -46,6 +46,7 @@ import java.util.Random;
 import java.util.Set;
 import java.util.TreeSet;
 import java.util.UUID;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.concurrent.TimeUnit;
 
 import org.apache.commons.io.FileUtils;
@@ -190,10 +191,7 @@ public class HBaseTestingUtility extends HBaseCommonTestingUtility {
     * HBaseTestingUtility*/
   private Path dataTestDirOnTestFS = null;
 
-  /**
-   * Shared cluster connection.
-   */
-  private volatile Connection connection;
+  private final AtomicReference<Connection> connectionRef = new AtomicReference<>();
 
   /**
    * System property key to get test directory value.
@@ -1170,10 +1168,6 @@ public class HBaseTestingUtility extends HBaseCommonTestingUtility
{
    */
   public void shutdownMiniCluster() throws Exception {
     LOG.info("Shutting down minicluster");
-    if (this.connection != null && !this.connection.isClosed()) {
-      this.connection.close();
-      this.connection = null;
-    }
     shutdownMiniHBaseCluster();
     if (!this.passedZkCluster){
       shutdownMiniZKCluster();
@@ -1203,10 +1197,7 @@ public class HBaseTestingUtility extends HBaseCommonTestingUtility
{
    * @throws IOException
    */
   public void shutdownMiniHBaseCluster() throws IOException {
-    if (hbaseAdmin != null) {
-      hbaseAdmin.close0();
-      hbaseAdmin = null;
-    }
+    closeConnection();
 
     // unset the configuration for MIN and MAX RS to start
     conf.setInt(ServerManager.WAIT_ON_REGIONSERVERS_MINTOSTART, -1);
@@ -3020,16 +3011,26 @@ public class HBaseTestingUtility extends HBaseCommonTestingUtility
{
   }
 
   /**
-   * Get a Connection to the cluster.
-   * Not thread-safe (This class needs a lot of work to make it thread-safe).
+   * Get a shared Connection to the cluster.
+   * this method is threadsafe.
    * @return A Connection that can be shared. Don't close. Will be closed on shutdown of
cluster.
    * @throws IOException
    */
   public Connection getConnection() throws IOException {
-    if (this.connection == null) {
-      this.connection = ConnectionFactory.createConnection(this.conf);
+    Connection connection = this.connectionRef.get();
+    while (connection == null) {
+      connection = ConnectionFactory.createConnection(this.conf);
+      if (! this.connectionRef.compareAndSet(null, connection)) {
+        try {
+          connection.close();
+        } catch (IOException exception) {
+          LOG.debug("Ignored failure while closing connection on contended connection creation.",
+              exception);
+        }
+        connection = this.connectionRef.get();
+      }
     }
-    return this.connection;
+    return connection;
   }
 
   /**
@@ -3067,6 +3068,25 @@ public class HBaseTestingUtility extends HBaseCommonTestingUtility
{
     }
   }
 
+  public void closeConnection() throws IOException {
+    if (hbaseAdmin != null) {
+      try {
+        hbaseAdmin.close0();
+      } catch (IOException exception) {
+        LOG.debug("Ignored failure while closing admin.", exception);
+      }
+      hbaseAdmin = null;
+    }
+    Connection connection = this.connectionRef.getAndSet(null);
+    if (connection != null) {
+      try {
+        connection.close();
+      } catch (IOException exception) {
+        LOG.debug("Ignored failure while closing connection.", exception);
+      }
+    }
+  }
+
   /**
    * Returns a ZooKeeperWatcher instance.
    * This instance is shared between HBaseTestingUtility instance users.
@@ -3240,7 +3260,7 @@ public class HBaseTestingUtility extends HBaseCommonTestingUtility {
               .getRegionAssignments();
       final List<Pair<HRegionInfo, ServerName>> metaLocations =
           MetaTableAccessor
-              .getTableRegionsAndLocations(getZooKeeperWatcher(), connection, tableName);
+              .getTableRegionsAndLocations(getZooKeeperWatcher(), getConnection(), tableName);
       for (Pair<HRegionInfo, ServerName> metaLocation : metaLocations) {
         HRegionInfo hri = metaLocation.getFirst();
         ServerName sn = metaLocation.getSecond();


Mime
View raw message