This is an automated email from the ASF dual-hosted git repository.
edcoleman pushed a commit to branch 2.1
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/2.1 by this push:
new a8cc689a77 Change Wait.waitFor to throw IllegalStateException (#3568)
a8cc689a77 is described below
commit a8cc689a77c50413f0780ae58f8db7ae1b227206
Author: EdColeman <[email protected]>
AuthorDate: Thu Jul 6 12:51:02 2023 +0000
Change Wait.waitFor to throw IllegalStateException (#3568)
* Change Wait.waitFor to throw IllegalStateException
- Instead of returning a boolean with the condition status, throw an
IllegalStateException if condition not satisfied after wait period
expires so that tests do not need to test return condition.
- Fixes #3564
---
.../org/apache/accumulo/test/LargeSplitRowIT.java | 2 +-
.../accumulo/test/conf/PropStoreConfigIT.java | 48 +++++++--------
.../accumulo/test/conf/util/ZooPropEditorIT.java | 37 ++++++------
.../test/functional/KerberosRenewalIT.java | 3 +-
.../apache/accumulo/test/shell/ShellServerIT.java | 8 +--
.../java/org/apache/accumulo/test/util/Wait.java | 69 ++++++++++++++++++----
6 files changed, 106 insertions(+), 61 deletions(-)
diff --git a/test/src/main/java/org/apache/accumulo/test/LargeSplitRowIT.java
b/test/src/main/java/org/apache/accumulo/test/LargeSplitRowIT.java
index ea97f8263e..bc9a38ff33 100644
--- a/test/src/main/java/org/apache/accumulo/test/LargeSplitRowIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/LargeSplitRowIT.java
@@ -235,7 +235,7 @@ public class LargeSplitRowIT extends ConfigurableMacBase {
// Make sure a split occurs
Wait.Condition splitsToBePresent =
() ->
client.tableOperations().listSplits(tableName).stream().findAny().isPresent();
- assertTrue(Wait.waitFor(splitsToBePresent, SECONDS.toMillis(60L), 250L));
+ Wait.waitFor(splitsToBePresent, SECONDS.toMillis(60L), 250L);
}
}
diff --git
a/test/src/main/java/org/apache/accumulo/test/conf/PropStoreConfigIT.java
b/test/src/main/java/org/apache/accumulo/test/conf/PropStoreConfigIT.java
index 6cb8b1972f..e80ffdde20 100644
--- a/test/src/main/java/org/apache/accumulo/test/conf/PropStoreConfigIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/conf/PropStoreConfigIT.java
@@ -89,7 +89,7 @@ public class PropStoreConfigIT extends SharedMiniClusterBase {
public void clear() throws Exception {
try (var client = Accumulo.newClient().from(getClientProps()).build()) {
client.instanceOperations().modifyProperties(Map::clear);
- assertTrue(Wait.waitFor(() -> getStoredConfiguration().size() == 0,
5000, 500));
+ Wait.waitFor(() -> getStoredConfiguration().size() == 0, 5000, 500);
}
}
@@ -107,19 +107,19 @@ public class PropStoreConfigIT extends
SharedMiniClusterBase {
client.instanceOperations().setProperty(Property.TABLE_BLOOM_ENABLED.getKey(),
"true");
client.tableOperations().setProperty(table,
Property.TABLE_BLOOM_ENABLED.getKey(), "false");
- assertTrue(Wait.waitFor(() ->
client.instanceOperations().getSystemConfiguration()
- .get(Property.TABLE_BLOOM_ENABLED.getKey()).equals("true"), 5000,
500));
- assertTrue(Wait.waitFor(() ->
client.tableOperations().getConfiguration(table)
- .get(Property.TABLE_BLOOM_ENABLED.getKey()).equals("false"), 5000,
500));
+ Wait.waitFor(() -> client.instanceOperations().getSystemConfiguration()
+ .get(Property.TABLE_BLOOM_ENABLED.getKey()).equals("true"), 5000,
500);
+ Wait.waitFor(() -> client.tableOperations().getConfiguration(table)
+ .get(Property.TABLE_BLOOM_ENABLED.getKey()).equals("false"), 5000,
500);
// revert sys, and then over-ride to true with table prop
client.instanceOperations().removeProperty(Property.TABLE_BLOOM_ENABLED.getKey());
client.tableOperations().setProperty(table,
Property.TABLE_BLOOM_ENABLED.getKey(), "true");
- assertTrue(Wait.waitFor(() ->
client.instanceOperations().getSystemConfiguration()
- .get(Property.TABLE_BLOOM_ENABLED.getKey()).equals("false"), 5000,
500));
- assertTrue(Wait.waitFor(() ->
client.tableOperations().getConfiguration(table)
- .get(Property.TABLE_BLOOM_ENABLED.getKey()).equals("true"), 5000,
500));
+ Wait.waitFor(() -> client.instanceOperations().getSystemConfiguration()
+ .get(Property.TABLE_BLOOM_ENABLED.getKey()).equals("false"), 5000,
500);
+ Wait.waitFor(() -> client.tableOperations().getConfiguration(table)
+ .get(Property.TABLE_BLOOM_ENABLED.getKey()).equals("true"), 5000,
500);
}
}
@@ -138,21 +138,21 @@ public class PropStoreConfigIT extends
SharedMiniClusterBase {
log.info("Tables: {}", client.tableOperations().list());
client.instanceOperations().setProperty(Property.TABLE_BLOOM_SIZE.getKey(),
"12345");
- assertTrue(Wait.waitFor(() ->
client.instanceOperations().getSystemConfiguration()
- .get(Property.TABLE_BLOOM_SIZE.getKey()).equals("12345"), 5000,
500));
+ Wait.waitFor(() -> client.instanceOperations().getSystemConfiguration()
+ .get(Property.TABLE_BLOOM_SIZE.getKey()).equals("12345"), 5000, 500);
assertEquals("12345",
client.tableOperations().getConfiguration(table).get(Property.TABLE_BLOOM_SIZE.getKey()));
client.namespaceOperations().setProperty(namespace,
Property.TABLE_BLOOM_SIZE.getKey(),
"23456");
- assertTrue(Wait.waitFor(() ->
client.namespaceOperations().getConfiguration(namespace)
- .get(Property.TABLE_BLOOM_SIZE.getKey()).equals("23456"), 5000,
500));
+ Wait.waitFor(() ->
client.namespaceOperations().getConfiguration(namespace)
+ .get(Property.TABLE_BLOOM_SIZE.getKey()).equals("23456"), 5000, 500);
assertEquals("23456",
client.tableOperations().getConfiguration(table).get(Property.TABLE_BLOOM_SIZE.getKey()));
client.tableOperations().setProperty(table,
Property.TABLE_BLOOM_SIZE.getKey(), "34567");
- assertTrue(Wait.waitFor(() ->
client.tableOperations().getConfiguration(table)
- .get(Property.TABLE_BLOOM_SIZE.getKey()).equals("34567"), 5000,
500));
+ Wait.waitFor(() -> client.tableOperations().getConfiguration(table)
+ .get(Property.TABLE_BLOOM_SIZE.getKey()).equals("34567"), 5000, 500);
assertEquals("12345",
client.instanceOperations().getSystemConfiguration()
.get(Property.TABLE_BLOOM_SIZE.getKey()));
assertEquals("23456",
client.namespaceOperations().getConfiguration(namespace)
@@ -196,7 +196,7 @@ public class PropStoreConfigIT extends
SharedMiniClusterBase {
final String maxOpenFiles =
config.get(Property.TSERV_SCAN_MAX_OPENFILES.getKey());
client.instanceOperations().modifyProperties(Map::clear);
- assertTrue(Wait.waitFor(() -> getStoredConfiguration().size() == 0,
5000, 500));
+ Wait.waitFor(() -> getStoredConfiguration().size() == 0, 5000, 500);
// Properties should be empty to start
final int numProps = properties.size();
@@ -207,7 +207,7 @@ public class PropStoreConfigIT extends
SharedMiniClusterBase {
});
// Verify system properties added
- assertTrue(Wait.waitFor(() -> getStoredConfiguration().size() >
numProps, 5000, 500));
+ Wait.waitFor(() -> getStoredConfiguration().size() > numProps, 5000,
500);
// Verify properties updated
properties = getStoredConfiguration();
@@ -291,7 +291,7 @@ public class PropStoreConfigIT extends
SharedMiniClusterBase {
// final long origMaxMem =
getMemoryAsBytes(config.get(Property.TSERV_MAXMEM.getKey()));
client.instanceOperations().modifyProperties(Map::clear);
- assertTrue(Wait.waitFor(() -> getStoredConfiguration().size() == 0,
5000, 500));
+ Wait.waitFor(() -> getStoredConfiguration().size() == 0, 5000, 500);
// should be empty to start
final int numProps = properties.size();
@@ -306,7 +306,7 @@ public class PropStoreConfigIT extends
SharedMiniClusterBase {
});
// Verify system properties added
- assertTrue(Wait.waitFor(() -> getStoredConfiguration().size() >
numProps, 5000, 500));
+ Wait.waitFor(() -> getStoredConfiguration().size() > numProps, 5000,
500);
// verify properties updated
properties = getStoredConfiguration();
@@ -323,7 +323,7 @@ public class PropStoreConfigIT extends
SharedMiniClusterBase {
// should be restored
client.instanceOperations().modifyProperties(Map::clear);
- assertTrue(Wait.waitFor(() -> getStoredConfiguration().size() == 0,
5000, 500));
+ Wait.waitFor(() -> getStoredConfiguration().size() == 0, 5000, 500);
// verify default system config restored
config = client.instanceOperations().getSystemConfiguration();
@@ -417,7 +417,7 @@ public class PropStoreConfigIT extends
SharedMiniClusterBase {
original.put(Property.TABLE_BLOOM_SIZE.getKey(), "1000");
});
- assertTrue(Wait.waitFor(() -> props.get().size() > propsSize, 5000, 500));
+ Wait.waitFor(() -> props.get().size() > propsSize, 5000, 500);
// verify properties updated
properties = props.get();
@@ -436,7 +436,7 @@ public class PropStoreConfigIT extends
SharedMiniClusterBase {
});
// Wait for clear
- assertTrue(Wait.waitFor(() -> props.get().size() == propsSize, 5000, 500));
+ Wait.waitFor(() -> props.get().size() == propsSize, 5000, 500);
// verify default system config restored
config = fullConfig.get();
@@ -632,7 +632,7 @@ public class PropStoreConfigIT extends
SharedMiniClusterBase {
final var IS_NOT_CUSTOM_TABLE_PROP =
Pattern.compile("table[.]custom[.][ABCDEF]").asMatchPredicate().negate();
- assertTrue(Wait.waitFor(() -> {
+ Wait.waitFor(() -> {
var tableProps = new HashMap<>(propShim.getProperties());
tableProps.keySet().removeIf(IS_NOT_CUSTOM_TABLE_PROP);
boolean equal = expected.equals(tableProps);
@@ -641,7 +641,7 @@ public class PropStoreConfigIT extends
SharedMiniClusterBase {
"Waiting for properties to converge. Actual:" + tableProps + "
Expected:" + expected);
}
return equal;
- }));
+ });
// now that there are not other thread modifying properties, make a
modification to check that
// the returned map
diff --git
a/test/src/main/java/org/apache/accumulo/test/conf/util/ZooPropEditorIT.java
b/test/src/main/java/org/apache/accumulo/test/conf/util/ZooPropEditorIT.java
index caea14064d..27e26b4fc9 100644
--- a/test/src/main/java/org/apache/accumulo/test/conf/util/ZooPropEditorIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/conf/util/ZooPropEditorIT.java
@@ -20,7 +20,6 @@ package org.apache.accumulo.test.conf.util;
import static org.apache.accumulo.harness.AccumuloITBase.MINI_CLUSTER_ONLY;
import static org.apache.accumulo.harness.AccumuloITBase.SUNNY_DAY;
-import static org.junit.jupiter.api.Assertions.assertTrue;
import java.time.Duration;
@@ -76,13 +75,13 @@ public class ZooPropEditorIT extends SharedMiniClusterBase {
"true");
client.tableOperations().setProperty(table,
Property.TABLE_BLOOM_ENABLED.getKey(), "false");
- assertTrue(Wait.waitFor(() ->
client.instanceOperations().getSystemConfiguration()
- .get(Property.TABLE_BLOOM_ENABLED.getKey()).equals("true"), 5000,
500));
+ Wait.waitFor(() -> client.instanceOperations().getSystemConfiguration()
+ .get(Property.TABLE_BLOOM_ENABLED.getKey()).equals("true"), 5000,
500);
ZooPropEditor tool = new ZooPropEditor();
// before - check setup correct
- assertTrue(Wait.waitFor(() ->
client.tableOperations().getTableProperties(table)
- .get(Property.TABLE_BLOOM_ENABLED.getKey()).equals("false"), 5000,
500));
+ Wait.waitFor(() -> client.tableOperations().getTableProperties(table)
+ .get(Property.TABLE_BLOOM_ENABLED.getKey()).equals("false"), 5000,
500);
// set table property (table.bloom.enabled=true)
String[] setTablePropArgs = {"-p",
getCluster().getAccumuloPropertiesPath(), "-t", table,
@@ -90,48 +89,48 @@ public class ZooPropEditorIT extends SharedMiniClusterBase {
tool.execute(setTablePropArgs);
// after set - check prop changed in ZooKeeper
- assertTrue(Wait.waitFor(() ->
client.tableOperations().getTableProperties(table)
- .get(Property.TABLE_BLOOM_ENABLED.getKey()).equals("true"), 5000,
500));
+ Wait.waitFor(() -> client.tableOperations().getTableProperties(table)
+ .get(Property.TABLE_BLOOM_ENABLED.getKey()).equals("true"), 5000,
500);
String[] deleteTablePropArgs = {"-p",
getCluster().getAccumuloPropertiesPath(), "-t", table,
"-d", Property.TABLE_BLOOM_ENABLED.getKey()};
tool.execute(deleteTablePropArgs);
// after delete - check map entry is null (removed from ZooKeeper)
- assertTrue(Wait.waitFor(() ->
client.tableOperations().getTableProperties(table)
- .get(Property.TABLE_BLOOM_ENABLED.getKey()) == null, 5000, 500));
+ Wait.waitFor(() -> client.tableOperations().getTableProperties(table)
+ .get(Property.TABLE_BLOOM_ENABLED.getKey()) == null, 5000, 500);
// set system property (changed from setup)
- assertTrue(Wait.waitFor(() ->
client.instanceOperations().getSystemConfiguration()
- .get(Property.TABLE_BLOOM_ENABLED.getKey()).equals("true"), 5000,
500));
+ Wait.waitFor(() -> client.instanceOperations().getSystemConfiguration()
+ .get(Property.TABLE_BLOOM_ENABLED.getKey()).equals("true"), 5000,
500);
String[] setSystemPropArgs = {"-p",
getCluster().getAccumuloPropertiesPath(), "-s",
Property.TABLE_BLOOM_ENABLED.getKey() + "=false"};
tool.execute(setSystemPropArgs);
// after set - check map entry is false
- assertTrue(Wait.waitFor(() ->
client.instanceOperations().getSystemConfiguration()
- .get(Property.TABLE_BLOOM_ENABLED.getKey()).equals("false"), 5000,
500));
+ Wait.waitFor(() -> client.instanceOperations().getSystemConfiguration()
+ .get(Property.TABLE_BLOOM_ENABLED.getKey()).equals("false"), 5000,
500);
// set namespace property (changed from setup)
- assertTrue(Wait.waitFor(() ->
client.namespaceOperations().getNamespaceProperties(namespace)
- .get(Property.TABLE_BLOOM_ENABLED.getKey()).equals("true"), 5000,
500));
+ Wait.waitFor(() ->
client.namespaceOperations().getNamespaceProperties(namespace)
+ .get(Property.TABLE_BLOOM_ENABLED.getKey()).equals("true"), 5000,
500);
String[] setNamespacePropArgs = {"-p",
getCluster().getAccumuloPropertiesPath(), "-ns",
namespace, "-s", Property.TABLE_BLOOM_ENABLED.getKey() + "=false"};
tool.execute(setNamespacePropArgs);
// after set - check map entry is false
- assertTrue(Wait.waitFor(() ->
client.namespaceOperations().getNamespaceProperties(namespace)
- .get(Property.TABLE_BLOOM_ENABLED.getKey()).equals("false"), 5000,
500));
+ Wait.waitFor(() ->
client.namespaceOperations().getNamespaceProperties(namespace)
+ .get(Property.TABLE_BLOOM_ENABLED.getKey()).equals("false"), 5000,
500);
String[] deleteNamespacePropArgs = {"-p",
getCluster().getAccumuloPropertiesPath(), "-ns",
namespace, "-d", Property.TABLE_BLOOM_ENABLED.getKey()};
tool.execute(deleteNamespacePropArgs);
// after set - check map entry is false
- assertTrue(Wait.waitFor(() ->
client.namespaceOperations().getNamespaceProperties(namespace)
- .get(Property.TABLE_BLOOM_ENABLED.getKey()) == null, 5000, 500));
+ Wait.waitFor(() ->
client.namespaceOperations().getNamespaceProperties(namespace)
+ .get(Property.TABLE_BLOOM_ENABLED.getKey()) == null, 5000, 500);
}
}
diff --git
a/test/src/main/java/org/apache/accumulo/test/functional/KerberosRenewalIT.java
b/test/src/main/java/org/apache/accumulo/test/functional/KerberosRenewalIT.java
index 804ece8b49..ab4e0d395c 100644
---
a/test/src/main/java/org/apache/accumulo/test/functional/KerberosRenewalIT.java
+++
b/test/src/main/java/org/apache/accumulo/test/functional/KerberosRenewalIT.java
@@ -21,7 +21,6 @@ package org.apache.accumulo.test.functional;
import static java.util.concurrent.TimeUnit.MINUTES;
import static org.apache.accumulo.harness.AccumuloITBase.MINI_CLUSTER_ONLY;
import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertTrue;
import java.time.Duration;
import java.util.Map;
@@ -189,6 +188,6 @@ public class KerberosRenewalIT extends AccumuloITBase {
assertEquals("d", entry.getValue().toString());
}
client.tableOperations().delete(tableName);
- assertTrue(Wait.waitFor(() -> !client.tableOperations().exists(tableName),
20_000L, 200L));
+ Wait.waitFor(() -> !client.tableOperations().exists(tableName), 20_000L,
200L);
}
}
diff --git
a/test/src/main/java/org/apache/accumulo/test/shell/ShellServerIT.java
b/test/src/main/java/org/apache/accumulo/test/shell/ShellServerIT.java
index 1aa4b36368..50afed22e8 100644
--- a/test/src/main/java/org/apache/accumulo/test/shell/ShellServerIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/shell/ShellServerIT.java
@@ -520,10 +520,10 @@ public class ShellServerIT extends SharedMiniClusterBase {
protected void checkTableForProperty(final AccumuloClient client, final
String tableName,
final String expectedKey, final String expectedValue) throws Exception {
- assertTrue(
- Wait.waitFor(() ->
client.tableOperations().getConfiguration(tableName).get(expectedKey)
- .equals(expectedValue), 5000, 500),
- "Failed to find expected value for key: " + expectedKey);
+ Wait.waitFor(
+ () ->
client.tableOperations().getConfiguration(tableName).get(expectedKey)
+ .equals(expectedValue),
+ 5000, 500, "Failed to find expected value for key: " + expectedKey);
}
@Test
diff --git a/test/src/main/java/org/apache/accumulo/test/util/Wait.java
b/test/src/main/java/org/apache/accumulo/test/util/Wait.java
index 16e7ddb152..7352d4a4cf 100644
--- a/test/src/main/java/org/apache/accumulo/test/util/Wait.java
+++ b/test/src/main/java/org/apache/accumulo/test/util/Wait.java
@@ -29,23 +29,70 @@ public class Wait {
boolean isSatisfied() throws Exception;
}
- public static boolean waitFor(Condition condition) throws Exception {
- return waitFor(condition, MAX_WAIT_MILLIS);
+ /**
+ * Wait for the provided condition - will throw an IllegalStateException is
the wait exceeds the
+ * default wait period of 30 seconds and a retry period of 1 second.
+ *
+ * @param condition when condition evaluates ture, return from wait
+ */
+ public static void waitFor(Condition condition) {
+ waitFor(condition, MAX_WAIT_MILLIS);
}
- public static boolean waitFor(final Condition condition, final long
duration) throws Exception {
- return waitFor(condition, duration, SLEEP_MILLIS);
+ /**
+ * Wait for the provided condition - will throw an IllegalStateException is
the wait exceeds the
+ * wait duration with a default retry period of 1 second.
+ *
+ * @param condition when condition evaluates ture, return from wait
+ * @param duration maximum total time to wait (milliseconds)
+ */
+ public static void waitFor(final Condition condition, final long duration) {
+ waitFor(condition, duration, SLEEP_MILLIS);
}
- public static boolean waitFor(final Condition condition, final long duration,
- final long sleepMillis) throws Exception {
+ /**
+ * Wait for the provided condition - will throw an IllegalStateException is
the wait exceeds the
+ * wait period.
+ *
+ * @param condition when condition evaluates ture, return from wait
+ * @param duration maximum total time to wait (milliseconds)
+ * @param sleepMillis time to sleep between condition checks
+ */
+ public static void waitFor(final Condition condition, final long duration,
+ final long sleepMillis) {
+ waitFor(condition, duration, sleepMillis, "");
+ }
+
+ /**
+ * Wait for the provided condition - will throw an IllegalStateException is
the wait exceeds the
+ * wait period.
+ *
+ * @param condition when condition evaluates ture, return from wait
+ * @param duration maximum total time to wait (milliseconds)
+ * @param sleepMillis time to sleep between condition checks
+ * @param failMessage optional message to include in IllegalStateException
if condition not met
+ * before expiration.
+ */
+ public static void waitFor(final Condition condition, final long duration,
final long sleepMillis,
+ final String failMessage) {
final long expiry = System.currentTimeMillis() + duration;
- boolean conditionSatisfied = condition.isSatisfied();
- while (!conditionSatisfied && System.currentTimeMillis() < expiry) {
- TimeUnit.MILLISECONDS.sleep(sleepMillis);
- conditionSatisfied = condition.isSatisfied();
+ boolean success;
+ try {
+ success = condition.isSatisfied();
+ while (!success && System.currentTimeMillis() < expiry) {
+ TimeUnit.MILLISECONDS.sleep(sleepMillis);
+ success = condition.isSatisfied();
+ }
+ } catch (InterruptedException ex) {
+ Thread.currentThread().interrupt();
+ throw new IllegalStateException("Interrupted during wait");
+ } catch (Exception ex) {
+ throw new IllegalStateException(failMessage + ". Failed because of
exception in condition",
+ ex);
+ }
+ if (!success) {
+ throw new IllegalStateException(failMessage + ". Timeout exceeded");
}
- return conditionSatisfied;
}
}
