apurtell commented on a change in pull request #2578:
URL: https://github.com/apache/hbase/pull/2578#discussion_r511089975
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RpcConnection.java
##########
@@ -126,8 +136,13 @@ protected RpcConnection(Configuration conf,
HashedWheelTimer timeoutTimer, Conne
LOG.debug("Use " + authMethod + " authentication for service " +
remoteId.serviceName
+ ", sasl=" + useSasl);
}
+
reloginMaxBackoff = conf.getInt("hbase.security.relogin.maxbackoff", 5000);
Review comment:
Isn't this confusing now? Consider an operator or a tech writer looking
at this code. What does "maxbackoff" mean? It's not the max time we would back
off (5 seconds?) when now there is "hbase.security.force.relogin.backoff" at
600000 I assume milliseconds. These things need better naming, as an operator I
have no clear idea what to do here.
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RpcConnection.java
##########
@@ -126,8 +136,13 @@ protected RpcConnection(Configuration conf,
HashedWheelTimer timeoutTimer, Conne
LOG.debug("Use " + authMethod + " authentication for service " +
remoteId.serviceName
+ ", sasl=" + useSasl);
}
+
reloginMaxBackoff = conf.getInt("hbase.security.relogin.maxbackoff", 5000);
this.remoteId = remoteId;
+
+ forceReloginEnabled =
conf.getBoolean("hbase.security.force.relogin.enabled", false);
Review comment:
Is there a reason this shouldn't default to 'true'? Otherwise we don't
clear the cache and we should?
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RpcConnection.java
##########
@@ -126,8 +136,13 @@ protected RpcConnection(Configuration conf,
HashedWheelTimer timeoutTimer, Conne
LOG.debug("Use " + authMethod + " authentication for service " +
remoteId.serviceName
+ ", sasl=" + useSasl);
}
+
reloginMaxBackoff = conf.getInt("hbase.security.relogin.maxbackoff", 5000);
this.remoteId = remoteId;
+
+ forceReloginEnabled =
conf.getBoolean("hbase.security.force.relogin.enabled", false);
+ // Default minimum time between force relogin attempts is 10 minutes
+ this.forceReloginBackoff =
conf.getInt("hbase.security.force.relogin.backoff", 10 * 60 * 1000);
Review comment:
See above comment. These config names do not help an operator to know
what to do at all.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
