From commits-return-94732-archive-asf-public=cust-asf.ponee.io@hbase.apache.org Sun Aug 2 06:19:06 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 E17FF180644 for ; Sun, 2 Aug 2020 08:19:05 +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 1805C1237C1 for ; Sun, 2 Aug 2020 06:19:05 +0000 (UTC) Received: (qmail 99458 invoked by uid 500); 2 Aug 2020 06:19:04 -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 99449 invoked by uid 99); 2 Aug 2020 06:19:04 -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; Sun, 02 Aug 2020 06:19:04 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id 03FA18242E; Sun, 2 Aug 2020 06:19:03 +0000 (UTC) Date: Sun, 02 Aug 2020 06:19:02 +0000 To: "commits@hbase.apache.org" Subject: [hbase] branch branch-2 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: <159634914194.31421.18154327497040206765@gitbox.apache.org> From: busbey@apache.org X-Git-Host: gitbox.apache.org X-Git-Repo: hbase X-Git-Refname: refs/heads/branch-2 X-Git-Reftype: branch X-Git-Oldrev: 86fccba0d0c1671ebf71259a3eed3abc43359714 X-Git-Newrev: 0806349adab338330428c900588234d7f6fcfcc2 X-Git-Rev: 0806349adab338330428c900588234d7f6fcfcc2 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-2 in repository https://gitbox.apache.org/repos/asf/hbase.git The following commit(s) were added to refs/heads/branch-2 by this push: new 0806349 HBASE-24805 HBaseTestingUtility.getConnection should be threadsafe 0806349 is described below commit 0806349adab338330428c900588234d7f6fcfcc2 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 #2180 Signed-off-by: Wellington Chevreuil Signed-off-by: Viraj Jasani (cherry picked from commit 86ebbdd8a2df89de37c2c3bd50e64292eaf28b11) --- .../hadoop/hbase/HBaseCommonTestingUtility.java | 2 +- .../apache/hadoop/hbase/HBaseTestingUtility.java | 66 +++++++++++----------- 2 files changed, 34 insertions(+), 34 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 eb04cca..487c926 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 @@ -69,7 +69,7 @@ public class HBaseCommonTestingUtility { Compression.Algorithm.NONE, Compression.Algorithm.GZ }; - 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 b73036e..a01f6a3 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 @@ -25,6 +25,7 @@ import edu.umd.cs.findbugs.annotations.Nullable; import java.io.File; import java.io.IOException; import java.io.OutputStream; +import java.io.UncheckedIOException; import java.lang.reflect.Field; import java.lang.reflect.Modifier; import java.net.BindException; @@ -209,10 +210,7 @@ public class HBaseTestingUtility extends HBaseZKTestingUtility { * HBaseTestingUtility*/ private Path dataTestDirOnTestFS = null; - /** - * Shared cluster connection. - */ - private volatile Connection connection; + private final AtomicReference connection = new AtomicReference<>(); /** Filesystem URI used for map-reduce mini-cluster setup */ private static String FS_URI; @@ -1300,14 +1298,7 @@ public class HBaseTestingUtility extends HBaseZKTestingUtility { public void restartHBaseCluster(StartMiniClusterOption option) throws IOException, InterruptedException { - if (hbaseAdmin != null) { - hbaseAdmin.close(); - hbaseAdmin = null; - } - if (this.connection != null) { - this.connection.close(); - this.connection = null; - } + closeConnection(); this.hbaseCluster = new MiniHBaseCluster(this.conf, option.getNumMasters(), option.getNumAlwaysStandByMasters(), option.getNumRegionServers(), option.getRsPorts(), option.getMasterClass(), @@ -1389,14 +1380,7 @@ public class HBaseTestingUtility extends HBaseZKTestingUtility { // close hbase admin, close current connection and reset MIN MAX configs for RS. private void cleanup() throws IOException { - if (hbaseAdmin != null) { - hbaseAdmin.close(); - hbaseAdmin = null; - } - if (this.connection != null) { - this.connection.close(); - this.connection = null; - } + closeConnection(); // unset the configuration for MIN and MAX RS to start conf.setInt(ServerManager.WAIT_ON_REGIONSERVERS_MINTOSTART, -1); conf.setInt(ServerManager.WAIT_ON_REGIONSERVERS_MAXTOSTART, -1); @@ -3143,13 +3127,6 @@ public class HBaseTestingUtility extends HBaseZKTestingUtility { return hbaseCluster; } - public void closeConnection() throws IOException { - Closeables.close(hbaseAdmin, true); - Closeables.close(connection, true); - this.hbaseAdmin = null; - this.connection = null; - } - /** * Resets the connections so that the next time getConnection() is called, a new connection is * created. This is needed in cases where the entire cluster / all the masters are shutdown and @@ -3171,14 +3148,26 @@ public class HBaseTestingUtility extends HBaseZKTestingUtility { } /** + * 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); + try { + return this.connection.updateAndGet(connection -> { + if (connection == null) { + try { + connection = ConnectionFactory.createConnection(this.conf); + } catch(IOException ioe) { + throw new UncheckedIOException("Failed to create connection", ioe); + } + } + return connection; + }); + } catch (UncheckedIOException exception) { + throw exception.getCause(); } - return this.connection; } /** @@ -3200,6 +3189,17 @@ public class HBaseTestingUtility extends HBaseZKTestingUtility { return hbaseAdmin; } + public void closeConnection() throws IOException { + if (hbaseAdmin != null) { + Closeables.close(hbaseAdmin, true); + hbaseAdmin = null; + } + Connection connection = this.connection.getAndSet(null); + if (connection != null) { + Closeables.close(connection, true); + } + } + /** * Returns an Admin instance which is shared between HBaseTestingUtility instance users. * Closing it has no effect, it will be closed automatically when the cluster shutdowns @@ -3363,7 +3363,7 @@ public class HBaseTestingUtility extends HBaseZKTestingUtility { getHBaseCluster().getMaster().getAssignmentManager().getRegionStates() .getRegionAssignments(); final List> metaLocations = - MetaTableAccessor.getTableRegionsAndLocations(connection, tableName); + MetaTableAccessor.getTableRegionsAndLocations(getConnection(), tableName); for (Pair metaLocation : metaLocations) { RegionInfo hri = metaLocation.getFirst(); ServerName sn = metaLocation.getSecond(); @@ -3386,7 +3386,7 @@ public class HBaseTestingUtility extends HBaseZKTestingUtility { public String explainTableState(final TableName table, TableState.State state) throws IOException { - TableState tableState = MetaTableAccessor.getTableState(connection, table); + TableState tableState = MetaTableAccessor.getTableState(getConnection(), table); if (tableState == null) { return "TableState in META: No table state in META for table " + table + " last state in meta (including deleted is " + findLastTableState(table) + ")"; @@ -3412,7 +3412,7 @@ public class HBaseTestingUtility extends HBaseZKTestingUtility { } }; MetaTableAccessor - .scanMeta(connection, null, null, + .scanMeta(getConnection(), null, null, MetaTableAccessor.QueryType.TABLE, Integer.MAX_VALUE, visitor); return lastTableState.get();