Flakiness in LB2SolrClientTest.testTwoServers (#3937)

Author: janhoy

solr/solrj/src/java/org/apache/solr/common/util/RetryUtil.java

@@ -25,23 +25,67 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+/** Utility class for retrying operations with various strategies. */
 public class RetryUtil {
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
+  /**
+   * Interface for commands that can be retried and may throw exceptions.
+   *
+   * <p>Implementations should define the operation to be executed in the {@link #execute()} method.
+   */
   public interface RetryCmd {
     void execute() throws Exception;
   }
 
+  /**
+   * Interface for commands that return a boolean result indicating success or failure.
+   *
+   * <p>Implementations should return {@code true} when the operation succeeds, or {@code false}
+   * when it should be retried.
+   */
   public interface BooleanRetryCmd {
     boolean execute();
   }
 
+  /**
+   * Retries a command when a specific exception type occurs, until successful or timeout is
+   * reached.
+   *
+   * <p>This is a convenience method that delegates to {@link #retryOnException(Set, long, long,
+   * RetryCmd)} with a singleton set containing the specified exception class.
+   *
+   * @param clazz the exception class to retry on
+   * @param timeoutms maximum time to retry in milliseconds
+   * @param intervalms wait interval between retries in milliseconds
+   * @param cmd the command to execute
+   * @throws Exception if the command fails with a different exception, or if timeout is reached
+   * @throws InterruptedException if the thread is interrupted while sleeping between retries
+   */
   public static void retryOnException(
       Class<? extends Exception> clazz, long timeoutms, long intervalms, RetryCmd cmd)
       throws Exception {
     retryOnException(Collections.singleton(clazz), timeoutms, intervalms, cmd);
   }
 
+  /**
+   * Retries a command when any of the specified exception types occur, until successful or timeout
+   * is reached.
+   *
+   * <p>The command is executed repeatedly until it succeeds (completes without throwing an
+   * exception) or the timeout is reached. If an exception matching one of the specified classes is
+   * thrown and there is time remaining, the method will sleep for {@code intervalms} milliseconds
+   * before retrying. If a different exception is thrown, or if the timeout is reached, the
+   * exception is rethrown.
+   *
+   * @param classes set of exception classes to retry on
+   * @param timeoutms maximum time to retry in milliseconds
+   * @param intervalms wait interval between retries in milliseconds
+   * @param cmd the command to execute
+   * @throws Exception if the command fails with an exception not in the specified set, or if
+   *     timeout is reached
+   * @throws InterruptedException if the thread is interrupted while sleeping between retries
+   */
   public static void retryOnException(
       Set<Class<? extends Exception>> classes, long timeoutms, long intervalms, RetryCmd cmd)
       throws Exception {
@@ -74,6 +118,22 @@ private static boolean isInstanceOf(Set<Class<? extends Exception>> classes, Thr
     return false;
   }
 
+  /**
+   * Retries a command until it returns {@code true} or the maximum number of retries is reached.
+   *
+   * <p>The command is executed up to {@code retries} times. After each failed attempt (when the
+   * command returns {@code false}), the method pauses for the specified duration before retrying.
+   * If all retries are exhausted without success, a {@link SolrException} is thrown with the
+   * provided error message.
+   *
+   * @param errorMessage the error message to use if all retries are exhausted
+   * @param retries maximum number of retry attempts
+   * @param pauseTime duration to pause between retries
+   * @param pauseUnit time unit for the pause duration
+   * @param cmd the command to execute
+   * @throws InterruptedException if the thread is interrupted while sleeping between retries
+   * @throws SolrException if all retries are exhausted without success
+   */
   public static void retryUntil(
       String errorMessage, int retries, long pauseTime, TimeUnit pauseUnit, BooleanRetryCmd cmd)
       throws InterruptedException {
@@ -84,12 +144,32 @@ public static void retryUntil(
     throw new SolrException(ErrorCode.SERVER_ERROR, errorMessage);
   }
 
+  /**
+   * Retries a command until it returns {@code true} or the timeout is reached.
+   *
+   * <p>The command is executed repeatedly until it returns {@code true} or the timeout expires. If
+   * the command returns {@code false} and there is time remaining, the method sleeps for {@code
+   * intervalms} milliseconds before retrying. If the timeout is reached before the command
+   * succeeds, a {@link SolrException} is thrown.
+   *
+   * @param timeoutms maximum time to retry in milliseconds
+   * @param intervalms wait interval between retries in milliseconds
+   * @param cmd the command to execute
+   * @throws SolrException if no success within timeout
+   */
   public static void retryOnBoolean(long timeoutms, long intervalms, BooleanRetryCmd cmd) {
     long timeout =
         System.nanoTime() + TimeUnit.NANOSECONDS.convert(timeoutms, TimeUnit.MILLISECONDS);
     while (true) {
       boolean resp = cmd.execute();
       if (!resp && System.nanoTime() < timeout) {
+        try {
+          //noinspection BusyWait
+          Thread.sleep(intervalms);
+        } catch (InterruptedException e) {
+          Thread.currentThread().interrupt();
+          throw new SolrException(ErrorCode.SERVER_ERROR, "Interrupted while retrying operation");
+        }
         continue;
       } else if (System.nanoTime() >= timeout) {
         throw new SolrException(ErrorCode.SERVER_ERROR, "Timed out while retrying operation");

solr/solrj/src/test/org/apache/solr/client/solrj/impl/LB2SolrClientTest.java

@@ -35,6 +35,7 @@
 import org.apache.solr.client.solrj.response.QueryResponse;
 import org.apache.solr.client.solrj.response.SolrResponseBase;
 import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.util.RetryUtil;
 import org.apache.solr.embedded.JettyConfig;
 import org.apache.solr.embedded.JettySolrRunner;
 import org.apache.solr.util.LogLevel;
@@ -196,9 +197,23 @@ public void testTwoServers() throws Exception {
       solr[1].jetty = null;
       startJettyAndWaitForAliveCheckQuery(solr[0]);
 
-      resp = h.lbClient.query(solrQuery);
-      name = resp.getResults().get(0).getFieldValue("name").toString();
-      assertEquals("solr/collection10", name);
+      RetryUtil.retryOnBoolean(
+          5000,
+          500,
+          () -> {
+            try {
+              return ("solr/collection10"
+                  .equals(
+                      h.lbClient
+                          .query(solrQuery)
+                          .getResults()
+                          .get(0)
+                          .getFieldValue("name")
+                          .toString()));
+            } catch (Exception e) {
+              return false;
+            }
+          });
     }
   }