ctubbsii commented on a change in pull request #1552: Fix and simplify
TransportCachingIT
URL: https://github.com/apache/accumulo/pull/1552#discussion_r389836782
##########
File path: test/src/main/java/org/apache/accumulo/test/TransportCachingIT.java
##########
@@ -49,122 +45,78 @@
*/
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 third and sixth transport to be the same instance",
second, seventh);
Review comment:
Comment is incorrect:
```suggestion
assertSame("Expected second and seventh transport to be the same
instance", second, seventh);
```
----------------------------------------------------------------
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]
With regards,
Apache Git Services
