ndimiduk commented on a change in pull request #4180:
URL: https://github.com/apache/hbase/pull/4180#discussion_r838794337
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionConfiguration.java
##########
@@ -44,6 +51,23 @@
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(
Review comment:
Is there any problem from this deprecation being added twice?
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionConfiguration.java
##########
@@ -116,6 +142,20 @@
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) {
Review comment:
Okay this is strange. This logic (which I think is good) is replicated
from the async client. However, on master, the sync client is implemented on
top of the async client, so I think this will at best produce the warning
message twice, and otherwise have no effect. Or maybe there are places where
the ConnectionConfiguration is used, even with the sync-over-async
implementation?
It may be that this is someplace where your patches for master and
branch-2[,.*] will differ, because on the others, async and sync client
implementations are completely independent.
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RemoteWithExtrasException.java
##########
@@ -58,15 +60,26 @@
}
public RemoteWithExtrasException(String className, String msg, final boolean
doNotRetry) {
- this(className, msg, null, -1, doNotRetry);
+ this(className, msg, doNotRetry, false);
+ }
+
+ public RemoteWithExtrasException(String className, String msg, final boolean
doNotRetry,
Review comment:
Oh, it pains me so that this class is `IA.Public` and it extends a
Hadoop class :'(
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
