Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,16 @@
@InterfaceAudience.Public
public interface AsyncConnection extends Closeable {

/**
* Returns the clusterId of this connection.
*/
String getClusterId2();
Comment thread
vli02 marked this conversation as resolved.
Outdated

/**
* Returns this connection identity.
*/
String getIdentity();
Comment thread
vli02 marked this conversation as resolved.
Outdated

/**
* Returns the {@link org.apache.hadoop.conf.Configuration} object used by this instance.
* <p>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ public class AsyncConnectionImpl implements AsyncConnection {

private final AtomicBoolean closed = new AtomicBoolean(false);

private final String clusterId;
Comment thread
vli02 marked this conversation as resolved.
Outdated
private final String identity;
private final Optional<MetricsConnection> metrics;

private final ClusterStatusListener clusterStatusListener;
Expand All @@ -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;

Expand All @@ -135,8 +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)) {
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();
}
Expand Down Expand Up @@ -205,6 +214,16 @@ public ConnectionRegistry getConnectionRegistry() {
return registry;
}

@Override
public String getClusterId2() {
return clusterId;
}

@Override
public String getIdentity() {
return identity;
}

@Override
public Configuration getConfiguration() {
return conf;
Expand Down Expand Up @@ -235,7 +254,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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 <code>conf</code> 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. <br>
* The caller is responsible for calling {@link Connection#close()} on the returned connection
* instance. Typical usage:
*
* <pre>
* Connection connection = ConnectionFactory.createConnection(conf);
* Table table = connection.getTable(TableName.valueOf("table1"));
* try {
* table.get(...);
* ...
* } finally {
* table.close();
* connection.close();
* }
* </pre>
*
* @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.
Comment thread
vli02 marked this conversation as resolved.
Outdated
* @return Connection object for <code>conf</code>
*/
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) {
Expand All @@ -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));
Comment thread
vli02 marked this conversation as resolved.
Outdated
} catch (Exception e) {
throw new IOException(e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@
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;
import java.util.concurrent.ConcurrentSkipListMap;
Expand Down Expand Up @@ -54,6 +58,38 @@
@InterfaceAudience.Private
public class MetricsConnection implements StatisticTrackable {
Comment thread
vli02 marked this conversation as resolved.
Outdated

static final Map<String, MetricsConnection> METRICS_INSTANCES =
new HashMap<String, MetricsConnection>();

static MetricsConnection getMetricsConnection(final AsyncConnection conn,
Supplier<ThreadPoolExecutor> batchPool, Supplier<ThreadPoolExecutor> metaPool) {
String scope = getScope(conn);
MetricsConnection metrics;
synchronized (METRICS_INSTANCES) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider a synchronized or concurrent map type instead.

@vli02 vli02 Nov 12, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if a synchronized or concurrent map can protect the entire block, I want this entire block to run in single thread mode. Especially in the deletion method below, the decrementing count, getting count, and remove it from the map have to be single threaded. @apurtell @d-c-manning

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The scope here should be unique per async connection object, is that correct?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the scope that you meant here is the scope of this code block, it is per async connection object, and the single metrics object which might be shared among multiple async connection objects. Please refer the comment in the deleteMetricsConnection() code block, too.

metrics = METRICS_INSTANCES.get(scope);
if (metrics == null) {
metrics = new MetricsConnection(scope, batchPool, metaPool);
METRICS_INSTANCES.put(scope, metrics);
} else {
metrics.addThreadPools(batchPool, metaPool);
}
metrics.incrConnectionCount();
}
return metrics;
}

static void deleteMetricsConnection(final AsyncConnection conn) {
String scope = getScope(conn);
synchronized (METRICS_INSTANCES) {
Comment thread
vli02 marked this conversation as resolved.
Outdated
MetricsConnection metrics = METRICS_INSTANCES.get(scope);
metrics.decrConnectionCount();
Comment thread
vli02 marked this conversation as resolved.
Outdated
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";

Expand All @@ -78,9 +114,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_";
Expand Down Expand Up @@ -295,8 +337,13 @@ public Counter newMetric(Class<?> clazz, String name, String scope) {
}
};

// List of thread pool per connection of the metrics.
private List batchPools = new ArrayList<Supplier>();
private List metaPools = new ArrayList<Supplier>();
Comment thread
vli02 marked this conversation as resolved.
Outdated

// static metrics

protected final Counter connectionCount;
protected final Counter metaCacheHits;
protected final Counter metaCacheMisses;
protected final CallTracker getTracker;
Expand Down Expand Up @@ -334,27 +381,48 @@ public Counter newMetric(Class<?> clazz, String name, String scope) {
MetricsConnection(String scope, Supplier<ThreadPoolExecutor> batchPool,
Supplier<ThreadPoolExecutor> metaPool) {
this.scope = scope;
this.batchPools.add(batchPool);
Comment thread
vli02 marked this conversation as resolved.
Outdated
this.metaPools.add(metaPool);
Comment thread
vli02 marked this conversation as resolved.
Outdated
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;
Comment thread
vli02 marked this conversation as resolved.
int denominator = 0;
for (int i = 0; i < batchPools.size(); i++) {
ThreadPoolExecutor pool = (ThreadPoolExecutor) ((Supplier) batchPools.get(i)).get();
Comment thread
vli02 marked this conversation as resolved.
Outdated
if (pool != null) {
int activeCount = pool.getActiveCount();
int maxPoolSize = pool.getMaximumPoolSize();
if (numerator == 0 || (numerator * maxPoolSize) < (activeCount * denominator)) {
numerator = activeCount;
denominator = maxPoolSize;
}
Comment thread
vli02 marked this conversation as resolved.
}
}
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));
this.metaCacheHits = registry.counter(name(this.getClass(), "metaCacheHits", scope));
this.metaCacheMisses = registry.counter(name(this.getClass(), "metaCacheMisses", scope));
this.metaCacheNumClearServer =
Expand Down Expand Up @@ -457,6 +525,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();
Comment thread
vli02 marked this conversation as resolved.
}

/** 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) {
Comment thread
vli02 marked this conversation as resolved.
Outdated
batchPools.add(batchPool);
metaPools.add(metaPool);
}

/**
* Get a metric for {@code key} from {@code map}, or create it with {@code factory}.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down