You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by jm...@apache.org on 2020/05/06 11:40:49 UTC
[accumulo] branch master updated: Remove non-looping 'while' loops.
(#1604)
This is an automated email from the ASF dual-hosted git repository.
jmark99 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 438b011 Remove non-looping 'while' loops. (#1604)
438b011 is described below
commit 438b0112cec29eb0024756387805b9c1058df856
Author: Mark Owens <jm...@apache.org>
AuthorDate: Wed May 6 07:40:38 2020 -0400
Remove non-looping 'while' loops. (#1604)
ReplicationClient.java and ThriftServerBindsBeforeZooKeeperLockIT.java contain instances of 'while' loops which never loop.
In the case of ReplicationClient, the loop is always entered but either returns or throws and exception.
ThriftServerBindsBeforeZooKepperLockIT contains three 'while' loops that never loop.
These loops have been removed to increase readability of the code.
---
.../core/clientImpl/ReplicationClient.java | 26 ++-
.../ThriftServerBindsBeforeZooKeeperLockIT.java | 180 ++++++++++-----------
2 files changed, 99 insertions(+), 107 deletions(-)
diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/ReplicationClient.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/ReplicationClient.java
index 7cb7aae..7f985e9 100644
--- a/core/src/main/java/org/apache/accumulo/core/clientImpl/ReplicationClient.java
+++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/ReplicationClient.java
@@ -184,20 +184,18 @@ public class ReplicationClient {
ClientExecReturn<T,ReplicationServicer.Client> exec, long timeout)
throws AccumuloException, AccumuloSecurityException {
ReplicationServicer.Client client = null;
- while (true) {
- try {
- client = getServicerConnection(context, tserver, timeout);
- return exec.execute(client);
- } catch (ThriftSecurityException e) {
- throw new AccumuloSecurityException(e.user, e.code, e);
- } catch (AccumuloException e) {
- throw e;
- } catch (Exception e) {
- throw new AccumuloException(e);
- } finally {
- if (client != null)
- close(client);
- }
+ try {
+ client = getServicerConnection(context, tserver, timeout);
+ return exec.execute(client);
+ } catch (ThriftSecurityException e) {
+ throw new AccumuloSecurityException(e.user, e.code, e);
+ } catch (AccumuloException e) {
+ throw e;
+ } catch (Exception e) {
+ throw new AccumuloException(e);
+ } finally {
+ if (client != null)
+ close(client);
}
}
diff --git a/test/src/main/java/org/apache/accumulo/test/ThriftServerBindsBeforeZooKeeperLockIT.java b/test/src/main/java/org/apache/accumulo/test/ThriftServerBindsBeforeZooKeeperLockIT.java
index b55b826..6fec7d1 100644
--- a/test/src/main/java/org/apache/accumulo/test/ThriftServerBindsBeforeZooKeeperLockIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/ThriftServerBindsBeforeZooKeeperLockIT.java
@@ -85,48 +85,46 @@ public class ThriftServerBindsBeforeZooKeeperLockIT extends AccumuloClusterHarne
LOG.debug("Found active monitor");
- while (true) {
- int freePort = PortUtils.getRandomFreePort();
- String monitorUrl = "http://localhost:" + freePort;
- Process monitor = null;
- try {
- LOG.debug("Starting standby monitor on {}", freePort);
- monitor = startProcess(cluster, ServerType.MONITOR, freePort);
+ int freePort = PortUtils.getRandomFreePort();
+ String monitorUrl = "http://localhost:" + freePort;
+ Process monitor = null;
+ try {
+ LOG.debug("Starting standby monitor on {}", freePort);
+ monitor = startProcess(cluster, ServerType.MONITOR, freePort);
- while (true) {
- URL url = new URL(monitorUrl);
- try {
- HttpURLConnection cnxn = (HttpURLConnection) url.openConnection();
- final int responseCode = cnxn.getResponseCode();
- String errorText;
- // This is our "assertion", but we want to re-check it if it's not what we expect
- if (responseCode == HttpURLConnection.HTTP_OK) {
- return;
- } else {
- errorText = FunctionalTestUtils.readAll(cnxn.getErrorStream());
- }
- LOG.debug("Unexpected responseCode and/or error text, will retry: '{}' '{}'",
- responseCode, errorText);
- } catch (Exception e) {
- LOG.debug("Caught exception trying to fetch monitor info", e);
- }
- // Wait before trying again
- Thread.sleep(1000);
- // Make sure the process is still up. Possible the "randomFreePort" we got wasn't actually
- // free and the process
- // died trying to bind it. Pick a new port and restart it in that case.
- if (!monitor.isAlive()) {
- freePort = PortUtils.getRandomFreePort();
- monitorUrl = "http://localhost:" + freePort;
- LOG.debug("Monitor died, restarting it listening on {}", freePort);
- monitor = startProcess(cluster, ServerType.MONITOR, freePort);
+ while (true) {
+ URL url = new URL(monitorUrl);
+ try {
+ HttpURLConnection cnxn = (HttpURLConnection) url.openConnection();
+ final int responseCode = cnxn.getResponseCode();
+ String errorText;
+ // This is our "assertion", but we want to re-check it if it's not what we expect
+ if (responseCode == HttpURLConnection.HTTP_OK) {
+ return;
+ } else {
+ errorText = FunctionalTestUtils.readAll(cnxn.getErrorStream());
}
+ LOG.debug("Unexpected responseCode and/or error text, will retry: '{}' '{}'",
+ responseCode, errorText);
+ } catch (Exception e) {
+ LOG.debug("Caught exception trying to fetch monitor info", e);
}
- } finally {
- if (monitor != null) {
- monitor.destroyForcibly();
+ // Wait before trying again
+ Thread.sleep(1000);
+ // Make sure the process is still up. Possible the "randomFreePort" we got wasn't actually
+ // free and the process
+ // died trying to bind it. Pick a new port and restart it in that case.
+ if (!monitor.isAlive()) {
+ freePort = PortUtils.getRandomFreePort();
+ monitorUrl = "http://localhost:" + freePort;
+ LOG.debug("Monitor died, restarting it listening on {}", freePort);
+ monitor = startProcess(cluster, ServerType.MONITOR, freePort);
}
}
+ } finally {
+ if (monitor != null) {
+ monitor.destroyForcibly();
+ }
}
}
@@ -155,39 +153,37 @@ public class ThriftServerBindsBeforeZooKeeperLockIT extends AccumuloClusterHarne
LOG.debug("Found active master");
- while (true) {
- int freePort = PortUtils.getRandomFreePort();
- Process master = null;
- try {
- LOG.debug("Starting standby master on {}", freePort);
- master = startProcess(cluster, ServerType.MASTER, freePort);
+ int freePort = PortUtils.getRandomFreePort();
+ Process master = null;
+ try {
+ LOG.debug("Starting standby master on {}", freePort);
+ master = startProcess(cluster, ServerType.MASTER, freePort);
- while (true) {
- try (Socket s = new Socket("localhost", freePort)) {
- if (s.isConnected()) {
- // Pass
- return;
- }
- } catch (Exception e) {
- LOG.debug("Caught exception trying to connect to Master", e);
- }
- // Wait before trying again
- Thread.sleep(1000);
- // Make sure the process is still up. Possible the "randomFreePort" we got wasn't
- // actually
- // free and the process
- // died trying to bind it. Pick a new port and restart it in that case.
- if (!master.isAlive()) {
- freePort = PortUtils.getRandomFreePort();
- LOG.debug("Master died, restarting it listening on {}", freePort);
- master = startProcess(cluster, ServerType.MASTER, freePort);
+ while (true) {
+ try (Socket s = new Socket("localhost", freePort)) {
+ if (s.isConnected()) {
+ // Pass
+ return;
}
+ } catch (Exception e) {
+ LOG.debug("Caught exception trying to connect to Master", e);
}
- } finally {
- if (master != null) {
- master.destroyForcibly();
+ // Wait before trying again
+ Thread.sleep(1000);
+ // Make sure the process is still up. Possible the "randomFreePort" we got wasn't
+ // actually
+ // free and the process
+ // died trying to bind it. Pick a new port and restart it in that case.
+ if (!master.isAlive()) {
+ freePort = PortUtils.getRandomFreePort();
+ LOG.debug("Master died, restarting it listening on {}", freePort);
+ master = startProcess(cluster, ServerType.MASTER, freePort);
}
}
+ } finally {
+ if (master != null) {
+ master.destroyForcibly();
+ }
}
}
}
@@ -217,39 +213,37 @@ public class ThriftServerBindsBeforeZooKeeperLockIT extends AccumuloClusterHarne
LOG.debug("Found active gc");
- while (true) {
- int freePort = PortUtils.getRandomFreePort();
- Process master = null;
- try {
- LOG.debug("Starting standby gc on {}", freePort);
- master = startProcess(cluster, ServerType.GARBAGE_COLLECTOR, freePort);
+ int freePort = PortUtils.getRandomFreePort();
+ Process master = null;
+ try {
+ LOG.debug("Starting standby gc on {}", freePort);
+ master = startProcess(cluster, ServerType.GARBAGE_COLLECTOR, freePort);
- while (true) {
- try (Socket s = new Socket("localhost", freePort)) {
- if (s.isConnected()) {
- // Pass
- return;
- }
- } catch (Exception e) {
- LOG.debug("Caught exception trying to connect to GC", e);
- }
- // Wait before trying again
- Thread.sleep(1000);
- // Make sure the process is still up. Possible the "randomFreePort" we got wasn't
- // actually
- // free and the process
- // died trying to bind it. Pick a new port and restart it in that case.
- if (!master.isAlive()) {
- freePort = PortUtils.getRandomFreePort();
- LOG.debug("GC died, restarting it listening on {}", freePort);
- master = startProcess(cluster, ServerType.GARBAGE_COLLECTOR, freePort);
+ while (true) {
+ try (Socket s = new Socket("localhost", freePort)) {
+ if (s.isConnected()) {
+ // Pass
+ return;
}
+ } catch (Exception e) {
+ LOG.debug("Caught exception trying to connect to GC", e);
}
- } finally {
- if (master != null) {
- master.destroyForcibly();
+ // Wait before trying again
+ Thread.sleep(1000);
+ // Make sure the process is still up. Possible the "randomFreePort" we got wasn't
+ // actually
+ // free and the process
+ // died trying to bind it. Pick a new port and restart it in that case.
+ if (!master.isAlive()) {
+ freePort = PortUtils.getRandomFreePort();
+ LOG.debug("GC died, restarting it listening on {}", freePort);
+ master = startProcess(cluster, ServerType.GARBAGE_COLLECTOR, freePort);
}
}
+ } finally {
+ if (master != null) {
+ master.destroyForcibly();
+ }
}
}
}