Skip to content

Commit 3f219b2

Browse files
ndimidukndimiduk
committed
HBASE-27652 Client-side lock contention around Configuration when using read replica regions
1 parent 6bc84de commit 3f219b2

23 files changed

Lines changed: 115 additions & 60 deletions

‎hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -344,8 +344,8 @@ public class ConnectionImplementation implements ClusterConnection, Closeable {
344344

345345
this.rpcClient = RpcClientFactory.createClient(this.conf, this.clusterId, this.metrics);
346346
this.rpcControllerFactory = RpcControllerFactory.instantiate(conf);
347-
this.rpcCallerFactory =
348-
RpcRetryingCallerFactory.instantiate(conf, interceptor, this.stats, this.metrics);
347+
this.rpcCallerFactory = RpcRetryingCallerFactory.instantiate(conf, connectionConfig,
348+
interceptor, this.stats, this.metrics);
349349
this.asyncProcess = new AsyncProcess(this, conf, rpcCallerFactory, rpcControllerFactory);
350350

351351
// Do we publish the status?
@@ -2250,8 +2250,8 @@ public TableState getTableState(TableName tableName) throws IOException {
22502250

22512251
@Override
22522252
public RpcRetryingCallerFactory getNewRpcRetryingCallerFactory(Configuration conf) {
2253-
return RpcRetryingCallerFactory.instantiate(conf, this.interceptor, this.getStatisticsTracker(),
2254-
metrics);
2253+
return RpcRetryingCallerFactory.instantiate(conf, connectionConfig, this.interceptor,
2254+
this.getStatisticsTracker(), metrics);
22552255
}
22562256

22572257
@Override

‎hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1305,8 +1305,8 @@ public <R extends Message> void batchCoprocessorService(
13051305
final List<String> callbackErrorServers = new ArrayList<>();
13061306
Object[] results = new Object[execs.size()];
13071307

1308-
AsyncProcess asyncProcess = new AsyncProcess(
1309-
connection, configuration, RpcRetryingCallerFactory.instantiate(configuration,
1308+
AsyncProcess asyncProcess = new AsyncProcess(connection, configuration,
1309+
RpcRetryingCallerFactory.instantiate(configuration, connConfiguration,
13101310
connection.getStatisticsTracker(), connection.getConnectionMetrics()),
13111311
RpcControllerFactory.instantiate(configuration));
13121312

‎hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTableMultiplexer.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -423,8 +423,10 @@ public FlushWorker(Configuration conf, ClusterConnection conn, HRegionLocation a
423423
this.addr = addr;
424424
this.multiplexer = htableMultiplexer;
425425
this.queue = new LinkedBlockingQueue<>(perRegionServerBufferQueueSize);
426+
final ConnectionConfiguration connectionConfig =
427+
conn != null ? conn.getConnectionConfiguration() : new ConnectionConfiguration(conf);
426428
RpcRetryingCallerFactory rpcCallerFactory = RpcRetryingCallerFactory.instantiate(conf,
427-
conn == null ? null : conn.getConnectionMetrics());
429+
connectionConfig, conn == null ? null : conn.getConnectionMetrics());
428430
RpcControllerFactory rpcControllerFactory = RpcControllerFactory.instantiate(conf);
429431
this.writeRpcTimeout = conf.getInt(HConstants.HBASE_RPC_WRITE_TIMEOUT_KEY,
430432
conf.getInt(HConstants.HBASE_RPC_TIMEOUT_KEY, HConstants.DEFAULT_HBASE_RPC_TIMEOUT));

‎hbase-client/src/main/java/org/apache/hadoop/hbase/client/RpcRetryingCallerFactory.java‎

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
*/
1818
package org.apache.hadoop.hbase.client;
1919

20+
import com.google.errorprone.annotations.RestrictedApi;
2021
import org.apache.hadoop.conf.Configuration;
2122
import org.apache.hadoop.hbase.util.ReflectionUtils;
2223
import org.apache.yetus.audience.InterfaceAudience;
@@ -29,20 +30,18 @@ public class RpcRetryingCallerFactory {
2930

3031
/** Configuration key for a custom {@link RpcRetryingCaller} */
3132
public static final String CUSTOM_CALLER_CONF_KEY = "hbase.rpc.callerfactory.class";
32-
protected final Configuration conf;
3333
private final ConnectionConfiguration connectionConf;
3434
private final RetryingCallerInterceptor interceptor;
3535
private final int startLogErrorsCnt;
3636
private final MetricsConnection metrics;
3737

38-
public RpcRetryingCallerFactory(Configuration conf) {
39-
this(conf, RetryingCallerInterceptorFactory.NO_OP_INTERCEPTOR, null);
38+
public RpcRetryingCallerFactory(Configuration conf, ConnectionConfiguration connectionConf) {
39+
this(conf, connectionConf, RetryingCallerInterceptorFactory.NO_OP_INTERCEPTOR, null);
4040
}
4141

42-
public RpcRetryingCallerFactory(Configuration conf, RetryingCallerInterceptor interceptor,
43-
MetricsConnection metrics) {
44-
this.conf = conf;
45-
this.connectionConf = new ConnectionConfiguration(conf);
42+
public RpcRetryingCallerFactory(Configuration conf, ConnectionConfiguration connectionConf,
43+
RetryingCallerInterceptor interceptor, MetricsConnection metrics) {
44+
this.connectionConf = connectionConf;
4645
startLogErrorsCnt = conf.getInt(AsyncProcess.START_LOG_ERRORS_AFTER_COUNT_KEY,
4746
AsyncProcess.DEFAULT_START_LOG_ERRORS_AFTER_COUNT);
4847
this.interceptor = interceptor;
@@ -71,30 +70,43 @@ public <T> RpcRetryingCaller<T> newCaller() {
7170
interceptor, startLogErrorsCnt, connectionConf.getRpcTimeout(), metrics);
7271
}
7372

73+
@RestrictedApi(explanation = "Should only be called on process initialization", link = "",
74+
allowedOnPath = ".*/hbase-server/src/main/java/.*/(HRegionServer|LoadIncrementalHFiles|SecureBulkLoadClient)\\.java")
7475
public static RpcRetryingCallerFactory instantiate(Configuration configuration,
7576
MetricsConnection metrics) {
76-
return instantiate(configuration, RetryingCallerInterceptorFactory.NO_OP_INTERCEPTOR, null,
77-
metrics);
77+
return instantiate(configuration, new ConnectionConfiguration(configuration), metrics);
7878
}
7979

8080
public static RpcRetryingCallerFactory instantiate(Configuration configuration,
81-
ServerStatisticTracker stats, MetricsConnection metrics) {
82-
return instantiate(configuration, RetryingCallerInterceptorFactory.NO_OP_INTERCEPTOR, stats,
83-
metrics);
81+
ConnectionConfiguration connectionConf, MetricsConnection metrics) {
82+
return instantiate(configuration, connectionConf,
83+
RetryingCallerInterceptorFactory.NO_OP_INTERCEPTOR, null, metrics);
8484
}
8585

8686
public static RpcRetryingCallerFactory instantiate(Configuration configuration,
87-
RetryingCallerInterceptor interceptor, ServerStatisticTracker stats,
87+
ConnectionConfiguration connectionConf, ServerStatisticTracker stats,
8888
MetricsConnection metrics) {
89+
return instantiate(configuration, connectionConf,
90+
RetryingCallerInterceptorFactory.NO_OP_INTERCEPTOR, stats, metrics);
91+
}
92+
93+
public static RpcRetryingCallerFactory instantiate(Configuration configuration,
94+
ConnectionConfiguration connectionConf, RetryingCallerInterceptor interceptor,
95+
ServerStatisticTracker stats, MetricsConnection metrics) {
8996
String clazzName = RpcRetryingCallerFactory.class.getName();
9097
String rpcCallerFactoryClazz =
9198
configuration.get(RpcRetryingCallerFactory.CUSTOM_CALLER_CONF_KEY, clazzName);
9299
RpcRetryingCallerFactory factory;
93100
if (rpcCallerFactoryClazz.equals(clazzName)) {
94-
factory = new RpcRetryingCallerFactory(configuration, interceptor, metrics);
101+
factory = new RpcRetryingCallerFactory(configuration, connectionConf, interceptor, metrics);
95102
} else {
96-
factory = ReflectionUtils.instantiateWithCustomCtor(rpcCallerFactoryClazz,
97-
new Class[] { Configuration.class }, new Object[] { configuration });
103+
try {
104+
factory = ReflectionUtils.instantiateWithCustomCtor(rpcCallerFactoryClazz,
105+
new Class[] { Configuration.class }, new Object[] { configuration, connectionConf });
106+
} catch (UnsupportedOperationException e) {
107+
factory = ReflectionUtils.instantiateWithCustomCtor(rpcCallerFactoryClazz,
108+
new Class[] { Configuration.class }, new Object[] { configuration });
109+
}
98110
}
99111
return factory;
100112
}

‎hbase-client/src/main/java/org/apache/hadoop/hbase/client/RpcRetryingCallerWithReadReplicas.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,8 @@ public RpcRetryingCallerWithReadReplicas(RpcControllerFactory rpcControllerFacto
8181
this.operationTimeout = operationTimeout;
8282
this.rpcTimeout = rpcTimeout;
8383
this.timeBeforeReplicas = timeBeforeReplicas;
84-
this.rpcRetryingCallerFactory = new RpcRetryingCallerFactory(conf);
84+
this.rpcRetryingCallerFactory =
85+
new RpcRetryingCallerFactory(conf, cConnection.getConnectionConfiguration());
8586
}
8687

8788
/**

‎hbase-client/src/main/java/org/apache/hadoop/hbase/client/ScannerCallableWithReplicas.java‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ class ScannerCallableWithReplicas implements RetryingCallable<Result[]> {
5555
private static final Logger LOG = LoggerFactory.getLogger(ScannerCallableWithReplicas.class);
5656
volatile ScannerCallable currentScannerCallable;
5757
AtomicBoolean replicaSwitched = new AtomicBoolean(false);
58-
final ClusterConnection cConnection;
58+
private final ClusterConnection cConnection;
5959
protected final ExecutorService pool;
6060
protected final int timeBeforeReplicas;
6161
private final Scan scan;
@@ -175,12 +175,15 @@ public Result[] call(int timeout) throws IOException {
175175
}
176176
regionReplication = rl.size();
177177
}
178-
// allocate a boundedcompletion pool of some multiple of number of replicas.
179-
// We want to accomodate some RPCs for redundant replica scans (but are still in progress)
178+
// allocate a bounded-completion pool of some multiple of number of replicas.
179+
// We want to accommodate some RPCs for redundant replica scans (but are still in progress)
180+
final ConnectionConfiguration connectionConfig = cConnection != null
181+
? cConnection.getConnectionConfiguration()
182+
: new ConnectionConfiguration(ScannerCallableWithReplicas.this.conf);
180183
ResultBoundedCompletionService<Pair<Result[], ScannerCallable>> cs =
181184
new ResultBoundedCompletionService<>(
182185
RpcRetryingCallerFactory.instantiate(ScannerCallableWithReplicas.this.conf,
183-
cConnection == null ? null : cConnection.getConnectionMetrics()),
186+
connectionConfig, cConnection == null ? null : cConnection.getConnectionMetrics()),
184187
pool, regionReplication * 5);
185188

186189
AtomicBoolean done = new AtomicBoolean(false);
@@ -382,9 +385,12 @@ class RetryingRPC implements RetryingCallable<Pair<Result[], ScannerCallable>>,
382385
// and we can't invoke it multiple times at the same time)
383386
this.caller = ScannerCallableWithReplicas.this.caller;
384387
if (scan.getConsistency() == Consistency.TIMELINE) {
388+
final ConnectionConfiguration connectionConfig = cConnection != null
389+
? cConnection.getConnectionConfiguration()
390+
: new ConnectionConfiguration(ScannerCallableWithReplicas.this.conf);
385391
this.caller =
386392
RpcRetryingCallerFactory
387-
.instantiate(ScannerCallableWithReplicas.this.conf,
393+
.instantiate(ScannerCallableWithReplicas.this.conf, connectionConfig,
388394
cConnection == null ? null : cConnection.getConnectionMetrics())
389395
.<Result[]> newCaller();
390396
}

‎hbase-client/src/main/java/org/apache/hadoop/hbase/client/SecureBulkLoadClient.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ protected String rpcCall() throws Exception {
6969
return response.getBulkToken();
7070
}
7171
};
72-
return RpcRetryingCallerFactory.instantiate(conn.getConfiguration(), null, null)
72+
return RpcRetryingCallerFactory.instantiate(conn.getConfiguration(), null)
7373
.<String> newCaller().callWithRetries(callable, Integer.MAX_VALUE);
7474
} catch (Throwable throwable) {
7575
throw new IOException(throwable);
@@ -91,7 +91,7 @@ protected Void rpcCall() throws Exception {
9191
return null;
9292
}
9393
};
94-
RpcRetryingCallerFactory.instantiate(conn.getConfiguration(), null, null).<Void> newCaller()
94+
RpcRetryingCallerFactory.instantiate(conn.getConfiguration(), null).<Void> newCaller()
9595
.callWithRetries(callable, Integer.MAX_VALUE);
9696
} catch (Throwable throwable) {
9797
throw new IOException(throwable);

‎hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncProcess.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -184,13 +184,15 @@ public TableName getTableName() {
184184
}
185185

186186
public MyAsyncProcess(ClusterConnection hc, Configuration conf) {
187-
super(hc, conf, new RpcRetryingCallerFactory(conf), new RpcControllerFactory(conf));
187+
super(hc, conf, new RpcRetryingCallerFactory(conf, hc.getConnectionConfiguration()),
188+
new RpcControllerFactory(conf));
188189
service = Executors.newFixedThreadPool(5);
189190
this.conf = conf;
190191
}
191192

192193
public MyAsyncProcess(ClusterConnection hc, Configuration conf, AtomicInteger nbThreads) {
193-
super(hc, conf, new RpcRetryingCallerFactory(conf), new RpcControllerFactory(conf));
194+
super(hc, conf, new RpcRetryingCallerFactory(conf, hc.getConnectionConfiguration()),
195+
new RpcControllerFactory(conf));
194196
service = new ThreadPoolExecutor(1, 20, 60, TimeUnit.SECONDS, new SynchronousQueue<>(),
195197
new CountingThreadFactory(nbThreads));
196198
}
@@ -1702,7 +1704,8 @@ public Future submit(Runnable runnable) {
17021704

17031705
static class AsyncProcessForThrowableCheck extends AsyncProcess {
17041706
public AsyncProcessForThrowableCheck(ClusterConnection hc, Configuration conf) {
1705-
super(hc, conf, new RpcRetryingCallerFactory(conf), new RpcControllerFactory(conf));
1707+
super(hc, conf, new RpcRetryingCallerFactory(conf, hc.getConnectionConfiguration()),
1708+
new RpcControllerFactory(conf));
17061709
}
17071710
}
17081711

‎hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncProcessWithRegionException.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ public class TestAsyncProcessWithRegionException {
6868
private static final Result EMPTY_RESULT = Result.create(null, true);
6969
private static final IOException IOE = new IOException("YOU CAN'T PASS");
7070
private static final Configuration CONF = new Configuration();
71+
private static final ConnectionConfiguration CONNECTION_CONFIG =
72+
new ConnectionConfiguration(CONF);
7173
private static final TableName DUMMY_TABLE = TableName.valueOf("DUMMY_TABLE");
7274
private static final byte[] GOOD_ROW = Bytes.toBytes("GOOD_ROW");
7375
private static final byte[] BAD_ROW = Bytes.toBytes("BAD_ROW");
@@ -175,7 +177,7 @@ private static ClusterConnection createHConnection() throws IOException {
175177
Mockito.when(ng.getNonceGroup()).thenReturn(HConstants.NO_NONCE);
176178
Mockito.when(hc.getNonceGenerator()).thenReturn(ng);
177179
Mockito.when(hc.getConfiguration()).thenReturn(CONF);
178-
Mockito.when(hc.getConnectionConfiguration()).thenReturn(new ConnectionConfiguration(CONF));
180+
Mockito.when(hc.getConnectionConfiguration()).thenReturn(CONNECTION_CONFIG);
179181
setMockLocation(hc, GOOD_ROW, new RegionLocations(REGION_LOCATION));
180182
setMockLocation(hc, BAD_ROW, new RegionLocations(REGION_LOCATION));
181183
Mockito
@@ -196,7 +198,8 @@ private static class MyAsyncProcess extends AsyncProcess {
196198
private final ExecutorService service = Executors.newFixedThreadPool(5);
197199

198200
MyAsyncProcess(ClusterConnection hc, Configuration conf) {
199-
super(hc, conf, new RpcRetryingCallerFactory(conf), new RpcControllerFactory(conf));
201+
super(hc, conf, new RpcRetryingCallerFactory(conf, hc.getConnectionConfiguration()),
202+
new RpcControllerFactory(conf));
200203
}
201204

202205
public AsyncRequestFuture submit(TableName tableName, List<? extends Row> rows)

‎hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestClientScanner.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ public class TestClientScanner {
7070
Scan scan;
7171
ExecutorService pool;
7272
Configuration conf;
73+
ConnectionConfiguration connectionConfig;
7374

7475
ClusterConnection clusterConn;
7576
RpcRetryingCallerFactory rpcFactory;
@@ -86,6 +87,7 @@ public void setup() throws IOException {
8687
pool = Executors.newSingleThreadExecutor();
8788
scan = new Scan();
8889
conf = new Configuration();
90+
connectionConfig = new ConnectionConfiguration(conf);
8991
Mockito.when(clusterConn.getConfiguration()).thenReturn(conf);
9092
}
9193

@@ -473,7 +475,7 @@ public void testExceptionsFromReplicasArePropagated() throws IOException {
473475

474476
// Mock a caller which calls the callable for ScannerCallableWithReplicas,
475477
// but throws an exception for the actual scanner calls via callWithRetries.
476-
rpcFactory = new MockRpcRetryingCallerFactory(conf);
478+
rpcFactory = new MockRpcRetryingCallerFactory(conf, connectionConfig);
477479
conf.set(RpcRetryingCallerFactory.CUSTOM_CALLER_CONF_KEY,
478480
MockRpcRetryingCallerFactory.class.getName());
479481

@@ -496,8 +498,9 @@ rpcFactory, new RpcControllerFactory(conf), pool, Integer.MAX_VALUE)) {
496498

497499
public static class MockRpcRetryingCallerFactory extends RpcRetryingCallerFactory {
498500

499-
public MockRpcRetryingCallerFactory(Configuration conf) {
500-
super(conf);
501+
public MockRpcRetryingCallerFactory(Configuration conf,
502+
ConnectionConfiguration connectionConf) {
503+
super(conf, connectionConf);
501504
}
502505

503506
@Override

0 commit comments

Comments
 (0)