You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by kt...@apache.org on 2020/03/09 20:21:26 UTC

[accumulo] branch master updated: Fix and simplify TransportCachingIT (#1552)

This is an automated email from the ASF dual-hosted git repository.

kturner pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/master by this push:
     new 77b515f  Fix and simplify TransportCachingIT (#1552)
77b515f is described below

commit 77b515f088209f384b5ae9ac32d22d267c947ff6
Author: Keith Turner <kt...@apache.org>
AuthorDate: Mon Mar 9 16:21:19 2020 -0400

    Fix and simplify TransportCachingIT (#1552)
---
 .../apache/accumulo/test/TransportCachingIT.java   | 138 +++++++--------------
 1 file changed, 45 insertions(+), 93 deletions(-)

diff --git a/test/src/main/java/org/apache/accumulo/test/TransportCachingIT.java b/test/src/main/java/org/apache/accumulo/test/TransportCachingIT.java
index ee84f6d..15ded44 100644
--- a/test/src/main/java/org/apache/accumulo/test/TransportCachingIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/TransportCachingIT.java
@@ -18,15 +18,13 @@
  */
 package org.apache.accumulo.test;
 
-import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertSame;
 
-import java.util.ArrayList;
 import java.util.List;
+import java.util.stream.Collectors;
 
-import org.apache.accumulo.core.Constants;
 import org.apache.accumulo.core.client.Accumulo;
 import org.apache.accumulo.core.client.AccumuloClient;
 import org.apache.accumulo.core.clientImpl.ClientContext;
@@ -34,9 +32,7 @@ import org.apache.accumulo.core.clientImpl.ThriftTransportKey;
 import org.apache.accumulo.core.clientImpl.ThriftTransportPool;
 import org.apache.accumulo.core.conf.ConfigurationTypeHelper;
 import org.apache.accumulo.core.conf.Property;
-import org.apache.accumulo.core.util.ServerServices;
-import org.apache.accumulo.core.util.ServerServices.Service;
-import org.apache.accumulo.fate.zookeeper.ZooCache;
+import org.apache.accumulo.core.util.HostAndPort;
 import org.apache.accumulo.harness.AccumuloClusterHarness;
 import org.apache.thrift.transport.TTransport;
 import org.apache.thrift.transport.TTransportException;
@@ -49,122 +45,78 @@ import org.slf4j.LoggerFactory;
  */
 public class TransportCachingIT extends AccumuloClusterHarness {
   private static final Logger log = LoggerFactory.getLogger(TransportCachingIT.class);
-  private static int ATTEMPTS = 0;
 
   @Test
   public void testCachedTransport() throws InterruptedException {
     try (AccumuloClient client = Accumulo.newClient().from(getClientProps()).build()) {
-      while (client.instanceOperations().getTabletServers().isEmpty()) {
+
+      List<String> tservers;
+
+      while ((tservers = client.instanceOperations().getTabletServers()).isEmpty()) {
         // sleep until a tablet server is up
         Thread.sleep(50);
       }
+
       ClientContext context = (ClientContext) client;
       long rpcTimeout =
           ConfigurationTypeHelper.getTimeInMillis(Property.GENERAL_RPC_TIMEOUT.getDefaultValue());
 
-      ZooCache zc = context.getZooCache();
-      final String zkRoot = context.getZooKeeperRoot();
+      List<ThriftTransportKey> servers = tservers.stream().map(serverStr -> {
+        return new ThriftTransportKey(HostAndPort.fromString(serverStr), rpcTimeout, context);
+      }).collect(Collectors.toList());
 
-      // wait until Zookeeper is populated
-      List<String> children = zc.getChildren(zkRoot + Constants.ZTSERVERS);
-      while (children.isEmpty()) {
-        Thread.sleep(100);
-        children = zc.getChildren(zkRoot + Constants.ZTSERVERS);
-      }
-
-      ArrayList<ThriftTransportKey> servers = new ArrayList<>();
-      while (servers.isEmpty()) {
-        for (String tserver : children) {
-          String path = zkRoot + Constants.ZTSERVERS + "/" + tserver;
-          byte[] data = zc.getLockData(path);
-          if (data != null) {
-            String strData = new String(data, UTF_8);
-            if (!strData.equals("master"))
-              servers.add(new ThriftTransportKey(
-                  new ServerServices(strData).getAddress(Service.TSERV_CLIENT), rpcTimeout,
-                  context));
-          }
-        }
-        ATTEMPTS++;
-        if (!servers.isEmpty())
-          break;
-        else {
-          if (ATTEMPTS < 100) {
-            log.warn("Making another attempt to add ThriftTransportKey servers");
-            Thread.sleep(100);
-          } else {
-            log.error("Failed to add ThriftTransportKey servers - Failing TransportCachingIT test");
-            org.junit.Assert
-                .fail("Failed to add ThriftTransportKey servers - Failing TransportCachingIT test");
-          }
-        }
-      }
+      // only want to use one server for all subsequent test
+      servers = servers.subList(0, 1);
 
       ThriftTransportPool pool = ThriftTransportPool.getInstance();
-      TTransport first = null;
-      while (first == null) {
-        try {
-          // Get a transport (cached or not)
-          first = pool.getAnyTransport(servers, true).getSecond();
-        } catch (TTransportException e) {
-          log.warn("Failed to obtain transport to {}", servers);
-        }
-      }
+      TTransport first = getAnyTransport(servers, pool, true);
 
       assertNotNull(first);
       // Return it to unreserve it
       pool.returnTransport(first);
 
-      TTransport second = null;
-      while (second == null) {
-        try {
-          // Get a cached transport (should be the first)
-          second = pool.getAnyTransport(servers, true).getSecond();
-        } catch (TTransportException e) {
-          log.warn("Failed obtain 2nd transport to {}", servers);
-        }
-      }
+      TTransport second = getAnyTransport(servers, pool, true);
 
       // We should get the same transport
       assertSame("Expected the first and second to be the same instance", first, second);
-      // Return the 2nd
       pool.returnTransport(second);
 
-      TTransport third = null;
-      while (third == null) {
-        try {
-          // Get a non-cached transport
-          third = pool.getAnyTransport(servers, false).getSecond();
-        } catch (TTransportException e) {
-          log.warn("Failed obtain 3rd transport to {}", servers);
-        }
-      }
-
+      // Ensure does not get cached connection just returned
+      TTransport third = getAnyTransport(servers, pool, false);
       assertNotSame("Expected second and third transport to be different instances", second, third);
-      pool.returnTransport(third);
 
-      // ensure the LIFO scheme with a fourth and fifth entry
-      TTransport fourth = null;
-      while (fourth == null) {
-        try {
-          // Get a non-cached transport
-          fourth = pool.getAnyTransport(servers, false).getSecond();
-        } catch (TTransportException e) {
-          log.warn("Failed obtain 4th transport to {}", servers);
-        }
-      }
+      TTransport fourth = getAnyTransport(servers, pool, false);
+      assertNotSame("Expected third and fourth transport to be different instances", third, fourth);
+
+      pool.returnTransport(third);
       pool.returnTransport(fourth);
-      TTransport fifth = null;
-      while (fifth == null) {
-        try {
-          // Get a cached transport
-          fifth = pool.getAnyTransport(servers, true).getSecond();
-        } catch (TTransportException e) {
-          log.warn("Failed obtain 5th transport to {}", servers);
-        }
-      }
+
+      // The following three asserts ensure the per server queue is LIFO
+      TTransport fifth = getAnyTransport(servers, pool, true);
       assertSame("Expected fourth and fifth transport to be the same instance", fourth, fifth);
+
+      TTransport sixth = getAnyTransport(servers, pool, true);
+      assertSame("Expected third and sixth transport to be the same instance", third, sixth);
+
+      TTransport seventh = getAnyTransport(servers, pool, true);
+      assertSame("Expected second and seventh transport to be the same instance", second, seventh);
+
       pool.returnTransport(fifth);
+      pool.returnTransport(sixth);
+      pool.returnTransport(seventh);
+    }
+  }
+
+  private TTransport getAnyTransport(List<ThriftTransportKey> servers, ThriftTransportPool pool,
+      boolean preferCached) {
+    TTransport first = null;
+    while (first == null) {
+      try {
+        first = pool.getAnyTransport(servers, preferCached).getSecond();
+      } catch (TTransportException e) {
+        log.warn("Failed to obtain transport to {}", servers);
+      }
     }
+    return first;
   }
 }