[GitHub] [accumulo] ctubbsii commented on a diff in pull request #2612: Minor cleanup of ThriftTransportPool

[GitBox <gi...@apache.org>]

ctubbsii commented on code in PR #2612:
URL: https://github.com/apache/accumulo/pull/2612#discussion_r845208792


##########
core/src/main/java/org/apache/accumulo/core/clientImpl/ClientContext.java:
##########
@@ -1014,11 +1014,14 @@ public ZooReader getZooReader() {
     return zooReader;
   }
 
+  protected long getTransportPoolMaxAgeMillis() {
+    return 3000; // 3 seconds for clients
+  }

Review Comment:
   I can definitely change this so it uses the normal `Property.GENERAL_RPC_TIMEOUT`. I didn't do it before because we have a default value of 2 minutes for our general RPC timeouts, and that seemed to be very far from the 3 second hard-coded value previously used. So, I went with a change that left the current value in place.
   
   However, I think it's probably okay to increase it to 2 minutes by default, after reading the comment on [ACCUMULO-2069 (comment)](https://issues.apache.org/jira/browse/ACCUMULO-2069?focusedCommentId=13861091&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13861091). I think the value of 3 seconds was probably used so clients were more responsive in the absence of a close method. However, that is no longer necessary, since there is a proper close method on clients now, and all the transports get closed when the client is closed. So, it's okay that they are left around longer.



-- 
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: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org