Browse Source

Use `TimeValue` for timeouts in `safeAwait` etc. (#126509) (#126711)

There's no need to force callers to deconstruct the `TimeValue` in their
possession into a `long` and a `TimeUnit`, we can do it ourselves.
David Turner 6 months ago
parent
commit
abfbe1d63f

+ 10 - 8
test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java

@@ -2345,8 +2345,9 @@ public abstract class ESTestCase extends LuceneTestCase {
      * complete are a big drag on CI times which slows everyone down.
      * <p>
      * For instance, tests which verify things that require the passage of time ought to simulate this (e.g. using a {@link
-     * org.elasticsearch.common.util.concurrent.DeterministicTaskQueue}). Excessive busy-waits ought to be replaced by blocking waits (e.g.
-     * using a {@link CountDownLatch}) which release as soon as the condition is satisfied.
+     * org.elasticsearch.common.util.concurrent.DeterministicTaskQueue}). Excessive busy-waits ought to be replaced by blocking waits. For
+     * instance, use a {@link CountDownLatch} or {@link CyclicBarrier} or similar to continue execution as soon as a condition is satisfied.
+     * To wait for a particular cluster state, use {@link ClusterServiceUtils#addTemporaryStateListener} rather than busy-waiting on an API.
      */
     public static final TimeValue SAFE_AWAIT_TIMEOUT = TimeValue.timeValueSeconds(10);
 
@@ -2424,7 +2425,7 @@ public abstract class ESTestCase extends LuceneTestCase {
      * @return The value with which the {@code listener} was completed.
      */
     public static <T> T safeAwait(SubscribableListener<T> listener) {
-        return safeAwait(listener, SAFE_AWAIT_TIMEOUT.getMillis(), TimeUnit.MILLISECONDS);
+        return safeAwait(listener, SAFE_AWAIT_TIMEOUT);
     }
 
     /**
@@ -2433,10 +2434,10 @@ public abstract class ESTestCase extends LuceneTestCase {
      *
      * @return The value with which the {@code listener} was completed.
      */
-    public static <T> T safeAwait(SubscribableListener<T> listener, long timeout, TimeUnit unit) {
+    public static <T> T safeAwait(SubscribableListener<T> listener, TimeValue timeout) {
         final var future = new TestPlainActionFuture<T>();
         listener.addListener(future);
-        return safeGet(future, timeout, unit);
+        return safeGet(future, timeout);
     }
 
     /**
@@ -2466,7 +2467,7 @@ public abstract class ESTestCase extends LuceneTestCase {
      * @return The value with which the {@code future} was completed.
      */
     public static <T> T safeGet(Future<T> future) {
-        return safeGet(future, SAFE_AWAIT_TIMEOUT.millis(), TimeUnit.MILLISECONDS);
+        return safeGet(future, SAFE_AWAIT_TIMEOUT);
     }
 
     /**
@@ -2475,9 +2476,10 @@ public abstract class ESTestCase extends LuceneTestCase {
      *
      * @return The value with which the {@code future} was completed.
      */
-    public static <T> T safeGet(Future<T> future, long timeout, TimeUnit unit) {
+    // NB private because tests should be designed not to need to wait for longer than SAFE_AWAIT_TIMEOUT.
+    private static <T> T safeGet(Future<T> future, TimeValue timeout) {
         try {
-            return future.get(timeout, unit);
+            return future.get(timeout.millis(), TimeUnit.MILLISECONDS);
         } catch (InterruptedException e) {
             Thread.currentThread().interrupt();
             throw new AssertionError("safeGet: interrupted waiting for SubscribableListener", e);

+ 1 - 2
x-pack/plugin/downsample/src/internalClusterTest/java/org/elasticsearch/xpack/downsample/DataStreamLifecycleDownsampleDisruptionIT.java

@@ -29,7 +29,6 @@ import org.elasticsearch.xpack.core.LocalStateCompositeXPackPlugin;
 
 import java.util.Collection;
 import java.util.List;
-import java.util.concurrent.TimeUnit;
 
 import static org.elasticsearch.cluster.metadata.IndexMetadata.INDEX_DOWNSAMPLE_STATUS;
 import static org.elasticsearch.xpack.downsample.DataStreamLifecycleDriver.getBackingIndices;
@@ -116,6 +115,6 @@ public class DataStreamLifecycleDownsampleDisruptionIT extends ESIntegTestCase {
             }
             return false;
         });
-        safeAwait(listener, timeout.millis(), TimeUnit.MILLISECONDS);
+        safeAwait(listener, timeout);
     }
 }