This is an automated email from the ASF dual-hosted git repository.
ctubbsii 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 26a71d0627 Scale Wait by timeout.factor and more IT QA work (#3675)
26a71d0627 is described below
commit 26a71d062702ad703f445ab144880c4c0e323c6e
Author: Christopher Tubbs <[email protected]>
AuthorDate: Wed Aug 2 11:37:36 2023 -0400
Scale Wait by timeout.factor and more IT QA work (#3675)
* Move processing of timeout.factor into the Wait class
* Remove redundant processing of timeout.factor (except for deprecated
and disabled replication tests, which are deleted in the main branch)
* Incorporate the timeout.factor into the Wait tool's sleep and duration
parameters
* Use nanoTime to compute duration for Wait, to avoid weirdness with
system clock
* Change default timeout in ITs to 24 hours instead of 5 days (no IT
should be running that long under any circumstance)
* Update ScanConsistencyIT to construct scanner or isolated scanner in
the try-with-resources resource block, to avoid warning about unclosed
Scanner when the reference is copied to wrap it with isolation
* Avoid unused variable warning in for loop by using a lambda in
ManagerAssignmentIT
* Fix typos in Wait for the word "true" (was "ture")
* Remove ScanConsistencyIT from sunny test group, as it represents
edge-case coverage rather than minimal base Accumulo functional
testing
* Make ManagerAssignmentIT less flaky by increasing the Wait.waitFor
timeout to 60 seconds instead of the default 30
---
test/pom.xml | 1 +
.../apache/accumulo/harness/AccumuloITBase.java | 18 ++++----
.../accumulo/test/BadDeleteMarkersCreatedIT.java | 10 +----
.../apache/accumulo/test/ScanConsistencyIT.java | 11 +----
.../accumulo/test/functional/LargeRowIT.java | 12 +-----
.../test/functional/ManagerAssignmentIT.java | 38 ++++++++---------
.../accumulo/test/functional/WALSunnyDayIT.java | 15 +------
.../java/org/apache/accumulo/test/util/Wait.java | 48 ++++++++++++++++++----
8 files changed, 71 insertions(+), 82 deletions(-)
diff --git a/test/pom.xml b/test/pom.xml
index de428691e8..16450b80c2 100644
--- a/test/pom.xml
+++ b/test/pom.xml
@@ -245,6 +245,7 @@
<testSourceDirectory>${project.basedir}/src/main/java/</testSourceDirectory>
<testClassesDirectory>${project.build.directory}/classes/</testClassesDirectory>
<systemPropertyVariables>
+ <!-- this is needed to pass the system property given to Maven
to failsafe -->
<timeout.factor>${timeout.factor}</timeout.factor>
<org.apache.accumulo.test.functional.useCredProviderForIT>${useCredProviderForIT}</org.apache.accumulo.test.functional.useCredProviderForIT>
<org.apache.accumulo.test.functional.useSslForIT>${useSslForIT}</org.apache.accumulo.test.functional.useSslForIT>
diff --git a/test/src/main/java/org/apache/accumulo/harness/AccumuloITBase.java
b/test/src/main/java/org/apache/accumulo/harness/AccumuloITBase.java
index 4d07f2393a..0c29ca8cae 100644
--- a/test/src/main/java/org/apache/accumulo/harness/AccumuloITBase.java
+++ b/test/src/main/java/org/apache/accumulo/harness/AccumuloITBase.java
@@ -32,6 +32,7 @@ import java.util.Map.Entry;
import org.apache.accumulo.core.client.Scanner;
import org.apache.accumulo.core.data.Key;
import org.apache.accumulo.core.data.Value;
+import org.apache.accumulo.test.util.Wait;
import org.apache.commons.io.FileUtils;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.slf4j.Logger;
@@ -118,19 +119,14 @@ public class AccumuloITBase extends WithTestNames {
Timeout timeout = Timeout.from(() -> {
assertFalse(defaultTimeout().isZero(), "defaultTimeout should not return
0");
- int timeoutFactor = 0;
- try {
- String timeoutString = System.getProperty("timeout.factor");
- if (timeoutString != null && !timeoutString.isEmpty()) {
- timeoutFactor = Integer.parseInt(timeoutString);
- }
- } catch (NumberFormatException exception) {
- log.warn("Could not parse timeout.factor, defaulting to no timeout.");
- }
+ int timeoutFactor = Wait.getTimeoutFactor(e -> {
+ log.warn("Could not parse timeout.factor, defaulting to 24 hours.");
+ return 0;
+ });
- // if the user sets a timeout factor of 0, apply a very long timeout
(effectively no timeout)
+ // if the user sets a timeout factor of 0, apply a very long timeout
if (timeoutFactor == 0) {
- return Duration.ofDays(5);
+ return Duration.ofDays(1);
}
return defaultTimeout().multipliedBy(timeoutFactor);
diff --git
a/test/src/main/java/org/apache/accumulo/test/BadDeleteMarkersCreatedIT.java
b/test/src/main/java/org/apache/accumulo/test/BadDeleteMarkersCreatedIT.java
index 4a795c926c..14beb2b723 100644
--- a/test/src/main/java/org/apache/accumulo/test/BadDeleteMarkersCreatedIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/BadDeleteMarkersCreatedIT.java
@@ -20,7 +20,6 @@ package org.apache.accumulo.test;
import static
org.apache.accumulo.core.util.UtilWaitThread.sleepUninterruptibly;
import static org.junit.jupiter.api.Assertions.assertNotNull;
-import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
import java.time.Duration;
@@ -48,6 +47,7 @@ import org.apache.accumulo.core.security.Authorizations;
import org.apache.accumulo.harness.AccumuloClusterHarness;
import org.apache.accumulo.minicluster.ServerType;
import org.apache.accumulo.miniclusterImpl.MiniAccumuloConfigImpl;
+import org.apache.accumulo.test.util.Wait;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.io.Text;
import org.junit.jupiter.api.AfterEach;
@@ -76,13 +76,7 @@ public class BadDeleteMarkersCreatedIT extends
AccumuloClusterHarness {
@BeforeEach
public void getTimeoutFactor() {
- try {
- timeoutFactor = Integer.parseInt(System.getProperty("timeout.factor"));
- } catch (NumberFormatException e) {
- log.warn("Could not parse integer from timeout.factor");
- }
-
- assertTrue(timeoutFactor >= 1, "timeout.factor must be greater than or
equal to 1");
+ timeoutFactor = Wait.getTimeoutFactor(e -> 1);
}
private String gcCycleDelay, gcCycleStart;
diff --git a/test/src/main/java/org/apache/accumulo/test/ScanConsistencyIT.java
b/test/src/main/java/org/apache/accumulo/test/ScanConsistencyIT.java
index 752d7c6a79..a882547a05 100644
--- a/test/src/main/java/org/apache/accumulo/test/ScanConsistencyIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/ScanConsistencyIT.java
@@ -19,7 +19,6 @@
package org.apache.accumulo.test;
import static java.nio.charset.StandardCharsets.UTF_8;
-import static org.apache.accumulo.harness.AccumuloITBase.SUNNY_DAY;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -63,7 +62,6 @@ import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.io.Text;
-import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -78,7 +76,6 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
* This test verifies that scans will always see data written before the scan
started even when
* there are concurrent scans, writes, and table operations running.
*/
-@Tag(SUNNY_DAY)
public class ScanConsistencyIT extends AccumuloClusterHarness {
private static final Logger log =
LoggerFactory.getLogger(ScanConsistencyIT.class);
@@ -356,14 +353,10 @@ public class ScanConsistencyIT extends
AccumuloClusterHarness {
private static ScanStats scanData(TestContext tctx, Random random, Range
range,
boolean scanIsolated) throws Exception {
try (ExpectedScanData expectedScanData = tctx.dataTracker.beginScan();
- Scanner baseScanner = tctx.client.createScanner(tctx.table)) {
+ Scanner scanner = scanIsolated ? new
IsolatedScanner(tctx.client.createScanner(tctx.table))
+ : tctx.client.createScanner(tctx.table)) {
Set<Key> expected =
expectedScanData.getExpectedData(range).collect(Collectors.toSet());
- Scanner scanner = baseScanner;
- if (scanIsolated) {
- scanner = new IsolatedScanner(baseScanner);
- }
-
Stream<Map.Entry<Key,Value>> scanStream;
if (!range.isInfiniteStopKey() && random.nextBoolean()) {
diff --git
a/test/src/main/java/org/apache/accumulo/test/functional/LargeRowIT.java
b/test/src/main/java/org/apache/accumulo/test/functional/LargeRowIT.java
index fa7d4edc7e..bef85c4d43 100644
--- a/test/src/main/java/org/apache/accumulo/test/functional/LargeRowIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/functional/LargeRowIT.java
@@ -20,7 +20,6 @@ package org.apache.accumulo.test.functional;
import static java.util.Collections.singletonMap;
import static
org.apache.accumulo.core.util.UtilWaitThread.sleepUninterruptibly;
-import static org.junit.jupiter.api.Assertions.assertTrue;
import java.time.Duration;
import java.util.Map;
@@ -45,6 +44,7 @@ import org.apache.accumulo.minicluster.MemoryUnit;
import org.apache.accumulo.minicluster.ServerType;
import org.apache.accumulo.miniclusterImpl.MiniAccumuloConfigImpl;
import org.apache.accumulo.test.TestIngest;
+import org.apache.accumulo.test.util.Wait;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.io.Text;
import org.junit.jupiter.api.AfterEach;
@@ -85,15 +85,7 @@ public class LargeRowIT extends AccumuloClusterHarness {
@BeforeEach
public void getTimeoutFactor() throws Exception {
- try {
- timeoutFactor = Integer.parseInt(System.getProperty("timeout.factor"));
- } catch (NumberFormatException e) {
- log.warn("Could not parse property value for 'timeout.factor' as
integer: {}",
- System.getProperty("timeout.factor"));
- }
-
- assertTrue(timeoutFactor >= 1,
- "org.apache.accumulo.Timeout factor must be greater than or equal to
1");
+ timeoutFactor = Wait.getTimeoutFactor(e -> 1); // default to 1
String[] names = getUniqueNames(2);
REG_TABLE_NAME = names[0];
diff --git
a/test/src/main/java/org/apache/accumulo/test/functional/ManagerAssignmentIT.java
b/test/src/main/java/org/apache/accumulo/test/functional/ManagerAssignmentIT.java
index 04dc8490d2..b1b71177e0 100644
---
a/test/src/main/java/org/apache/accumulo/test/functional/ManagerAssignmentIT.java
+++
b/test/src/main/java/org/apache/accumulo/test/functional/ManagerAssignmentIT.java
@@ -25,10 +25,10 @@ import static org.junit.jupiter.api.Assertions.fail;
import java.time.Duration;
import java.util.Collections;
-import java.util.Map.Entry;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
+import java.util.concurrent.atomic.AtomicInteger;
import org.apache.accumulo.core.Constants;
import org.apache.accumulo.core.client.Accumulo;
@@ -39,11 +39,9 @@ import org.apache.accumulo.core.client.BatchWriter;
import org.apache.accumulo.core.client.IsolatedScanner;
import org.apache.accumulo.core.client.admin.Locations;
import org.apache.accumulo.core.clientImpl.ClientContext;
-import org.apache.accumulo.core.data.Key;
import org.apache.accumulo.core.data.Mutation;
import org.apache.accumulo.core.data.Range;
import org.apache.accumulo.core.data.TableId;
-import org.apache.accumulo.core.data.Value;
import org.apache.accumulo.core.fate.zookeeper.ServiceLock;
import org.apache.accumulo.core.metadata.MetadataTable;
import org.apache.accumulo.core.metadata.RootTable;
@@ -142,25 +140,21 @@ public class ManagerAssignmentIT extends
AccumuloClusterHarness {
final CountDownLatch latch = new CountDownLatch(10);
- Runnable task = new Runnable() {
- @Override
- public void run() {
- while (true) {
- try (var scanner = new
IsolatedScanner(client.createScanner(tableName))) {
- // TODO maybe do not close scanner? The following limit was
placed on the stream to
- // avoid reading all the data possibly leaving a scan session
active on the tserver
- int count = 0;
- for (Entry<Key,Value> e : scanner) {
- count++;
- // let the test thread know that this thread has read some data
- if (count == 1_000) {
- latch.countDown();
- }
+ Runnable task = () -> {
+ while (true) {
+ try (var scanner = new
IsolatedScanner(client.createScanner(tableName))) {
+ // TODO maybe do not close scanner? The following limit was placed
on the stream to
+ // avoid reading all the data possibly leaving a scan session
active on the tserver
+ AtomicInteger count = new AtomicInteger(0);
+ scanner.forEach(e -> {
+ // let the test thread know that this thread has read some data
+ if (count.incrementAndGet() == 1_000) {
+ latch.countDown();
}
- } catch (Exception e) {
- e.printStackTrace();
- break;
- }
+ });
+ } catch (Exception e) {
+ e.printStackTrace();
+ break;
}
}
};
@@ -218,7 +212,7 @@ public class ManagerAssignmentIT extends
AccumuloClusterHarness {
try (AccumuloClient client =
Accumulo.newClient().from(getClientProps()).build()) {
- Wait.waitFor(() -> client.instanceOperations().getTabletServers().size()
== 1);
+ Wait.waitFor(() -> client.instanceOperations().getTabletServers().size()
== 1, 60_000);
client.instanceOperations().waitForBalance();
diff --git
a/test/src/main/java/org/apache/accumulo/test/functional/WALSunnyDayIT.java
b/test/src/main/java/org/apache/accumulo/test/functional/WALSunnyDayIT.java
index d0c4d8c606..22dc863d0b 100644
--- a/test/src/main/java/org/apache/accumulo/test/functional/WALSunnyDayIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/functional/WALSunnyDayIT.java
@@ -62,6 +62,7 @@ import org.apache.accumulo.server.ServerContext;
import org.apache.accumulo.server.log.WalStateManager;
import org.apache.accumulo.server.log.WalStateManager.WalMarkerException;
import org.apache.accumulo.server.log.WalStateManager.WalState;
+import org.apache.accumulo.test.util.Wait;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.RawLocalFileSystem;
@@ -241,7 +242,7 @@ public class WALSunnyDayIT extends ConfigurableMacBase {
return wals;
}
- int waitLonger = getWaitFactor();
+ int waitLonger = Wait.getTimeoutFactor(e -> 1); // default to 1
for (int i = 1; i <= TIMES_TO_COUNT; i++) {
Thread.sleep(i * PAUSE_BETWEEN_COUNTS * waitLonger);
wals = _getWals(c);
@@ -255,18 +256,6 @@ public class WALSunnyDayIT extends ConfigurableMacBase {
return new HashMap<>();
}
- private int getWaitFactor() {
- int waitLonger = 1;
- String timeoutString = System.getProperty("timeout.factor");
- if (timeoutString != null && !timeoutString.isEmpty()) {
- int timeout = Integer.parseInt(timeoutString);
- if (timeout > 1) {
- waitLonger = timeout;
- }
- }
- return waitLonger;
- }
-
static Map<String,WalState> _getWals(ServerContext c) throws Exception {
while (true) {
try {
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 7352d4a4cf..c7a470c70b 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
@@ -18,13 +18,39 @@
*/
package org.apache.accumulo.test.util;
-import java.util.concurrent.TimeUnit;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static java.util.concurrent.TimeUnit.SECONDS;
+
+import java.util.function.ToIntFunction;
public class Wait {
- public static final long MAX_WAIT_MILLIS = TimeUnit.SECONDS.toMillis(30);
+ public static final long MAX_WAIT_MILLIS = SECONDS.toMillis(30);
public static final long SLEEP_MILLIS = 1000;
+ /**
+ * Get the user-specified timeout.factor value from the system properties.
The parsed value must
+ * be a valid integer greater-than-or-equal-to 1. On a parse error,
including parsing values less
+ * than 1, the caller can handle the error and substitute in a different
value, which could be any
+ * integer (even less than 1).
+ *
+ * @param onError allows parse exceptions to be detected and the value
replaced with a substitute
+ * @return the parsed value or the value from the onError function, if an
error occurred
+ */
+ public static int getTimeoutFactor(ToIntFunction<NumberFormatException>
onError) {
+ String timeoutString = System.getProperty("timeout.factor");
+ try {
+ int factor = Integer.parseInt(timeoutString);
+ if (factor < 1) {
+ throw new NumberFormatException("timeout.factor must be at least 1");
+ }
+ return factor;
+ } catch (NumberFormatException e) {
+ return onError.applyAsInt(e);
+ }
+
+ }
+
public interface Condition {
boolean isSatisfied() throws Exception;
}
@@ -33,7 +59,7 @@ public class Wait {
* 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
+ * @param condition when condition evaluates true, return from wait
*/
public static void waitFor(Condition condition) {
waitFor(condition, MAX_WAIT_MILLIS);
@@ -43,7 +69,7 @@ public class Wait {
* 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 condition when condition evaluates true, return from wait
* @param duration maximum total time to wait (milliseconds)
*/
public static void waitFor(final Condition condition, final long duration) {
@@ -54,7 +80,7 @@ public class Wait {
* 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 condition when condition evaluates true, return from wait
* @param duration maximum total time to wait (milliseconds)
* @param sleepMillis time to sleep between condition checks
*/
@@ -67,7 +93,7 @@ public class Wait {
* 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 condition when condition evaluates true, 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
@@ -76,12 +102,16 @@ public class Wait {
public static void waitFor(final Condition condition, final long duration,
final long sleepMillis,
final String failMessage) {
- final long expiry = System.currentTimeMillis() + duration;
+ final int timeoutFactor = getTimeoutFactor(e -> 1); // default to factor
of 1
+ final long scaledDurationNanos = MILLISECONDS.toNanos(duration) *
timeoutFactor;
+ final long scaledSleepMillis = sleepMillis * timeoutFactor;
+
+ final long startNanos = System.nanoTime();
boolean success;
try {
success = condition.isSatisfied();
- while (!success && System.currentTimeMillis() < expiry) {
- TimeUnit.MILLISECONDS.sleep(sleepMillis);
+ while (!success && System.nanoTime() - startNanos < scaledDurationNanos)
{
+ MILLISECONDS.sleep(scaledSleepMillis);
success = condition.isSatisfied();
}
} catch (InterruptedException ex) {
