RetryHelper: Refactor to take multiple options

Pass an AutoValue Options arg in instead of one (and soon more than one)
nullable argument. The Options arg is a slightly more user-friendly
wrapper around upstream's RetryerHelper.

Use this approach instead of making RetryHelper a non-singleton and
using a more builder-ish approach for RetryHelper itself. This would
require much more churn at call sites, the vast majority of which are
fine with the defaults.

Change-Id: I285843940c06d9e520b611d9bfa5053634682ce1

gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java

@@ -491,7 +491,7 @@ public class MergeOp implements AutoCloseable {
             }
             return null;
           },
-          retryTracker);
+          RetryHelper.options().listener(retryTracker).build());
 
       if (projects > 1) {
         topicMetrics.topicSubmissionsCompleted.increment();

gerrit-server/src/main/java/com/google/gerrit/server/update/RetryHelper.java

@@ -24,6 +24,7 @@ import com.github.rholder.retry.StopStrategies;
 import com.github.rholder.retry.StopStrategy;
 import com.github.rholder.retry.WaitStrategies;
 import com.github.rholder.retry.WaitStrategy;
+import com.google.auto.value.AutoValue;
 import com.google.common.base.Throwables;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.extensions.restapi.RestApiException;
@@ -41,6 +42,41 @@ public class RetryHelper {
     T call(BatchUpdate.Factory updateFactory) throws Exception;
   }
 
+  /**
+   * Options for retrying a single operation.
+   *
+   * <p>This class is similar in function to upstream's {@link RetryerBuilder}, but it exists as its
+   * own class in Gerrit for several reasons:
+   *
+   * <ul>
+   *   <li>Gerrit needs to support defaults for some of the options, such as a default timeout.
+   *       {@code RetryerBuilder} doesn't support calling the same setter multiple times, so doing
+   *       this with {@code RetryerBuilder} directly would not be easy.
+   *   <li>Gerrit explicitly does not want callers to have full control over all possible options,
+   *       so this class exposes a curated subset.
+   * </ul>
+   */
+  @AutoValue
+  public abstract static class Options {
+    @Nullable
+    abstract RetryListener listener();
+
+    @AutoValue.Builder
+    public abstract static class Builder {
+      public abstract Builder listener(RetryListener listener);
+
+      public abstract Options build();
+    }
+  }
+
+  public static Options.Builder options() {
+    return new AutoValue_RetryHelper_Options.Builder();
+  }
+
+  public static Options defaults() {
+    return options().build();
+  }
+
   private final NotesMigration migration;
   private final BatchUpdate.Factory updateFactory;
   private final StopStrategy stopStrategy;
@@ -68,11 +104,10 @@ public class RetryHelper {
   }
 
   public <T> T execute(Action<T> action) throws RestApiException, UpdateException {
-    return execute(action, null);
+    return execute(action, defaults());
   }
 
-  public <T> T execute(Action<T> action, @Nullable RetryListener listener)
-      throws RestApiException, UpdateException {
+  public <T> T execute(Action<T> action, Options opts) throws RestApiException, UpdateException {
     try {
       RetryerBuilder<T> builder = RetryerBuilder.newBuilder();
       if (migration.disableChangeReviewDb()) {
@@ -80,8 +115,8 @@ public class RetryHelper {
             .withStopStrategy(stopStrategy)
             .withWaitStrategy(waitStrategy)
             .retryIfException(RetryHelper::isLockFailure);
-        if (listener != null) {
-          builder.withRetryListener(listener);
+        if (opts.listener() != null) {
+          builder.withRetryListener(opts.listener());
         }
       } else {
         // Either we aren't full-NoteDb, or the underlying ref storage doesn't support atomic

plugins/reviewnotes

@@ -1 +1 @@
-Subproject commit be803eb40fdcd5bfa11d9a808863585f86228e06
+Subproject commit fe99eb4399054d3fd67ff2330aa92841d76e53bb