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


##########
core/src/test/java/org/apache/accumulo/core/clientImpl/ThriftTransportKeyTest.java:
##########
@@ -136,8 +138,8 @@ public void testSimpleEquivalence() {
 
     replay(clientCtx);
 
-    ThriftTransportKey ttk =
-        new ThriftTransportKey(HostAndPort.fromParts("localhost", 9999), 
120_000, clientCtx);
+    ThriftTransportKey ttk = new ThriftTransportKey(ThriftClientTypes.CLIENT,
+        HostAndPort.fromParts("localhost", 9999), 120_000, clientCtx);
 
     assertEquals(ttk, ttk, "Normal ThriftTransportKey doesn't equal itself");

Review Comment:
   Added this in 1a3d5e0



##########
core/src/main/java/org/apache/accumulo/core/rpc/clients/TServerClient.java:
##########
@@ -57,44 +59,56 @@ default Pair<String,C> getTabletServerConnection(Logger 
LOG, ThriftClientTypes<C
       ClientContext context, boolean preferCachedConnections, AtomicBoolean 
warned)
       throws TTransportException {
     checkArgument(context != null, "context is null");
-    long rpcTimeout = context.getClientTimeoutInMillis();
-    // create list of servers
-    ArrayList<ThriftTransportKey> servers = new ArrayList<>();
 
-    // add tservers
-    ZooCache zc = context.getZooCache();
-    for (String tserver : zc.getChildren(context.getZooKeeperRoot() + 
Constants.ZTSERVERS)) {
+    if (preferCachedConnections) {
+      Pair<String,TTransport> cachedTransport =
+          context.getTransportPool().getAnyCachedTransport(type);
+      if (cachedTransport != null) {
+        C client = ThriftUtil.createClient(type, cachedTransport.getSecond());
+        warned.set(false);
+        return new Pair<String,C>(cachedTransport.getFirst(), client);
+      }
+    }
+
+    final long rpcTimeout = context.getClientTimeoutInMillis();
+    final ZooCache zc = context.getZooCache();
+    final List<String> tservers = new ArrayList<>();
+
+    tservers.addAll(zc.getChildren(context.getZooKeeperRoot() + 
Constants.ZTSERVERS));
+
+    if (tservers.isEmpty()) {
+      if (warned.compareAndSet(false, true)) {
+        LOG.warn("There are no tablet servers: check that zookeeper and 
accumulo are running.");
+      }
+      throw new TTransportException("There are no servers for type: " + type);
+    }
+
+    // Try to connect to an online tserver
+    Collections.shuffle(tservers);
+    for (String tserver : tservers) {
       var zLocPath =
           ServiceLock.path(context.getZooKeeperRoot() + Constants.ZTSERVERS + 
"/" + tserver);
       byte[] data = zc.getLockData(zLocPath);
       if (data != null) {
         String strData = new String(data, UTF_8);
         if (!strData.equals("manager")) {
-          servers.add(new ThriftTransportKey(
-              new ServerServices(strData).getAddress(Service.TSERV_CLIENT), 
rpcTimeout, context));
-        }
-      }
-    }
-
-    boolean opened = false;
-    try {
-      Pair<String,TTransport> pair =
-          context.getTransportPool().getAnyTransport(servers, 
preferCachedConnections);
-      C client = ThriftUtil.createClient(type, pair.getSecond());
-      opened = true;
-      warned.set(false);
-      return new Pair<>(pair.getFirst(), client);
-    } finally {
-      if (!opened) {
-        if (warned.compareAndSet(false, true)) {
-          if (servers.isEmpty()) {
-            LOG.warn("There are no tablet servers: check that zookeeper and 
accumulo are running.");
-          } else {
-            LOG.warn("Failed to find an available server in the list of 
servers: {}", servers);
+          final HostAndPort tserverClientAddress =
+              new ServerServices(strData).getAddress(Service.TSERV_CLIENT);
+          try {
+            TTransport transport = 
context.getTransportPool().getTransport(type,
+                tserverClientAddress, rpcTimeout, context, 
preferCachedConnections);
+            C client = ThriftUtil.createClient(type, transport);
+            warned.set(false);
+            return new Pair<String,C>(tserverClientAddress.toString(), client);
+          } catch (TTransportException e) {
+            LOG.trace("Error creating transport to {}", tserverClientAddress);
+            continue;
           }
         }
       }
     }
+    LOG.warn("Failed to find an available server in the list of servers: {}", 
tservers);
+    throw new TTransportException("Failed to connect to any server");

Review Comment:
   Added this in 1a3d5e0



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

Reply via email to