Re: [PR] Optimized logic for getting a random TabletServer connection [accumulo]

["dlmarion (via GitHub)" <gi...@apache.org>]

dlmarion commented on code in PR #4309:
URL: https://github.com/apache/accumulo/pull/4309#discussion_r1508934819


##########
core/src/main/java/org/apache/accumulo/core/clientImpl/ThriftTransportKey.java:
##########
@@ -81,7 +92,7 @@ public boolean equals(Object o) {
       return false;
     }
     ThriftTransportKey ttk = (ThriftTransportKey) o;
-    return server.equals(ttk.server) && timeout == ttk.timeout
+    return type.equals(ttk.type) && server.equals(ttk.server) && timeout == ttk.timeout

Review Comment:
   > could we make ThriftClientTypes an enum? Does not seem like this would work with the type parameter.
   
   IIRC I tried this when I originally created ThriftClientTypes and it didn't work with the generics
   
   > could we make ThriftClientTypes constructor private? This makes it easier to reason about single instances. The subclasses would probably need to be pulled in as inner classes to do this. Not sure if its workable even then though.
   
   I think we could probably make it `protected` vs `public`, or remove the modifier and make it package protected. I looked at all of the impls and their constructors have no modifier, so they are package protected. 
   



-- 
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