From commits-return-94763-archive-asf-public=cust-asf.ponee.io@hbase.apache.org Tue Aug 4 16:16:58 2020 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mailroute1-lw-us.apache.org (mailroute1-lw-us.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with ESMTPS id 0196A18057A for ; Tue, 4 Aug 2020 18:16:58 +0200 (CEST) Received: from mail.apache.org (localhost [127.0.0.1]) by mailroute1-lw-us.apache.org (ASF Mail Server at mailroute1-lw-us.apache.org) with SMTP id 2A7A3124B5D for ; Tue, 4 Aug 2020 16:16:57 +0000 (UTC) Received: (qmail 6382 invoked by uid 500); 4 Aug 2020 16:16:56 -0000 Mailing-List: contact commits-help@hbase.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@hbase.apache.org Delivered-To: mailing list commits@hbase.apache.org Received: (qmail 6373 invoked by uid 99); 4 Aug 2020 16:16:56 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 04 Aug 2020 16:16:56 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id 576828242E; Tue, 4 Aug 2020 16:16:56 +0000 (UTC) Date: Tue, 04 Aug 2020 16:16:55 +0000 To: "commits@hbase.apache.org" Subject: [hbase] branch branch-1 updated: HBASE-24805 HBaseTestingUtility.getConnection should be threadsafe MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Message-ID: <159655781454.13838.13010395550779856149@gitbox.apache.org> From: busbey@apache.org X-Git-Host: gitbox.apache.org X-Git-Repo: hbase X-Git-Refname: refs/heads/branch-1 X-Git-Reftype: branch X-Git-Oldrev: 1f0abf8279df15bf259c229300387a432e5a725c X-Git-Newrev: af18670665ee84ded074ee11ae010ff203376d66 X-Git-Rev: af18670665ee84ded074ee11ae010ff203376d66 X-Git-NotificationType: ref_changed_plus_diff X-Git-Multimail-Version: 1.5.dev Auto-Submitted: auto-generated 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 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 (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 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> metaLocations = MetaTableAccessor - .getTableRegionsAndLocations(getZooKeeperWatcher(), connection, tableName); + .getTableRegionsAndLocations(getZooKeeperWatcher(), getConnection(), tableName); for (Pair metaLocation : metaLocations) { HRegionInfo hri = metaLocation.getFirst(); ServerName sn = metaLocation.getSecond();