Skip to content

Commit

Permalink
Remove unnecessary ConnectionConfiguration parsing of overloaded paus…
Browse files Browse the repository at this point in the history
…e config
  • Loading branch information
bbeaudreault committed Mar 31, 2022
1 parent 2d235e3 commit e9389d2
Show file tree
Hide file tree
Showing 7 changed files with 9 additions and 115 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import static org.apache.hadoop.hbase.HConstants.HBASE_RPC_READ_TIMEOUT_KEY;
import static org.apache.hadoop.hbase.HConstants.HBASE_RPC_TIMEOUT_KEY;
import static org.apache.hadoop.hbase.HConstants.HBASE_RPC_WRITE_TIMEOUT_KEY;
import static org.apache.hadoop.hbase.client.ConnectionConfiguration.HBASE_CLIENT_PAUSE_FOR_SERVER_OVERLOADED;
import static org.apache.hadoop.hbase.client.ConnectionConfiguration.MAX_KEYVALUE_SIZE_DEFAULT;
import static org.apache.hadoop.hbase.client.ConnectionConfiguration.MAX_KEYVALUE_SIZE_KEY;
import static org.apache.hadoop.hbase.client.ConnectionConfiguration.PRIMARY_CALL_TIMEOUT_MICROSECOND;
Expand All @@ -65,6 +64,13 @@ class AsyncConnectionConfiguration {

private static final Logger LOG = LoggerFactory.getLogger(AsyncConnectionConfiguration.class);

/**
* Parameter name for client pause when server is overloaded, denoted by
* {@link org.apache.hadoop.hbase.HBaseServerException#isServerOverloaded()}
*/
public static final String HBASE_CLIENT_PAUSE_FOR_SERVER_OVERLOADED =
"hbase.client.pause.server.overloaded";

static {
// This is added where the configs are referenced. It may be too late to happen before
// any user _sets_ the old cqtbe config onto a Configuration option. So we still need
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,9 @@

package org.apache.hadoop.hbase.client;

import static org.apache.hadoop.hbase.HConstants.DEFAULT_HBASE_CLIENT_PAUSE;
import static org.apache.hadoop.hbase.HConstants.HBASE_CLIENT_PAUSE;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.HBaseServerException;
import org.apache.hadoop.hbase.HConstants;
import org.apache.yetus.audience.InterfaceAudience;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* Configuration parameters for the connection.
Expand All @@ -31,8 +26,6 @@
@InterfaceAudience.Private
public class ConnectionConfiguration {

private static final Logger LOG = LoggerFactory.getLogger(ConnectionConfiguration.class);

public static final String WRITE_BUFFER_SIZE_KEY = "hbase.client.write.buffer";
public static final long WRITE_BUFFER_SIZE_DEFAULT = 2097152;
public static final String WRITE_BUFFER_PERIODIC_FLUSH_TIMEOUT_MS =
Expand All @@ -51,23 +44,6 @@ public class ConnectionConfiguration {
public static final int PRIMARY_SCAN_TIMEOUT_MICROSECOND_DEFAULT = 1000000; // 1s
public static final String LOG_SCANNER_ACTIVITY = "hbase.client.log.scanner.activity";

/**
* Parameter name for client pause when server is overloaded, denoted by
* {@link HBaseServerException#isServerOverloaded()}
*/
public static final String HBASE_CLIENT_PAUSE_FOR_SERVER_OVERLOADED =
"hbase.client.pause.server.overloaded";

static {
// This is added where the configs are referenced. It may be too late to happen before
// any user _sets_ the old cqtbe config onto a Configuration option. So we still need
// to handle checking both properties in parsing below. The benefit of calling this is
// that it should still cause Configuration to log a warning if we do end up falling
// through to the old deprecated config.
Configuration.addDeprecation(
HConstants.HBASE_CLIENT_PAUSE_FOR_CQTBE, HBASE_CLIENT_PAUSE_FOR_SERVER_OVERLOADED);
}

private final long writeBufferSize;
private final long writeBufferPeriodicFlushTimeoutMs;
private final long writeBufferPeriodicFlushTimerTickMs;
Expand All @@ -85,8 +61,6 @@ public class ConnectionConfiguration {
private final int writeRpcTimeout;
// toggle for async/sync prefetch
private final boolean clientScannerAsyncPrefetch;
private final long pauseMs;
private final long pauseMsForServerOverloaded;

/**
* Constructor
Expand Down Expand Up @@ -142,20 +116,6 @@ public class ConnectionConfiguration {

this.writeRpcTimeout = conf.getInt(HConstants.HBASE_RPC_WRITE_TIMEOUT_KEY,
conf.getInt(HConstants.HBASE_RPC_TIMEOUT_KEY, HConstants.DEFAULT_HBASE_RPC_TIMEOUT));

long pauseMs = conf.getLong(HBASE_CLIENT_PAUSE, DEFAULT_HBASE_CLIENT_PAUSE);
long pauseMsForServerOverloaded = conf.getLong(HBASE_CLIENT_PAUSE_FOR_SERVER_OVERLOADED,
conf.getLong(HConstants.HBASE_CLIENT_PAUSE_FOR_CQTBE, pauseMs));
if (pauseMsForServerOverloaded < pauseMs) {
LOG.warn(
"The {} setting: {} ms is less than the {} setting: {} ms, use the greater one instead",
HBASE_CLIENT_PAUSE_FOR_SERVER_OVERLOADED, pauseMsForServerOverloaded,
HBASE_CLIENT_PAUSE, pauseMs);
pauseMsForServerOverloaded = pauseMs;
}

this.pauseMs = pauseMs;
this.pauseMsForServerOverloaded = pauseMsForServerOverloaded;
}

/**
Expand All @@ -181,8 +141,6 @@ protected ConnectionConfiguration() {
this.readRpcTimeout = HConstants.DEFAULT_HBASE_RPC_TIMEOUT;
this.writeRpcTimeout = HConstants.DEFAULT_HBASE_RPC_TIMEOUT;
this.rpcTimeout = HConstants.DEFAULT_HBASE_RPC_TIMEOUT;
this.pauseMs = DEFAULT_HBASE_CLIENT_PAUSE;
this.pauseMsForServerOverloaded = DEFAULT_HBASE_CLIENT_PAUSE;
}

public int getReadRpcTimeout() {
Expand Down Expand Up @@ -248,12 +206,4 @@ public boolean isClientScannerAsyncPrefetch() {
public int getRpcTimeout() {
return rpcTimeout;
}

public long getPauseMillis() {
return pauseMs;
}

public long getPauseMillisForServerOverloaded() {
return pauseMsForServerOverloaded;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,6 @@ public Table build() {
.setReadRpcTimeout(readRpcTimeout, TimeUnit.MILLISECONDS)
.setWriteRpcTimeout(writeRpcTimeout, TimeUnit.MILLISECONDS)
.setOperationTimeout(operationTimeout, TimeUnit.MILLISECONDS)
.setRetryPause(pauseMs, TimeUnit.MILLISECONDS)
.setRetryPauseForServerOverloaded(pauseMsForServerOverloaded, TimeUnit.MILLISECONDS)
.build(),
poolSupplier);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,6 @@ abstract class TableBuilderBase implements TableBuilder {

protected int writeRpcTimeout;

protected long pauseMs;

protected long pauseMsForServerOverloaded;

TableBuilderBase(TableName tableName, ConnectionConfiguration connConf) {
if (tableName == null) {
throw new IllegalArgumentException("Given table name is null");
Expand All @@ -50,8 +46,6 @@ abstract class TableBuilderBase implements TableBuilder {
this.rpcTimeout = connConf.getRpcTimeout();
this.readRpcTimeout = connConf.getReadRpcTimeout();
this.writeRpcTimeout = connConf.getWriteRpcTimeout();
this.pauseMs = connConf.getPauseMillis();
this.pauseMsForServerOverloaded = connConf.getPauseMillisForServerOverloaded();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public void itHandlesDeprecatedPauseForCQTBE() {
assertEquals(expected, config.getPauseNsForServerOverloaded());

conf = new Configuration();
conf.setLong(ConnectionConfiguration.HBASE_CLIENT_PAUSE_FOR_SERVER_OVERLOADED, timeoutMs);
conf.setLong(AsyncConnectionConfiguration.HBASE_CLIENT_PAUSE_FOR_SERVER_OVERLOADED, timeoutMs);
config = new AsyncConnectionConfiguration(conf);
assertEquals(expected, config.getPauseNsForServerOverloaded());
}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
*/
package org.apache.hadoop.hbase.client;

import static org.apache.hadoop.hbase.client.ConnectionConfiguration.HBASE_CLIENT_PAUSE_FOR_SERVER_OVERLOADED;
import static org.apache.hadoop.hbase.client.AsyncConnectionConfiguration.HBASE_CLIENT_PAUSE_FOR_SERVER_OVERLOADED;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
Expand Down

0 comments on commit e9389d2

Please sign in to comment.