From 9a525b7090f10417ca8ffca66745630682c9046e Mon Sep 17 00:00:00 2001 From: Victor Li Date: Wed, 9 Nov 2022 17:56:51 -0800 Subject: [PATCH 01/15] HBASE-27466: Add an option for user to specify an identity for: metrics object of connections. --- .../hadoop/hbase/client/AsyncConnection.java | 10 +++ .../hbase/client/AsyncConnectionImpl.java | 26 +++++++- .../hbase/client/ConnectionFactory.java | 33 +++++++++- .../hbase/client/MetricsConnection.java | 61 ++++++++++++++++++- 4 files changed, 123 insertions(+), 7 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnection.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnection.java index 6e96918d1d9a..6ff42dd2e9ee 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnection.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnection.java @@ -34,6 +34,16 @@ @InterfaceAudience.Public public interface AsyncConnection extends Closeable { + /** + * Returns the clusterId of this connection. + */ + String getClusterId2(); + + /** + * Returns this connection identity. + */ + String getIdentity(); + /** * Returns the {@link org.apache.hadoop.conf.Configuration} object used by this instance. *

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java index 1f29b556b127..ede495d4f16c 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java @@ -118,6 +118,8 @@ public class AsyncConnectionImpl implements AsyncConnection { private final AtomicBoolean closed = new AtomicBoolean(false); + private final String clusterId; + private final String identity; private final Optional metrics; private final ClusterStatusListener clusterStatusListener; @@ -126,6 +128,13 @@ public class AsyncConnectionImpl implements AsyncConnection { public AsyncConnectionImpl(Configuration conf, ConnectionRegistry registry, String clusterId, SocketAddress localAddress, User user) { + this(conf, registry, clusterId, localAddress, user, null); + } + + public AsyncConnectionImpl(Configuration conf, ConnectionRegistry registry, String clusterId, + SocketAddress localAddress, User user, String identity) { + this.clusterId = clusterId; + this.identity = identity; this.conf = conf; this.user = user; @@ -135,8 +144,7 @@ public AsyncConnectionImpl(Configuration conf, ConnectionRegistry registry, Stri this.connConf = new AsyncConnectionConfiguration(conf); this.registry = registry; if (conf.getBoolean(CLIENT_SIDE_METRICS_ENABLED_KEY, false)) { - String scope = MetricsConnection.getScope(conf, clusterId, this); - this.metrics = Optional.of(new MetricsConnection(scope, () -> null, () -> null)); + this.metrics = Optional.of(MetricsConnection.getMetricsConnection(this, () -> null, () -> null)); } else { this.metrics = Optional.empty(); } @@ -205,6 +213,16 @@ public ConnectionRegistry getConnectionRegistry() { return registry; } + @Override + public String getClusterId2() { + return clusterId; + } + + @Override + public String getIdentity() { + return identity; + } + @Override public Configuration getConfiguration() { return conf; @@ -235,7 +253,9 @@ public void close() { choreService = null; } } - metrics.ifPresent(MetricsConnection::shutdown); + if (metrics.isPresent()) { + MetricsConnection.deleteMetricsConnection(this); + } ConnectionOverAsyncConnection c = this.conn; if (c != null) { c.closePool(); diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionFactory.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionFactory.java index 4d4559f4b7a9..ea2bf2307862 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionFactory.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionFactory.java @@ -216,6 +216,37 @@ public static Connection createConnection(Configuration conf, User user) throws */ public static Connection createConnection(Configuration conf, ExecutorService pool, final User user) throws IOException { + return createConnection(conf, pool, user, null); + } + + /** + * Create a new Connection instance using the passed conf instance. Connection + * encapsulates all housekeeping for a connection to the cluster. All tables and interfaces + * created from returned connection share zookeeper connection, meta cache, and connections to + * region servers and masters.
+ * The caller is responsible for calling {@link Connection#close()} on the returned connection + * instance. Typical usage: + * + * 

+   * Connection connection = ConnectionFactory.createConnection(conf);
+   * Table table = connection.getTable(TableName.valueOf("table1"));
+   * try {
+   *   table.get(...);
+   *   ...
+   * } finally {
+   *   table.close();
+   *   connection.close();
+   * }
+   *
+ * + * @param conf configuration + * @param user the user the connection is for + * @param pool the thread pool to use for batch operations + * @param identity the identity of the metrics for this connection. + * @return Connection object for conf + */ + public static Connection createConnection(Configuration conf, ExecutorService pool, + final User user, String identity) throws IOException { Class clazz = conf.getClass(ConnectionUtils.HBASE_CLIENT_CONNECTION_IMPL, ConnectionOverAsyncConnection.class, Connection.class); if (clazz != ConnectionOverAsyncConnection.class) { @@ -225,7 +256,7 @@ public static Connection createConnection(Configuration conf, ExecutorService po clazz.getDeclaredConstructor(Configuration.class, ExecutorService.class, User.class); constructor.setAccessible(true); return user.runAs((PrivilegedExceptionAction< - Connection>) () -> (Connection) constructor.newInstance(conf, pool, user)); + Connection>) () -> (Connection) constructor.newInstance(conf, pool, user, identity)); } catch (Exception e) { throw new IOException(e); } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java index f844c47e4065..c8a1d015b8c0 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java @@ -26,6 +26,8 @@ import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.RatioGauge; import com.codahale.metrics.Timer; +import java.util.HashMap; +import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ConcurrentSkipListMap; @@ -54,6 +56,36 @@ @InterfaceAudience.Private public class MetricsConnection implements StatisticTrackable { + static final Map METRICS_INSTANCES = + new HashMap(); + + static MetricsConnection getMetricsConnection(final AsyncConnection conn, Supplier batchPool, + Supplier metaPool) { + String scope = getScope(conn); + MetricsConnection metrics; + synchronized(METRICS_INSTANCES) { + metrics = METRICS_INSTANCES.get(scope); + if (metrics == null) { + metrics = new MetricsConnection(scope, batchPool, metaPool); + metrics.incrConnectionCount(); + METRICS_INSTANCES.put(scope, metrics); + } + } + return metrics; + } + + static void deleteMetricsConnection(final AsyncConnection conn) { + String scope = getScope(conn); + synchronized(METRICS_INSTANCES) { + MetricsConnection metrics = METRICS_INSTANCES.get(scope); + metrics.decrConnectionCount(); + if (metrics.getConnectionCount() == 0) { + METRICS_INSTANCES.remove(scope); + metrics.shutdown(); + } + } + } + /** Set this key to {@code true} to enable metrics collection of client requests. */ public static final String CLIENT_SIDE_METRICS_ENABLED_KEY = "hbase.client.metrics.enable"; @@ -78,9 +110,15 @@ public class MetricsConnection implements StatisticTrackable { * @param connectionObj either a Connection or AsyncConnectionImpl, the instance creating this * MetricsConnection. */ - static String getScope(Configuration conf, String clusterId, Object connectionObj) { - return conf.get(METRICS_SCOPE_KEY, - clusterId + "@" + Integer.toHexString(connectionObj.hashCode())); + private static String getScope(final AsyncConnection conn) { + String identity = conn.getIdentity(); + if (identity != null) { + return identity; + } + Configuration conf = conn.getConfiguration(); + String clusterId = conn.getClusterId2(); + int connHashCode = conn.hashCode(); + return conf.get(METRICS_SCOPE_KEY, clusterId + "@" + Integer.toHexString(connHashCode)); } private static final String CNT_BASE = "rpcCount_"; @@ -297,6 +335,7 @@ public Counter newMetric(Class clazz, String name, String scope) { // static metrics + protected final Counter connectionCount; protected final Counter metaCacheHits; protected final Counter metaCacheMisses; protected final CallTracker getTracker; @@ -355,6 +394,7 @@ protected Ratio getRatio() { return Ratio.of(pool.getActiveCount(), pool.getMaximumPoolSize()); } }); + this.connectionCount = registry.counter(name(this.getClass(), "connectionCount", scope)); this.metaCacheHits = registry.counter(name(this.getClass(), "metaCacheHits", scope)); this.metaCacheMisses = registry.counter(name(this.getClass(), "metaCacheMisses", scope)); this.metaCacheNumClearServer = @@ -406,6 +446,21 @@ public static CallStats newCallStats() { return new CallStats(); } + /** Return the connection count of the metrics within a scope */ + public long getConnectionCount() { + return connectionCount.getCount(); + } + + /** Increment the connection count of the metrics within a scope */ + public void incrConnectionCount() { + connectionCount.inc(); + } + + /** Decrement the connection count of the metrics within a scope */ + public void decrConnectionCount() { + connectionCount.dec(); + } + /** Increment the number of meta cache hits. */ public void incrMetaCacheHit() { metaCacheHits.inc(); From 7bbfc8144b3c27c5ac66cff8e05f3d5dd7183b80 Mon Sep 17 00:00:00 2001 From: Victor Li Date: Wed, 9 Nov 2022 21:00:08 -0800 Subject: [PATCH 02/15] Fixed compilation errors in other directories. --- .../hadoop/hbase/client/SharedAsyncConnection.java | 10 ++++++++++ .../hbase/client/DummyAsyncClusterConnection.java | 10 ++++++++++ 2 files changed, 20 insertions(+) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/client/SharedAsyncConnection.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/client/SharedAsyncConnection.java index ebffc7ee5111..cb334799276e 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/client/SharedAsyncConnection.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/client/SharedAsyncConnection.java @@ -52,6 +52,16 @@ public Configuration getConfiguration() { return conn.getConfiguration(); } + @Override + public String getClusterId2() { + return conn.getClusterId2(); + } + + @Override + public String getIdentity() { + return conn.getIdentity(); + } + @Override public AsyncTableRegionLocator getRegionLocator(TableName tableName) { return conn.getRegionLocator(tableName); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/DummyAsyncClusterConnection.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/DummyAsyncClusterConnection.java index cb54e6e72634..3a3cedea2028 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/DummyAsyncClusterConnection.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/DummyAsyncClusterConnection.java @@ -44,6 +44,16 @@ public Configuration getConfiguration() { return null; } + @Override + public String getClusterId2() { + return null; + } + + @Override + public String getIdentity() { + return null; + } + @Override public AsyncTableRegionLocator getRegionLocator(TableName tableName) { return null; From c39aec2e4ca7366d680b034c8f47148a8037d4d4 Mon Sep 17 00:00:00 2001 From: Victor Li Date: Wed, 9 Nov 2022 22:33:33 -0800 Subject: [PATCH 03/15] Fixed spotless check errors. --- .../hadoop/hbase/client/AsyncConnectionImpl.java | 3 ++- .../apache/hadoop/hbase/client/ConnectionFactory.java | 6 +++--- .../apache/hadoop/hbase/client/MetricsConnection.java | 10 +++++----- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java index ede495d4f16c..bf77dcc34114 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java @@ -144,7 +144,8 @@ public AsyncConnectionImpl(Configuration conf, ConnectionRegistry registry, Stri this.connConf = new AsyncConnectionConfiguration(conf); this.registry = registry; if (conf.getBoolean(CLIENT_SIDE_METRICS_ENABLED_KEY, false)) { - this.metrics = Optional.of(MetricsConnection.getMetricsConnection(this, () -> null, () -> null)); + this.metrics = + Optional.of(MetricsConnection.getMetricsConnection(this, () -> null, () -> null)); } else { this.metrics = Optional.empty(); } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionFactory.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionFactory.java index ea2bf2307862..07afad408fa2 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionFactory.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionFactory.java @@ -239,9 +239,9 @@ public static Connection createConnection(Configuration conf, ExecutorService po * } * * - * @param conf configuration - * @param user the user the connection is for - * @param pool the thread pool to use for batch operations + * @param conf configuration + * @param user the user the connection is for + * @param pool the thread pool to use for batch operations * @param identity the identity of the metrics for this connection. * @return Connection object for conf */ diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java index c8a1d015b8c0..024f8ebdac1a 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java @@ -59,11 +59,11 @@ public class MetricsConnection implements StatisticTrackable { static final Map METRICS_INSTANCES = new HashMap(); - static MetricsConnection getMetricsConnection(final AsyncConnection conn, Supplier batchPool, - Supplier metaPool) { + static MetricsConnection getMetricsConnection(final AsyncConnection conn, + Supplier batchPool, Supplier metaPool) { String scope = getScope(conn); MetricsConnection metrics; - synchronized(METRICS_INSTANCES) { + synchronized (METRICS_INSTANCES) { metrics = METRICS_INSTANCES.get(scope); if (metrics == null) { metrics = new MetricsConnection(scope, batchPool, metaPool); @@ -76,7 +76,7 @@ static MetricsConnection getMetricsConnection(final AsyncConnection conn, Suppli static void deleteMetricsConnection(final AsyncConnection conn) { String scope = getScope(conn); - synchronized(METRICS_INSTANCES) { + synchronized (METRICS_INSTANCES) { MetricsConnection metrics = METRICS_INSTANCES.get(scope); metrics.decrConnectionCount(); if (metrics.getConnectionCount() == 0) { @@ -111,7 +111,7 @@ static void deleteMetricsConnection(final AsyncConnection conn) { * MetricsConnection. */ private static String getScope(final AsyncConnection conn) { - String identity = conn.getIdentity(); + String identity = conn.getIdentity(); if (identity != null) { return identity; } From 9a7d4fa89e049b2b961bd87e3a685f17f2768748 Mon Sep 17 00:00:00 2001 From: Victor Li Date: Fri, 11 Nov 2022 12:26:37 -0800 Subject: [PATCH 04/15] Make the max ration of thread usage as the thread ratio metrics. --- .../hbase/client/MetricsConnection.java | 84 +++++++++++++------ 1 file changed, 60 insertions(+), 24 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java index 024f8ebdac1a..68e697cedf1e 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java @@ -26,7 +26,9 @@ import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.RatioGauge; import com.codahale.metrics.Timer; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -67,9 +69,11 @@ static MetricsConnection getMetricsConnection(final AsyncConnection conn, metrics = METRICS_INSTANCES.get(scope); if (metrics == null) { metrics = new MetricsConnection(scope, batchPool, metaPool); - metrics.incrConnectionCount(); METRICS_INSTANCES.put(scope, metrics); + } else { + metrics.addThreadPools(batchPool, metaPool); } + metrics.incrConnectionCount(); } return metrics; } @@ -333,6 +337,10 @@ public Counter newMetric(Class clazz, String name, String scope) { } }; + // List of thread pool per connection of the metrics. + private List batchPools = new ArrayList(); + private List metaPools = new ArrayList(); + // static metrics protected final Counter connectionCount; @@ -373,25 +381,47 @@ public Counter newMetric(Class clazz, String name, String scope) { MetricsConnection(String scope, Supplier batchPool, Supplier metaPool) { this.scope = scope; + this.batchPools.add(batchPool); + this.metaPools.add(metaPool); this.registry = new MetricRegistry(); this.registry.register(getExecutorPoolName(), new RatioGauge() { @Override protected Ratio getRatio() { - ThreadPoolExecutor pool = batchPool.get(); - if (pool == null) { - return Ratio.of(0, 0); + int numerator = 0; + int denominator = 0; + for (int i = 0; i < batchPools.size(); i++) { + ThreadPoolExecutor pool = (ThreadPoolExecutor)((Supplier)batchPools.get(i)).get(); + if (pool != null) { + int activeCount = pool.getActiveCount(); + int maxPoolSize = pool.getMaximumPoolSize(); + if (numerator == 0 || + (numerator * maxPoolSize) < (activeCount * denominator)) { + numerator = activeCount; + denominator = maxPoolSize; + } + } } - return Ratio.of(pool.getActiveCount(), pool.getMaximumPoolSize()); + return Ratio.of(numerator, denominator); } }); this.registry.register(getMetaPoolName(), new RatioGauge() { @Override protected Ratio getRatio() { - ThreadPoolExecutor pool = metaPool.get(); - if (pool == null) { - return Ratio.of(0, 0); + int numerator = 0; + int denominator = 0; + for (int i = 0; i < metaPools.size(); i++) { + ThreadPoolExecutor pool = (ThreadPoolExecutor)((Supplier)metaPools.get(i)).get(); + if (pool != null) { + int activeCount = pool.getActiveCount(); + int maxPoolSize = pool.getMaximumPoolSize(); + if (numerator == 0 || + (numerator * maxPoolSize) < (activeCount * denominator)) { + numerator = activeCount; + denominator = maxPoolSize; + } + } } - return Ratio.of(pool.getActiveCount(), pool.getMaximumPoolSize()); + return Ratio.of(numerator, denominator); } }); this.connectionCount = registry.counter(name(this.getClass(), "connectionCount", scope)); @@ -446,21 +476,6 @@ public static CallStats newCallStats() { return new CallStats(); } - /** Return the connection count of the metrics within a scope */ - public long getConnectionCount() { - return connectionCount.getCount(); - } - - /** Increment the connection count of the metrics within a scope */ - public void incrConnectionCount() { - connectionCount.inc(); - } - - /** Decrement the connection count of the metrics within a scope */ - public void decrConnectionCount() { - connectionCount.dec(); - } - /** Increment the number of meta cache hits. */ public void incrMetaCacheHit() { metaCacheHits.inc(); @@ -512,6 +527,27 @@ public void incrementServerOverloadedBackoffTime(long time, TimeUnit timeUnit) { overloadedBackoffTimer.update(time, timeUnit); } + /** Return the connection count of the metrics within a scope */ + private long getConnectionCount() { + return connectionCount.getCount(); + } + + /** Increment the connection count of the metrics within a scope */ + private void incrConnectionCount() { + connectionCount.inc(); + } + + /** Decrement the connection count of the metrics within a scope */ + private void decrConnectionCount() { + connectionCount.dec(); + } + + /** Add thread pools of additional connections to the metrics */ + private void addThreadPools(Supplier batchPool, Supplier metaPool) { + batchPools.add(batchPool); + metaPools.add(metaPool); + } + /** * Get a metric for {@code key} from {@code map}, or create it with {@code factory}. */ From ed9b1e38b3245a3e09c31ae718273c96bc26369b Mon Sep 17 00:00:00 2001 From: Victor Li Date: Fri, 11 Nov 2022 12:30:53 -0800 Subject: [PATCH 05/15] Fixed spotless check. --- .../apache/hadoop/hbase/client/MetricsConnection.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java index 68e697cedf1e..c39f99cd20b0 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java @@ -390,12 +390,11 @@ protected Ratio getRatio() { int numerator = 0; int denominator = 0; for (int i = 0; i < batchPools.size(); i++) { - ThreadPoolExecutor pool = (ThreadPoolExecutor)((Supplier)batchPools.get(i)).get(); + ThreadPoolExecutor pool = (ThreadPoolExecutor) ((Supplier) batchPools.get(i)).get(); if (pool != null) { int activeCount = pool.getActiveCount(); int maxPoolSize = pool.getMaximumPoolSize(); - if (numerator == 0 || - (numerator * maxPoolSize) < (activeCount * denominator)) { + if (numerator == 0 || (numerator * maxPoolSize) < (activeCount * denominator)) { numerator = activeCount; denominator = maxPoolSize; } @@ -410,12 +409,11 @@ protected Ratio getRatio() { int numerator = 0; int denominator = 0; for (int i = 0; i < metaPools.size(); i++) { - ThreadPoolExecutor pool = (ThreadPoolExecutor)((Supplier)metaPools.get(i)).get(); + ThreadPoolExecutor pool = (ThreadPoolExecutor) ((Supplier) metaPools.get(i)).get(); if (pool != null) { int activeCount = pool.getActiveCount(); int maxPoolSize = pool.getMaximumPoolSize(); - if (numerator == 0 || - (numerator * maxPoolSize) < (activeCount * denominator)) { + if (numerator == 0 || (numerator * maxPoolSize) < (activeCount * denominator)) { numerator = activeCount; denominator = maxPoolSize; } From b51866cd91b3ae13ea84779d39dc3e8296b735f0 Mon Sep 17 00:00:00 2001 From: Victor Li Date: Fri, 11 Nov 2022 16:46:18 -0800 Subject: [PATCH 06/15] Addressed review comments. --- .../hadoop/hbase/client/AsyncConnection.java | 4 +-- .../hbase/client/AsyncConnectionImpl.java | 12 +++---- .../hbase/client/ConnectionFactory.java | 33 +------------------ .../hbase/client/MetricsConnection.java | 23 +++++++------ .../hbase/client/SharedAsyncConnection.java | 8 ++--- .../client/DummyAsyncClusterConnection.java | 4 +-- 6 files changed, 28 insertions(+), 56 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnection.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnection.java index 6ff42dd2e9ee..f85cce368c67 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnection.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnection.java @@ -37,12 +37,12 @@ public interface AsyncConnection extends Closeable { /** * Returns the clusterId of this connection. */ - String getClusterId2(); + String getClusterIdentity(); /** * Returns this connection identity. */ - String getIdentity(); + String getConnectionScope(); /** * Returns the {@link org.apache.hadoop.conf.Configuration} object used by this instance. diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java index bf77dcc34114..afd47842676b 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java @@ -119,7 +119,7 @@ public class AsyncConnectionImpl implements AsyncConnection { private final AtomicBoolean closed = new AtomicBoolean(false); private final String clusterId; - private final String identity; + private final String connScope; private final Optional metrics; private final ClusterStatusListener clusterStatusListener; @@ -132,9 +132,9 @@ public AsyncConnectionImpl(Configuration conf, ConnectionRegistry registry, Stri } public AsyncConnectionImpl(Configuration conf, ConnectionRegistry registry, String clusterId, - SocketAddress localAddress, User user, String identity) { + SocketAddress localAddress, User user, String connScope) { this.clusterId = clusterId; - this.identity = identity; + this.connScope = connScope; this.conf = conf; this.user = user; @@ -215,13 +215,13 @@ public ConnectionRegistry getConnectionRegistry() { } @Override - public String getClusterId2() { + public String getClusterIdentity() { return clusterId; } @Override - public String getIdentity() { - return identity; + public String getConnectionScope() { + return connScope; } @Override diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionFactory.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionFactory.java index 07afad408fa2..4d4559f4b7a9 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionFactory.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionFactory.java @@ -216,37 +216,6 @@ public static Connection createConnection(Configuration conf, User user) throws */ public static Connection createConnection(Configuration conf, ExecutorService pool, final User user) throws IOException { - return createConnection(conf, pool, user, null); - } - - /** - * Create a new Connection instance using the passed conf instance. Connection - * encapsulates all housekeeping for a connection to the cluster. All tables and interfaces - * created from returned connection share zookeeper connection, meta cache, and connections to - * region servers and masters.
- * The caller is responsible for calling {@link Connection#close()} on the returned connection - * instance. Typical usage: - * - * 
-   * Connection connection = ConnectionFactory.createConnection(conf);
-   * Table table = connection.getTable(TableName.valueOf("table1"));
-   * try {
-   *   table.get(...);
-   *   ...
-   * } finally {
-   *   table.close();
-   *   connection.close();
-   * }
-   *
- * - * @param conf configuration - * @param user the user the connection is for - * @param pool the thread pool to use for batch operations - * @param identity the identity of the metrics for this connection. - * @return Connection object for conf - */ - public static Connection createConnection(Configuration conf, ExecutorService pool, - final User user, String identity) throws IOException { Class clazz = conf.getClass(ConnectionUtils.HBASE_CLIENT_CONNECTION_IMPL, ConnectionOverAsyncConnection.class, Connection.class); if (clazz != ConnectionOverAsyncConnection.class) { @@ -256,7 +225,7 @@ public static Connection createConnection(Configuration conf, ExecutorService po clazz.getDeclaredConstructor(Configuration.class, ExecutorService.class, User.class); constructor.setAccessible(true); return user.runAs((PrivilegedExceptionAction< - Connection>) () -> (Connection) constructor.newInstance(conf, pool, user, identity)); + Connection>) () -> (Connection) constructor.newInstance(conf, pool, user)); } catch (Exception e) { throw new IOException(e); } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java index c39f99cd20b0..bf62d241cb15 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java @@ -82,6 +82,7 @@ static void deleteMetricsConnection(final AsyncConnection conn) { String scope = getScope(conn); synchronized (METRICS_INSTANCES) { MetricsConnection metrics = METRICS_INSTANCES.get(scope); + if (metrics == null) return; metrics.decrConnectionCount(); if (metrics.getConnectionCount() == 0) { METRICS_INSTANCES.remove(scope); @@ -115,12 +116,12 @@ static void deleteMetricsConnection(final AsyncConnection conn) { * MetricsConnection. */ private static String getScope(final AsyncConnection conn) { - String identity = conn.getIdentity(); - if (identity != null) { - return identity; + String scope = conn.getConnectionScope(); + if (scope != null) { + return scope; } Configuration conf = conn.getConfiguration(); - String clusterId = conn.getClusterId2(); + String clusterId = conn.getClusterIdentity(); int connHashCode = conn.hashCode(); return conf.get(METRICS_SCOPE_KEY, clusterId + "@" + Integer.toHexString(connHashCode)); } @@ -338,8 +339,8 @@ public Counter newMetric(Class clazz, String name, String scope) { }; // List of thread pool per connection of the metrics. - private List batchPools = new ArrayList(); - private List metaPools = new ArrayList(); + private List> batchPools = new ArrayList<>(); + private List> metaPools = new ArrayList<>(); // static metrics @@ -389,11 +390,12 @@ public Counter newMetric(Class clazz, String name, String scope) { protected Ratio getRatio() { int numerator = 0; int denominator = 0; - for (int i = 0; i < batchPools.size(); i++) { - ThreadPoolExecutor pool = (ThreadPoolExecutor) ((Supplier) batchPools.get(i)).get(); + for (Supplier poolSupplier : batchPools) { + ThreadPoolExecutor pool = poolSupplier.get(); if (pool != null) { int activeCount = pool.getActiveCount(); int maxPoolSize = pool.getMaximumPoolSize(); + /* The max thread usage ratio among batch pools of all connections */ if (numerator == 0 || (numerator * maxPoolSize) < (activeCount * denominator)) { numerator = activeCount; denominator = maxPoolSize; @@ -408,11 +410,12 @@ protected Ratio getRatio() { protected Ratio getRatio() { int numerator = 0; int denominator = 0; - for (int i = 0; i < metaPools.size(); i++) { - ThreadPoolExecutor pool = (ThreadPoolExecutor) ((Supplier) metaPools.get(i)).get(); + for (Supplier poolSupplier : metaPools) { + ThreadPoolExecutor pool = poolSupplier.get(); if (pool != null) { int activeCount = pool.getActiveCount(); int maxPoolSize = pool.getMaximumPoolSize(); + /* The max thread usage ratio among meta lookup pools of all connections */ if (numerator == 0 || (numerator * maxPoolSize) < (activeCount * denominator)) { numerator = activeCount; denominator = maxPoolSize; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/client/SharedAsyncConnection.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/client/SharedAsyncConnection.java index cb334799276e..93ef5fd911a0 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/client/SharedAsyncConnection.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/client/SharedAsyncConnection.java @@ -53,13 +53,13 @@ public Configuration getConfiguration() { } @Override - public String getClusterId2() { - return conn.getClusterId2(); + public String getClusterIdentity() { + return conn.getClusterIdentity(); } @Override - public String getIdentity() { - return conn.getIdentity(); + public String getConnectionScope() { + return conn.getConnectionScope(); } @Override diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/DummyAsyncClusterConnection.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/DummyAsyncClusterConnection.java index 3a3cedea2028..d550972f3ab0 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/DummyAsyncClusterConnection.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/DummyAsyncClusterConnection.java @@ -45,12 +45,12 @@ public Configuration getConfiguration() { } @Override - public String getClusterId2() { + public String getClusterIdentity() { return null; } @Override - public String getIdentity() { + public String getConnectionScope() { return null; } From 3eb93dcc7b0d7c01ae87ea726985b87125cef441 Mon Sep 17 00:00:00 2001 From: Victor Li Date: Thu, 17 Nov 2022 11:09:52 -0800 Subject: [PATCH 07/15] Utilize existing configuration METRICS_SCOPE_KEY for metrics identity/scope. --- .../hadoop/hbase/client/AsyncConnection.java | 10 -------- .../hbase/client/AsyncConnectionImpl.java | 23 ++++--------------- .../hbase/client/MetricsConnection.java | 18 ++++----------- .../hbase/client/SharedAsyncConnection.java | 10 -------- .../client/DummyAsyncClusterConnection.java | 10 -------- 5 files changed, 9 insertions(+), 62 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnection.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnection.java index f85cce368c67..6e96918d1d9a 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnection.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnection.java @@ -34,16 +34,6 @@ @InterfaceAudience.Public public interface AsyncConnection extends Closeable { - /** - * Returns the clusterId of this connection. - */ - String getClusterIdentity(); - - /** - * Returns this connection identity. - */ - String getConnectionScope(); - /** * Returns the {@link org.apache.hadoop.conf.Configuration} object used by this instance. *

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java index afd47842676b..2b7f4ae55f9d 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java @@ -119,7 +119,6 @@ public class AsyncConnectionImpl implements AsyncConnection { private final AtomicBoolean closed = new AtomicBoolean(false); private final String clusterId; - private final String connScope; private final Optional metrics; private final ClusterStatusListener clusterStatusListener; @@ -128,13 +127,7 @@ public class AsyncConnectionImpl implements AsyncConnection { public AsyncConnectionImpl(Configuration conf, ConnectionRegistry registry, String clusterId, SocketAddress localAddress, User user) { - this(conf, registry, clusterId, localAddress, user, null); - } - - public AsyncConnectionImpl(Configuration conf, ConnectionRegistry registry, String clusterId, - SocketAddress localAddress, User user, String connScope) { this.clusterId = clusterId; - this.connScope = connScope; this.conf = conf; this.user = user; @@ -144,8 +137,9 @@ public AsyncConnectionImpl(Configuration conf, ConnectionRegistry registry, Stri this.connConf = new AsyncConnectionConfiguration(conf); this.registry = registry; if (conf.getBoolean(CLIENT_SIDE_METRICS_ENABLED_KEY, false)) { + String scope = MetricsConnection.getScope(conf, clusterId, this); this.metrics = - Optional.of(MetricsConnection.getMetricsConnection(this, () -> null, () -> null)); + Optional.of(MetricsConnection.getMetricsConnection(scope, () -> null, () -> null)); } else { this.metrics = Optional.empty(); } @@ -214,16 +208,6 @@ public ConnectionRegistry getConnectionRegistry() { return registry; } - @Override - public String getClusterIdentity() { - return clusterId; - } - - @Override - public String getConnectionScope() { - return connScope; - } - @Override public Configuration getConfiguration() { return conf; @@ -255,7 +239,8 @@ public void close() { } } if (metrics.isPresent()) { - MetricsConnection.deleteMetricsConnection(this); + String scope = MetricsConnection.getScope(conf, clusterId, this); + MetricsConnection.deleteMetricsConnection(scope); } ConnectionOverAsyncConnection c = this.conn; if (c != null) { diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java index bf62d241cb15..afa59de01308 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java @@ -61,9 +61,8 @@ public class MetricsConnection implements StatisticTrackable { static final Map METRICS_INSTANCES = new HashMap(); - static MetricsConnection getMetricsConnection(final AsyncConnection conn, + static MetricsConnection getMetricsConnection(final String scope, Supplier batchPool, Supplier metaPool) { - String scope = getScope(conn); MetricsConnection metrics; synchronized (METRICS_INSTANCES) { metrics = METRICS_INSTANCES.get(scope); @@ -78,8 +77,7 @@ static MetricsConnection getMetricsConnection(final AsyncConnection conn, return metrics; } - static void deleteMetricsConnection(final AsyncConnection conn) { - String scope = getScope(conn); + static void deleteMetricsConnection(final String scope) { synchronized (METRICS_INSTANCES) { MetricsConnection metrics = METRICS_INSTANCES.get(scope); if (metrics == null) return; @@ -115,15 +113,9 @@ static void deleteMetricsConnection(final AsyncConnection conn) { * @param connectionObj either a Connection or AsyncConnectionImpl, the instance creating this * MetricsConnection. */ - private static String getScope(final AsyncConnection conn) { - String scope = conn.getConnectionScope(); - if (scope != null) { - return scope; - } - Configuration conf = conn.getConfiguration(); - String clusterId = conn.getClusterIdentity(); - int connHashCode = conn.hashCode(); - return conf.get(METRICS_SCOPE_KEY, clusterId + "@" + Integer.toHexString(connHashCode)); + static String getScope(Configuration conf, String clusterId, Object connectionObj) { + return conf.get(METRICS_SCOPE_KEY, + clusterId + "@" + Integer.toHexString(connectionObj.hashCode())); } private static final String CNT_BASE = "rpcCount_"; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/client/SharedAsyncConnection.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/client/SharedAsyncConnection.java index 93ef5fd911a0..ebffc7ee5111 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/client/SharedAsyncConnection.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/client/SharedAsyncConnection.java @@ -52,16 +52,6 @@ public Configuration getConfiguration() { return conn.getConfiguration(); } - @Override - public String getClusterIdentity() { - return conn.getClusterIdentity(); - } - - @Override - public String getConnectionScope() { - return conn.getConnectionScope(); - } - @Override public AsyncTableRegionLocator getRegionLocator(TableName tableName) { return conn.getRegionLocator(tableName); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/DummyAsyncClusterConnection.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/DummyAsyncClusterConnection.java index d550972f3ab0..cb54e6e72634 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/DummyAsyncClusterConnection.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/DummyAsyncClusterConnection.java @@ -44,16 +44,6 @@ public Configuration getConfiguration() { return null; } - @Override - public String getClusterIdentity() { - return null; - } - - @Override - public String getConnectionScope() { - return null; - } - @Override public AsyncTableRegionLocator getRegionLocator(TableName tableName) { return null; From 3abc7ab3f38a384552275599492817a84f43961c Mon Sep 17 00:00:00 2001 From: Victor Li Date: Thu, 17 Nov 2022 15:57:01 -0800 Subject: [PATCH 08/15] Update and add additional unit test for client metrics. --- .../hbase/client/MetricsConnection.java | 12 ++--- .../hbase/client/TestMetricsConnection.java | 52 ++++++++++++++++++- 2 files changed, 56 insertions(+), 8 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java index afa59de01308..2c6e7b1d0144 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java @@ -371,7 +371,7 @@ public Counter newMetric(Class clazz, String name, String scope) { protected final ConcurrentMap rpcCounters = new ConcurrentHashMap<>(CAPACITY, LOAD_FACTOR, CONCURRENCY_LEVEL); - MetricsConnection(String scope, Supplier batchPool, + private MetricsConnection(String scope, Supplier batchPool, Supplier metaPool) { this.scope = scope; this.batchPools.add(batchPool); @@ -459,10 +459,6 @@ MetricRegistry getMetricRegistry() { return registry; } - public void shutdown() { - this.reporter.stop(); - } - /** Produce an instance of {@link CallStats} for clients to attach to RPCs. */ public static CallStats newCallStats() { // TODO: instance pool to reduce GC? @@ -521,7 +517,7 @@ public void incrementServerOverloadedBackoffTime(long time, TimeUnit timeUnit) { } /** Return the connection count of the metrics within a scope */ - private long getConnectionCount() { + public long getConnectionCount() { return connectionCount.getCount(); } @@ -558,6 +554,10 @@ private void updateRpcGeneric(String methodName, CallStats stats) { .update(stats.getResponseSizeBytes()); } + private void shutdown() { + this.reporter.stop(); + } + /** Report RPC context to metrics system. */ public void updateRpc(MethodDescriptor method, Message param, CallStats stats) { int callsPerServer = stats.getConcurrentCallsPerServer(); diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestMetricsConnection.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestMetricsConnection.java index c4b04c40523b..9fecd053bf09 100644 --- a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestMetricsConnection.java +++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestMetricsConnection.java @@ -23,6 +23,8 @@ import com.codahale.metrics.RatioGauge; import com.codahale.metrics.RatioGauge.Ratio; import java.io.IOException; +import java.util.ArrayList; +import java.util.List; import java.util.Optional; import java.util.concurrent.Executors; import java.util.concurrent.ThreadPoolExecutor; @@ -61,14 +63,16 @@ public class TestMetricsConnection { private static final ThreadPoolExecutor BATCH_POOL = (ThreadPoolExecutor) Executors.newFixedThreadPool(2); + private static final String MOCK_CONN_STR = "mocked-connection"; + @BeforeClass public static void beforeClass() { - METRICS = new MetricsConnection("mocked-connection", () -> BATCH_POOL, () -> null); + METRICS = MetricsConnection.getMetricsConnection(MOCK_CONN_STR, () -> BATCH_POOL, () -> null); } @AfterClass public static void afterClass() { - METRICS.shutdown(); + MetricsConnection.deleteMetricsConnection(MOCK_CONN_STR); } @Test @@ -90,6 +94,50 @@ public void testMetricsConnectionScope() throws IOException { assertEquals(scope, metrics.get().scope); } + @Test + public void testMetricsWithMutiConnections() throws IOException { + Configuration conf = new Configuration(); + conf.setBoolean(MetricsConnection.CLIENT_SIDE_METRICS_ENABLED_KEY, true); + conf.set(MetricsConnection.METRICS_SCOPE_KEY, "unit-test"); + + User user = User.getCurrent(); + + /* create multiple connections */ + final int num = 3; + AsyncConnectionImpl impl; + List connList = new ArrayList(); + for (int i = 0; i < num; i++) { + impl = new AsyncConnectionImpl(conf, null, null, null, user); + connList.add(impl); + } + + /* verify metrics presence */ + impl = connList.get(0); + Optional metrics = impl.getConnectionMetrics(); + assertTrue("Metrics should be present", metrics.isPresent()); + + /* verify connection count in a shared metrics */ + long count = metrics.get().getConnectionCount(); + assertEquals("Failed to verify connection count." + count, count, num); + + /* close some connections */ + for (int i = 0; i < num - 1; i++) { + connList.get(i).close(); + } + + /* verify metrics presence again */ + impl = connList.get(num - 1); + metrics = impl.getConnectionMetrics(); + assertTrue("Metrics should be present after some of connections are closed.", metrics.isPresent()); + + /* verify count of remaining connections */ + count = metrics.get().getConnectionCount(); + assertEquals("Connection count suppose to be 1 but got: " + count, count, 1); + + /* shutdown */ + impl.close(); + } + @Test public void testStaticMetrics() throws IOException { final byte[] foo = Bytes.toBytes("foo"); From e12e75e50328503cc30574669480e2ddbf0a4da7 Mon Sep 17 00:00:00 2001 From: Victor Li Date: Thu, 17 Nov 2022 15:58:54 -0800 Subject: [PATCH 09/15] Fix spotless check. --- .../org/apache/hadoop/hbase/client/TestMetricsConnection.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestMetricsConnection.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestMetricsConnection.java index 9fecd053bf09..457c335d6896 100644 --- a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestMetricsConnection.java +++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestMetricsConnection.java @@ -128,7 +128,8 @@ public void testMetricsWithMutiConnections() throws IOException { /* verify metrics presence again */ impl = connList.get(num - 1); metrics = impl.getConnectionMetrics(); - assertTrue("Metrics should be present after some of connections are closed.", metrics.isPresent()); + assertTrue("Metrics should be present after some of connections are closed.", + metrics.isPresent()); /* verify count of remaining connections */ count = metrics.get().getConnectionCount(); From 77652cca2fdf026fa5a14137e169cd04a2f2346a Mon Sep 17 00:00:00 2001 From: Victor Li Date: Mon, 21 Nov 2022 10:23:28 -0800 Subject: [PATCH 10/15] Address review comments. --- .../hadoop/hbase/client/AsyncConnectionImpl.java | 10 ++++------ .../apache/hadoop/hbase/client/MetricsConnection.java | 11 ++++++----- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java index 2b7f4ae55f9d..3af574cfc0b2 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java @@ -118,7 +118,7 @@ public class AsyncConnectionImpl implements AsyncConnection { private final AtomicBoolean closed = new AtomicBoolean(false); - private final String clusterId; + private final String metricsScope; private final Optional metrics; private final ClusterStatusListener clusterStatusListener; @@ -127,9 +127,9 @@ public class AsyncConnectionImpl implements AsyncConnection { public AsyncConnectionImpl(Configuration conf, ConnectionRegistry registry, String clusterId, SocketAddress localAddress, User user) { - this.clusterId = clusterId; this.conf = conf; this.user = user; + this.metricsScope = MetricsConnection.getScope(conf, clusterId, this); if (user.isLoginFromKeytab()) { spawnRenewalChore(user.getUGI()); @@ -137,9 +137,8 @@ public AsyncConnectionImpl(Configuration conf, ConnectionRegistry registry, Stri this.connConf = new AsyncConnectionConfiguration(conf); this.registry = registry; if (conf.getBoolean(CLIENT_SIDE_METRICS_ENABLED_KEY, false)) { - String scope = MetricsConnection.getScope(conf, clusterId, this); this.metrics = - Optional.of(MetricsConnection.getMetricsConnection(scope, () -> null, () -> null)); + Optional.of(MetricsConnection.getMetricsConnection(metricsScope, () -> null, () -> null)); } else { this.metrics = Optional.empty(); } @@ -239,8 +238,7 @@ public void close() { } } if (metrics.isPresent()) { - String scope = MetricsConnection.getScope(conf, clusterId, this); - MetricsConnection.deleteMetricsConnection(scope); + MetricsConnection.deleteMetricsConnection(metricsScope); } ConnectionOverAsyncConnection c = this.conn; if (c != null) { diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java index 2c6e7b1d0144..4d7d9f193201 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java @@ -51,9 +51,11 @@ /** * This class is for maintaining the various connection statistics and publishing them through the * metrics interfaces. This class manages its own {@link MetricRegistry} and {@link JmxReporter} so - * as to not conflict with other uses of Yammer Metrics within the client application. Instantiating - * this class implicitly creates and "starts" instances of these classes; be sure to call - * {@link #shutdown()} to terminate the thread pools they allocate. + * as to not conflict with other uses of Yammer Metrics within the client application. Calling + * {@link #getMetricsConnection()} implicitly creates and "starts" instances of these classes; be + * sure to call {@link #deleteMetricsConnection()} to terminate the thread pools they allocate. The + * metrics reporter will be shutdown {@link #shutdown()} when all connections within this metrics + * instances are closed. */ @InterfaceAudience.Private public class MetricsConnection implements StatisticTrackable { @@ -374,8 +376,7 @@ public Counter newMetric(Class clazz, String name, String scope) { private MetricsConnection(String scope, Supplier batchPool, Supplier metaPool) { this.scope = scope; - this.batchPools.add(batchPool); - this.metaPools.add(metaPool); + addThreadPools(batchPool, metaPool); this.registry = new MetricRegistry(); this.registry.register(getExecutorPoolName(), new RatioGauge() { @Override From 298ece265dd84c8679cafe5abdb156ffcbfe7f05 Mon Sep 17 00:00:00 2001 From: Victor Li Date: Wed, 30 Nov 2022 15:53:39 -0800 Subject: [PATCH 11/15] Replaced a synchronized block of code with concurrent map. --- .../hbase/client/MetricsConnection.java | 40 +++++++++---------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java index 4d7d9f193201..71ae7bfa13c3 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java @@ -27,9 +27,7 @@ import com.codahale.metrics.RatioGauge; import com.codahale.metrics.Timer; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ConcurrentSkipListMap; @@ -60,35 +58,33 @@ @InterfaceAudience.Private public class MetricsConnection implements StatisticTrackable { - static final Map METRICS_INSTANCES = - new HashMap(); + private static final ConcurrentMap METRICS_INSTANCES = + new ConcurrentHashMap<>(); static MetricsConnection getMetricsConnection(final String scope, Supplier batchPool, Supplier metaPool) { - MetricsConnection metrics; - synchronized (METRICS_INSTANCES) { - metrics = METRICS_INSTANCES.get(scope); - if (metrics == null) { - metrics = new MetricsConnection(scope, batchPool, metaPool); - METRICS_INSTANCES.put(scope, metrics); + return METRICS_INSTANCES.compute(scope, (s, metricsConnection) -> { + if (metricsConnection == null) { + MetricsConnection newMetricsConn = new MetricsConnection(scope, batchPool, metaPool); + newMetricsConn.incrConnectionCount(); + return newMetricsConn; } else { - metrics.addThreadPools(batchPool, metaPool); + metricsConnection.addThreadPools(batchPool, metaPool); + metricsConnection.incrConnectionCount(); + return metricsConnection; } - metrics.incrConnectionCount(); - } - return metrics; + }); } static void deleteMetricsConnection(final String scope) { - synchronized (METRICS_INSTANCES) { - MetricsConnection metrics = METRICS_INSTANCES.get(scope); - if (metrics == null) return; - metrics.decrConnectionCount(); - if (metrics.getConnectionCount() == 0) { - METRICS_INSTANCES.remove(scope); - metrics.shutdown(); + METRICS_INSTANCES.computeIfPresent(scope, (s, metricsConnection) -> { + metricsConnection.decrConnectionCount(); + if (metricsConnection.getConnectionCount() == 0) { + metricsConnection.shutdown(); + return null; } - } + return metricsConnection; + }); } /** Set this key to {@code true} to enable metrics collection of client requests. */ From b9234b3dbf053fe72e3b4a584376a52690af417d Mon Sep 17 00:00:00 2001 From: Victor Li Date: Wed, 30 Nov 2022 16:16:16 -0800 Subject: [PATCH 12/15] Take care of javadoc warnings being introduced in this change. --- .../org/apache/hadoop/hbase/client/MetricsConnection.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java index 71ae7bfa13c3..0be10f8cd20a 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java @@ -50,8 +50,8 @@ * This class is for maintaining the various connection statistics and publishing them through the * metrics interfaces. This class manages its own {@link MetricRegistry} and {@link JmxReporter} so * as to not conflict with other uses of Yammer Metrics within the client application. Calling - * {@link #getMetricsConnection()} implicitly creates and "starts" instances of these classes; be - * sure to call {@link #deleteMetricsConnection()} to terminate the thread pools they allocate. The + * {@link #getMetricsConnection} implicitly creates and "starts" instances of these classes; be + * sure to call {@link #deleteMetricsConnection} to terminate the thread pools they allocate. The * metrics reporter will be shutdown {@link #shutdown()} when all connections within this metrics * instances are closed. */ From 80f135a0dab0bf3c3f726d67cb3ef9679383b5e7 Mon Sep 17 00:00:00 2001 From: Victor Li Date: Wed, 30 Nov 2022 17:24:50 -0800 Subject: [PATCH 13/15] Address review comments. --- .../hadoop/hbase/client/MetricsConnection.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java index 0be10f8cd20a..1edabb4e90f3 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java @@ -50,10 +50,10 @@ * This class is for maintaining the various connection statistics and publishing them through the * metrics interfaces. This class manages its own {@link MetricRegistry} and {@link JmxReporter} so * as to not conflict with other uses of Yammer Metrics within the client application. Calling - * {@link #getMetricsConnection} implicitly creates and "starts" instances of these classes; be - * sure to call {@link #deleteMetricsConnection} to terminate the thread pools they allocate. The - * metrics reporter will be shutdown {@link #shutdown()} when all connections within this metrics - * instances are closed. + * {@link #getMetricsConnection} implicitly creates and "starts" instances of these classes; be sure + * to call {@link #deleteMetricsConnection} to terminate the thread pools they allocate. The metrics + * reporter will be shutdown {@link #shutdown()} when all connections within this metrics instances + * are closed. */ @InterfaceAudience.Private public class MetricsConnection implements StatisticTrackable { @@ -329,8 +329,8 @@ public Counter newMetric(Class clazz, String name, String scope) { }; // List of thread pool per connection of the metrics. - private List> batchPools = new ArrayList<>(); - private List> metaPools = new ArrayList<>(); + private final List> batchPools = new ArrayList<>(); + private final List> metaPools = new ArrayList<>(); // static metrics @@ -529,7 +529,8 @@ private void decrConnectionCount() { } /** Add thread pools of additional connections to the metrics */ - private void addThreadPools(Supplier batchPool, Supplier metaPool) { + private void addThreadPools(Supplier batchPool, + Supplier metaPool) { batchPools.add(batchPool); metaPools.add(metaPool); } From c28a24824ac432122be8427072b08948fb40d8a7 Mon Sep 17 00:00:00 2001 From: Victor Li Date: Wed, 30 Nov 2022 19:08:08 -0800 Subject: [PATCH 14/15] Fixed pre-commit warnings. --- .../apache/hadoop/hbase/client/MetricsConnection.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java index 1edabb4e90f3..bec21bb9d4fa 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java @@ -50,13 +50,13 @@ * This class is for maintaining the various connection statistics and publishing them through the * metrics interfaces. This class manages its own {@link MetricRegistry} and {@link JmxReporter} so * as to not conflict with other uses of Yammer Metrics within the client application. Calling - * {@link #getMetricsConnection} implicitly creates and "starts" instances of these classes; be sure - * to call {@link #deleteMetricsConnection} to terminate the thread pools they allocate. The metrics - * reporter will be shutdown {@link #shutdown()} when all connections within this metrics instances - * are closed. + * {@link #getMetricsConnection(String, Supplier, Supplier)} implicitly creates and "starts" + * instances of these classes; be sure to call {@link #deleteMetricsConnection(String)} to terminate + * the thread pools they allocate. The metrics reporter will be shutdown {@link #shutdown()} when + * all connections within this metrics instances are closed. */ @InterfaceAudience.Private -public class MetricsConnection implements StatisticTrackable { +public final class MetricsConnection implements StatisticTrackable { private static final ConcurrentMap METRICS_INSTANCES = new ConcurrentHashMap<>(); From be2d559b9f0525fc41d58b576c12b7f99136343b Mon Sep 17 00:00:00 2001 From: Victor Li Date: Thu, 1 Dec 2022 11:23:03 -0800 Subject: [PATCH 15/15] Fix pre-commit warnings for the protected members in final class. --- .../hbase/client/MetricsConnection.java | 130 ++++++++++++++---- .../hbase/client/TestMetricsConnection.java | 14 +- .../hbase/client/ClientPushbackTestBase.java | 4 +- .../hadoop/hbase/client/TestMetaCache.java | 8 +- .../TestMultiActionMetricsFromClient.java | 6 +- .../hbase/client/TestReplicasClient.java | 4 +- 6 files changed, 124 insertions(+), 42 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java index bec21bb9d4fa..49f4e9ba0a5d 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java @@ -264,7 +264,7 @@ public void updateDelayInterval(long interval) { } } - protected ConcurrentHashMap> serverStats = + private ConcurrentHashMap> serverStats = new ConcurrentHashMap<>(); public void updateServerStats(ServerName serverName, byte[] regionName, Object r) { @@ -305,7 +305,7 @@ private static interface NewMetric { private final MetricRegistry registry; private final JmxReporter reporter; - protected final String scope; + private final String scope; private final NewMetric timerFactory = new NewMetric() { @Override @@ -334,39 +334,39 @@ public Counter newMetric(Class clazz, String name, String scope) { // static metrics - protected final Counter connectionCount; - protected final Counter metaCacheHits; - protected final Counter metaCacheMisses; - protected final CallTracker getTracker; - protected final CallTracker scanTracker; - protected final CallTracker appendTracker; - protected final CallTracker deleteTracker; - protected final CallTracker incrementTracker; - protected final CallTracker putTracker; - protected final CallTracker multiTracker; - protected final RunnerStats runnerStats; - protected final Counter metaCacheNumClearServer; - protected final Counter metaCacheNumClearRegion; - protected final Counter hedgedReadOps; - protected final Counter hedgedReadWin; - protected final Histogram concurrentCallsPerServerHist; - protected final Histogram numActionsPerServerHist; - protected final Counter nsLookups; - protected final Counter nsLookupsFailed; - protected final Timer overloadedBackoffTimer; + private final Counter connectionCount; + private final Counter metaCacheHits; + private final Counter metaCacheMisses; + private final CallTracker getTracker; + private final CallTracker scanTracker; + private final CallTracker appendTracker; + private final CallTracker deleteTracker; + private final CallTracker incrementTracker; + private final CallTracker putTracker; + private final CallTracker multiTracker; + private final RunnerStats runnerStats; + private final Counter metaCacheNumClearServer; + private final Counter metaCacheNumClearRegion; + private final Counter hedgedReadOps; + private final Counter hedgedReadWin; + private final Histogram concurrentCallsPerServerHist; + private final Histogram numActionsPerServerHist; + private final Counter nsLookups; + private final Counter nsLookupsFailed; + private final Timer overloadedBackoffTimer; // dynamic metrics // These maps are used to cache references to the metric instances that are managed by the // registry. I don't think their use perfectly removes redundant allocations, but it's // a big improvement over calling registry.newMetric each time. - protected final ConcurrentMap rpcTimers = + private final ConcurrentMap rpcTimers = new ConcurrentHashMap<>(CAPACITY, LOAD_FACTOR, CONCURRENCY_LEVEL); - protected final ConcurrentMap rpcHistograms = new ConcurrentHashMap<>( + private final ConcurrentMap rpcHistograms = new ConcurrentHashMap<>( CAPACITY * 2 /* tracking both request and response sizes */, LOAD_FACTOR, CONCURRENCY_LEVEL); private final ConcurrentMap cacheDroppingExceptions = new ConcurrentHashMap<>(CAPACITY, LOAD_FACTOR, CONCURRENCY_LEVEL); - protected final ConcurrentMap rpcCounters = + private final ConcurrentMap rpcCounters = new ConcurrentHashMap<>(CAPACITY, LOAD_FACTOR, CONCURRENCY_LEVEL); private MetricsConnection(String scope, Supplier batchPool, @@ -456,6 +456,86 @@ MetricRegistry getMetricRegistry() { return registry; } + /** scope of the metrics object */ + public String getMetricScope() { + return scope; + } + + /** serverStats metric */ + public ConcurrentHashMap> getServerStats() { + return serverStats; + } + + /** runnerStats metric */ + public RunnerStats getRunnerStats() { + return runnerStats; + } + + /** metaCacheNumClearServer metric */ + public Counter getMetaCacheNumClearServer() { + return metaCacheNumClearServer; + } + + /** metaCacheNumClearRegion metric */ + public Counter getMetaCacheNumClearRegion() { + return metaCacheNumClearRegion; + } + + /** hedgedReadOps metric */ + public Counter getHedgedReadOps() { + return hedgedReadOps; + } + + /** hedgedReadWin metric */ + public Counter getHedgedReadWin() { + return hedgedReadWin; + } + + /** numActionsPerServerHist metric */ + public Histogram getNumActionsPerServerHist() { + return numActionsPerServerHist; + } + + /** rpcCounters metric */ + public ConcurrentMap getRpcCounters() { + return rpcCounters; + } + + /** getTracker metric */ + public CallTracker getGetTracker() { + return getTracker; + } + + /** scanTracker metric */ + public CallTracker getScanTracker() { + return scanTracker; + } + + /** multiTracker metric */ + public CallTracker getMultiTracker() { + return multiTracker; + } + + /** appendTracker metric */ + public CallTracker getAppendTracker() { + return appendTracker; + } + + /** deleteTracker metric */ + public CallTracker getDeleteTracker() { + return deleteTracker; + } + + /** incrementTracker metric */ + public CallTracker getIncrementTracker() { + return incrementTracker; + } + + /** putTracker metric */ + public CallTracker getPutTracker() { + return putTracker; + } + /** Produce an instance of {@link CallStats} for clients to attach to RPCs. */ public static CallStats newCallStats() { // TODO: instance pool to reduce GC? diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestMetricsConnection.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestMetricsConnection.java index 457c335d6896..01b3ad549955 100644 --- a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestMetricsConnection.java +++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestMetricsConnection.java @@ -85,13 +85,14 @@ public void testMetricsConnectionScope() throws IOException { AsyncConnectionImpl impl = new AsyncConnectionImpl(conf, null, "foo", null, User.getCurrent()); Optional metrics = impl.getConnectionMetrics(); assertTrue("Metrics should be present", metrics.isPresent()); - assertEquals(clusterId + "@" + Integer.toHexString(impl.hashCode()), metrics.get().scope); + assertEquals(clusterId + "@" + Integer.toHexString(impl.hashCode()), + metrics.get().getMetricScope()); conf.set(MetricsConnection.METRICS_SCOPE_KEY, scope); impl = new AsyncConnectionImpl(conf, null, "foo", null, User.getCurrent()); metrics = impl.getConnectionMetrics(); assertTrue("Metrics should be present", metrics.isPresent()); - assertEquals(scope, metrics.get().scope); + assertEquals(scope, metrics.get().getMetricScope()); } @Test @@ -176,12 +177,13 @@ public void testStaticMetrics() throws IOException { } for (String method : new String[] { "Get", "Scan", "Mutate" }) { final String metricKey = "rpcCount_" + ClientService.getDescriptor().getName() + "_" + method; - final long metricVal = METRICS.rpcCounters.get(metricKey).getCount(); + final long metricVal = METRICS.getRpcCounters().get(metricKey).getCount(); assertTrue("metric: " + metricKey + " val: " + metricVal, metricVal >= loop); } - for (MetricsConnection.CallTracker t : new MetricsConnection.CallTracker[] { METRICS.getTracker, - METRICS.scanTracker, METRICS.multiTracker, METRICS.appendTracker, METRICS.deleteTracker, - METRICS.incrementTracker, METRICS.putTracker }) { + for (MetricsConnection.CallTracker t : new MetricsConnection.CallTracker[] { + METRICS.getGetTracker(), METRICS.getScanTracker(), METRICS.getMultiTracker(), + METRICS.getAppendTracker(), METRICS.getDeleteTracker(), METRICS.getIncrementTracker(), + METRICS.getPutTracker() }) { assertEquals("Failed to invoke callTimer on " + t, loop, t.callTimer.getCount()); assertEquals("Failed to invoke reqHist on " + t, loop, t.reqHist.getCount()); assertEquals("Failed to invoke respHist on " + t, loop, t.respHist.getCount()); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/ClientPushbackTestBase.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/ClientPushbackTestBase.java index b8816edb741b..8ea36bed0360 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/ClientPushbackTestBase.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/ClientPushbackTestBase.java @@ -142,14 +142,14 @@ public void testClientTracksServerPushback() throws Exception { // time reported by above debug logging has significantly deviated. MetricsConnection metrics = getConnectionMetrics(); String name = server.getServerName() + "," + Bytes.toStringBinary(regionName); - MetricsConnection.RegionStats rsStats = metrics.serverStats.get(server).get(regionName); + MetricsConnection.RegionStats rsStats = metrics.getServerStats().get(server).get(regionName); assertEquals(name, rsStats.name); assertEquals(rsStats.heapOccupancyHist.getSnapshot().getMean(), (double) regionStats.getHeapOccupancyPercent(), 0.1); assertEquals(rsStats.memstoreLoadHist.getSnapshot().getMean(), (double) regionStats.getMemStoreLoadPercent(), 0.1); - MetricsConnection.RunnerStats runnerStats = metrics.runnerStats; + MetricsConnection.RunnerStats runnerStats = metrics.getRunnerStats(); assertEquals(1, runnerStats.delayRunners.getCount()); assertEquals(1, runnerStats.normalRunners.getCount()); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMetaCache.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMetaCache.java index c74498508084..be07c8aaef01 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMetaCache.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMetaCache.java @@ -176,8 +176,8 @@ public void testCacheClearingOnCallQueueTooBig() throws Exception { table.put(put); // obtain the client metrics - long preGetRegionClears = metrics.metaCacheNumClearRegion.getCount(); - long preGetServerClears = metrics.metaCacheNumClearServer.getCount(); + long preGetRegionClears = metrics.getMetaCacheNumClearRegion().getCount(); + long preGetServerClears = metrics.getMetaCacheNumClearServer().getCount(); // attempt a get on the test table Get get = new Get(row); @@ -189,8 +189,8 @@ public void testCacheClearingOnCallQueueTooBig() throws Exception { } // verify that no cache clearing took place - long postGetRegionClears = metrics.metaCacheNumClearRegion.getCount(); - long postGetServerClears = metrics.metaCacheNumClearServer.getCount(); + long postGetRegionClears = metrics.getMetaCacheNumClearRegion().getCount(); + long postGetServerClears = metrics.getMetaCacheNumClearServer().getCount(); assertEquals(preGetRegionClears, postGetRegionClears); assertEquals(preGetServerClears, postGetServerClears); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMultiActionMetricsFromClient.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMultiActionMetricsFromClient.java index 6ffc523ee245..55646c35e435 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMultiActionMetricsFromClient.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMultiActionMetricsFromClient.java @@ -75,9 +75,9 @@ public void testMultiMetrics() throws Exception { MetricsConnection metrics = ((AsyncConnectionImpl) conn.toAsyncConnection()).getConnectionMetrics().get(); - assertEquals(1, metrics.multiTracker.reqHist.getCount()); - assertEquals(3, metrics.numActionsPerServerHist.getSnapshot().getMean(), 1e-15); - assertEquals(1, metrics.numActionsPerServerHist.getCount()); + assertEquals(1, metrics.getMultiTracker().reqHist.getCount()); + assertEquals(3, metrics.getNumActionsPerServerHist().getSnapshot().getMean(), 1e-15); + assertEquals(1, metrics.getNumActionsPerServerHist().getCount()); } } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestReplicasClient.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestReplicasClient.java index 2b96f0493bd4..1db90cb1b659 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestReplicasClient.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestReplicasClient.java @@ -569,8 +569,8 @@ public void testHedgedRead() throws Exception { // reset AsyncConnectionImpl conn = (AsyncConnectionImpl) HTU.getConnection().toAsyncConnection(); - Counter hedgedReadOps = conn.getConnectionMetrics().get().hedgedReadOps; - Counter hedgedReadWin = conn.getConnectionMetrics().get().hedgedReadWin; + Counter hedgedReadOps = conn.getConnectionMetrics().get().getHedgedReadOps(); + Counter hedgedReadWin = conn.getConnectionMetrics().get().getHedgedReadWin(); hedgedReadOps.dec(hedgedReadOps.getCount()); hedgedReadWin.dec(hedgedReadWin.getCount());