bharathv commented on a change in pull request #3601:
URL: https://github.com/apache/hbase/pull/3601#discussion_r694459326
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/RpcConnectionRegistry.java
##########
@@ -51,6 +51,9 @@
/** Configuration key that controls the fan out of requests **/
public static final String HEDGED_REQS_FANOUT_KEY =
"hbase.client.bootstrap.hedged.fanout";
+ public static final String INITIAL_REFRESH_DELAY_SECS =
Review comment:
nit: I think this could use a small comment (essentially what you added
in the jira) to make it obvious.
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegistryEndpointsRefresher.java
##########
@@ -60,9 +63,20 @@ public synchronized void stop() {
notifyAll();
}
+ private long getRefreshIntervalMs(boolean firstRefresh) {
+ if (refreshNow) {
+ return minTimeBetweenRefreshesMs;
+ }
+ if (firstRefresh) {
+ return initialDelayMs;
Review comment:
One minor edge case is initialDelayMs > periodicRefreshMs (like in a
misconfiguration), that needs to be handled ?
(or) we can make initialDelayMs = periodicRefreshMs/4 or something like
that, essentially a f(periodicRefreshMs) rather than a separate config,
whatever works for you, I don't have a preference.
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegistryEndpointsRefresher.java
##########
@@ -60,9 +63,20 @@ public synchronized void stop() {
notifyAll();
}
+ private long getRefreshIntervalMs(boolean firstRefresh) {
+ if (refreshNow) {
+ return minTimeBetweenRefreshesMs;
+ }
+ if (firstRefresh) {
Review comment:
Oh, sorry interpreted it as something else totally. I thought you were
talking about what if all requests hit the servers at the same time and
suggested jitter, my bad!
--
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]
