Throw StorageException instead of IOException from Indexers

This also means we can remove our usage of CheckedExceptions.

Change-Id: I84b7c42f6020e097401509134fe9473eef633e51

java/com/google/gerrit/server/index/IndexUtils.java

@@ -18,7 +18,6 @@ import static com.google.gerrit.server.index.change.ChangeField.CHANGE;
 import static com.google.gerrit.server.index.change.ChangeField.LEGACY_ID;
 import static com.google.gerrit.server.index.change.ChangeField.PROJECT;
 
-import com.google.common.base.Function;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
@@ -32,24 +31,12 @@ import com.google.gerrit.server.index.group.GroupField;
 import com.google.gerrit.server.query.change.SingleGroupUser;
 import java.io.IOException;
 import java.util.Set;
-import java.util.concurrent.ExecutionException;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 
 public final class IndexUtils {
   public static final ImmutableMap<String, String> CUSTOM_CHAR_MAPPING =
       ImmutableMap.of("_", " ", ".", " ");
 
-  public static final Function<Exception, IOException> MAPPER =
-      in -> {
-        if (in instanceof IOException) {
-          return (IOException) in;
-        } else if (in instanceof ExecutionException && in.getCause() instanceof IOException) {
-          return (IOException) in.getCause();
-        } else {
-          return new IOException(in);
-        }
-      };
-
   public static void setReady(SitePaths sitePaths, String name, int version, boolean ready) {
     try {
       GerritIndexStatus cfg = new GerritIndexStatus(sitePaths);

java/com/google/gerrit/server/index/account/AccountIndexer.java

@@ -15,7 +15,6 @@
 package com.google.gerrit.server.index.account;
 
 import com.google.gerrit.reviewdb.client.Account;
-import java.io.IOException;
 
 public interface AccountIndexer {
 
@@ -24,7 +23,7 @@ public interface AccountIndexer {
    *
    * @param id account id to index.
    */
-  void index(Account.Id id) throws IOException;
+  void index(Account.Id id);
 
   /**
    * Synchronously reindex an account if it is stale.
@@ -32,5 +31,5 @@ public interface AccountIndexer {
    * @param id account id to index.
    * @return whether the account was reindexed
    */
-  boolean reindexIfStale(Account.Id id) throws IOException;
+  boolean reindexIfStale(Account.Id id);
 }

java/com/google/gerrit/server/index/account/AccountIndexerImpl.java

@@ -17,6 +17,7 @@ package com.google.gerrit.server.index.account;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.common.Nullable;
+import com.google.gerrit.exceptions.StorageException;
 import com.google.gerrit.extensions.events.AccountIndexedListener;
 import com.google.gerrit.index.Index;
 import com.google.gerrit.reviewdb.client.Account;
@@ -74,7 +75,7 @@ public class AccountIndexerImpl implements AccountIndexer {
   }
 
   @Override
-  public void index(Account.Id id) throws IOException {
+  public void index(Account.Id id) {
     byIdCache.evict(id);
     Optional<AccountState> accountState = byIdCache.get(id);
 
@@ -104,10 +105,14 @@ public class AccountIndexerImpl implements AccountIndexer {
   }
 
   @Override
-  public boolean reindexIfStale(Account.Id id) throws IOException {
-    if (stalenessChecker.isStale(id)) {
-      index(id);
-      return true;
+  public boolean reindexIfStale(Account.Id id) {
+    try {
+      if (stalenessChecker.isStale(id)) {
+        index(id);
+        return true;
+      }
+    } catch (IOException e) {
+      throw new StorageException(e);
     }
     return false;
   }

java/com/google/gerrit/server/index/change/ChangeIndexer.java

@@ -27,7 +27,6 @@ import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.index.IndexExecutor;
-import com.google.gerrit.server.index.IndexUtils;
 import com.google.gerrit.server.logging.TraceContext;
 import com.google.gerrit.server.logging.TraceContext.TraceTimer;
 import com.google.gerrit.server.plugincontext.PluginSetContext;
@@ -37,7 +36,6 @@ import com.google.gerrit.server.util.ThreadLocalRequestContext;
 import com.google.inject.OutOfScopeException;
 import com.google.inject.assistedinject.Assisted;
 import com.google.inject.assistedinject.AssistedInject;
-import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -62,16 +60,6 @@ public class ChangeIndexer {
     ChangeIndexer create(ListeningExecutorService executor, ChangeIndexCollection indexes);
   }
 
-  @SuppressWarnings("deprecation")
-  public static com.google.common.util.concurrent.CheckedFuture<?, IOException> allAsList(
-      List<? extends ListenableFuture<?>> futures) {
-    // allAsList propagates the first seen exception, wrapped in
-    // ExecutionException, so we can reuse the same mapper as for a single
-    // future. Assume the actual contents of the exception are not useful to
-    // callers. All exceptions are already logged by IndexTask.
-    return Futures.makeChecked(Futures.allAsList(futures), IndexUtils.MAPPER);
-  }
-
   @Nullable private final ChangeIndexCollection indexes;
   @Nullable private final ChangeIndex index;
   private final ChangeData.Factory changeDataFactory;
@@ -134,9 +122,7 @@ public class ChangeIndexer {
    * @param id change to index.
    * @return future for the indexing task.
    */
-  @SuppressWarnings("deprecation")
-  public com.google.common.util.concurrent.CheckedFuture<?, IOException> indexAsync(
-      Project.NameKey project, Change.Id id) {
+  public ListenableFuture<?> indexAsync(Project.NameKey project, Change.Id id) {
     return submit(new IndexTask(project, id));
   }
 
@@ -146,14 +132,12 @@ public class ChangeIndexer {
    * @param ids changes to index.
    * @return future for completing indexing of all changes.
    */
-  @SuppressWarnings("deprecation")
-  public com.google.common.util.concurrent.CheckedFuture<?, IOException> indexAsync(
-      Project.NameKey project, Collection<Change.Id> ids) {
+  public ListenableFuture<?> indexAsync(Project.NameKey project, Collection<Change.Id> ids) {
     List<ListenableFuture<?>> futures = new ArrayList<>(ids.size());
     for (Change.Id id : ids) {
       futures.add(indexAsync(project, id));
     }
-    return allAsList(futures);
+    return Futures.allAsList(futures);
   }
 
   /**
@@ -161,7 +145,7 @@ public class ChangeIndexer {
    *
    * @param cd change to index.
    */
-  public void index(ChangeData cd) throws IOException {
+  public void index(ChangeData cd) {
     indexImpl(cd);
 
     // Always double-check whether the change might be stale immediately after
@@ -185,7 +169,7 @@ public class ChangeIndexer {
     autoReindexIfStale(cd);
   }
 
-  private void indexImpl(ChangeData cd) throws IOException {
+  private void indexImpl(ChangeData cd) {
     logger.atFine().log("Replace change %d in index.", cd.getId().get());
     for (Index<?, ChangeData> i : getWriteIndexes()) {
       try (TraceTimer traceTimer =
@@ -211,7 +195,7 @@ public class ChangeIndexer {
    *
    * @param change change to index.
    */
-  public void index(Change change) throws IOException {
+  public void index(Change change) {
     index(changeDataFactory.create(change));
   }
 
@@ -221,7 +205,7 @@ public class ChangeIndexer {
    * @param project the project to which the change belongs.
    * @param changeId ID of the change to index.
    */
-  public void index(Project.NameKey project, Change.Id changeId) throws IOException {
+  public void index(Project.NameKey project, Change.Id changeId) {
     index(changeDataFactory.create(project, changeId));
   }
 
@@ -231,8 +215,7 @@ public class ChangeIndexer {
    * @param id change to delete.
    * @return future for the deleting task.
    */
-  @SuppressWarnings("deprecation")
-  public com.google.common.util.concurrent.CheckedFuture<?, IOException> deleteAsync(Change.Id id) {
+  public ListenableFuture<?> deleteAsync(Change.Id id) {
     return submit(new DeleteTask(id));
   }
 
@@ -241,7 +224,7 @@ public class ChangeIndexer {
    *
    * @param id change ID to delete.
    */
-  public void delete(Change.Id id) throws IOException {
+  public void delete(Change.Id id) {
     new DeleteTask(id).call();
   }
 
@@ -255,9 +238,7 @@ public class ChangeIndexer {
    * @param id ID of the change to index.
    * @return future for reindexing the change; returns true if the change was stale.
    */
-  @SuppressWarnings("deprecation")
-  public com.google.common.util.concurrent.CheckedFuture<Boolean, IOException> reindexIfStale(
-      Project.NameKey project, Change.Id id) {
+  public ListenableFuture<Boolean> reindexIfStale(Project.NameKey project, Change.Id id) {
     return submit(new ReindexIfStaleTask(project, id), batchExecutor);
   }
 
@@ -277,17 +258,13 @@ public class ChangeIndexer {
     return indexes != null ? indexes.getWriteIndexes() : Collections.singleton(index);
   }
 
-  @SuppressWarnings("deprecation")
-  private <T> com.google.common.util.concurrent.CheckedFuture<T, IOException> submit(
-      Callable<T> task) {
+  private <T> ListenableFuture<T> submit(Callable<T> task) {
     return submit(task, executor);
   }
 
-  @SuppressWarnings("deprecation")
-  private static <T> com.google.common.util.concurrent.CheckedFuture<T, IOException> submit(
+  private static <T> ListenableFuture<T> submit(
       Callable<T> task, ListeningExecutorService executor) {
-    return Futures.makeChecked(
-        Futures.nonCancellationPropagating(executor.submit(task)), IndexUtils.MAPPER);
+    return Futures.nonCancellationPropagating(executor.submit(task));
   }
 
   private abstract class AbstractIndexTask<T> implements Callable<T> {
@@ -351,7 +328,7 @@ public class ChangeIndexer {
     }
 
     @Override
-    public Void call() throws IOException {
+    public Void call() {
       logger.atFine().log("Delete change %d from index.", id.get());
       // Don't bother setting a RequestContext to provide the DB.
       // Implementations should not need to access the DB in order to delete a

java/com/google/gerrit/server/index/change/ReindexAfterRefUpdate.java

@@ -90,12 +90,8 @@ public class ReindexAfterRefUpdate implements GitReferenceUpdatedListener {
     if (allUsersName.get().equals(event.getProjectName())) {
       Account.Id accountId = Account.Id.fromRef(event.getRefName());
       if (accountId != null && !event.getRefName().startsWith(RefNames.REFS_STARRED_CHANGES)) {
-        try {
-          accountCache.evict(accountId);
-          indexer.get().index(accountId);
-        } catch (IOException e) {
-          logger.atSevere().withCause(e).log("Reindex account %s failed.", accountId);
-        }
+        accountCache.evict(accountId);
+        indexer.get().index(accountId);
       }
     }
 

java/com/google/gerrit/server/index/group/GroupIndexer.java

@@ -15,7 +15,6 @@
 package com.google.gerrit.server.index.group;
 
 import com.google.gerrit.reviewdb.client.AccountGroup;
-import java.io.IOException;
 
 public interface GroupIndexer {
 
@@ -24,7 +23,7 @@ public interface GroupIndexer {
    *
    * @param uuid group UUID to index.
    */
-  void index(AccountGroup.UUID uuid) throws IOException;
+  void index(AccountGroup.UUID uuid);
 
   /**
    * Synchronously reindex a group if it is stale.
@@ -32,5 +31,5 @@ public interface GroupIndexer {
    * @param uuid group UUID to index.
    * @return whether the group was reindexed
    */
-  boolean reindexIfStale(AccountGroup.UUID uuid) throws IOException;
+  boolean reindexIfStale(AccountGroup.UUID uuid);
 }

java/com/google/gerrit/server/index/group/GroupIndexerImpl.java

@@ -17,6 +17,7 @@ package com.google.gerrit.server.index.group;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.common.Nullable;
+import com.google.gerrit.exceptions.StorageException;
 import com.google.gerrit.extensions.events.GroupIndexedListener;
 import com.google.gerrit.index.Index;
 import com.google.gerrit.reviewdb.client.AccountGroup;
@@ -74,7 +75,7 @@ public class GroupIndexerImpl implements GroupIndexer {
   }
 
   @Override
-  public void index(AccountGroup.UUID uuid) throws IOException {
+  public void index(AccountGroup.UUID uuid) {
     // Evict the cache to get an up-to-date value for sure.
     groupCache.evict(uuid);
     Optional<InternalGroup> internalGroup = groupCache.get(uuid);
@@ -104,10 +105,14 @@ public class GroupIndexerImpl implements GroupIndexer {
   }
 
   @Override
-  public boolean reindexIfStale(AccountGroup.UUID uuid) throws IOException {
-    if (stalenessChecker.isStale(uuid)) {
-      index(uuid);
-      return true;
+  public boolean reindexIfStale(AccountGroup.UUID uuid) {
+    try {
+      if (stalenessChecker.isStale(uuid)) {
+        index(uuid);
+        return true;
+      }
+    } catch (IOException e) {
+      throw new StorageException(e);
     }
     return false;
   }

java/com/google/gerrit/server/index/project/ProjectIndexerImpl.java

@@ -30,7 +30,6 @@ import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.project.ProjectState;
 import com.google.inject.assistedinject.Assisted;
 import com.google.inject.assistedinject.AssistedInject;
-import java.io.IOException;
 import java.util.Collection;
 import java.util.Collections;
 
@@ -71,7 +70,7 @@ public class ProjectIndexerImpl implements ProjectIndexer {
   }
 
   @Override
-  public void index(Project.NameKey nameKey) throws IOException {
+  public void index(Project.NameKey nameKey) {
     ProjectState projectState = projectCache.get(nameKey);
     if (projectState != null) {
       logger.atFine().log("Replace project %s in index", nameKey.get());

java/com/google/gerrit/server/restapi/project/Index.java

@@ -71,18 +71,10 @@ public class Index implements RestModifyView<ProjectResource, IndexProjectInput>
     return Response.accepted(response);
   }
 
-  private void reindex(Project.NameKey project, Boolean async) throws IOException {
+  private void reindex(Project.NameKey project, Boolean async) {
     if (Boolean.TRUE.equals(async)) {
       @SuppressWarnings("unused")
-      Future<?> possiblyIgnoredError =
-          executor.submit(
-              () -> {
-                try {
-                  indexer.index(project);
-                } catch (IOException e) {
-                  logger.atWarning().withCause(e).log("reindexing project %s failed", project);
-                }
-              });
+      Future<?> possiblyIgnoredError = executor.submit(() -> indexer.index(project));
     } else {
       indexer.index(project);
     }

java/com/google/gerrit/server/update/BatchUpdate.java

@@ -29,6 +29,8 @@ import com.google.common.collect.ListMultimap;
 import com.google.common.collect.MultimapBuilder;
 import com.google.common.collect.Multiset;
 import com.google.common.flogger.FluentLogger;
+import com.google.common.util.concurrent.Futures;
+import com.google.common.util.concurrent.ListenableFuture;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.extensions.api.changes.NotifyHandling;
 import com.google.gerrit.extensions.config.FactoryModule;
@@ -124,9 +126,7 @@ public class BatchUpdate implements AutoCloseable {
     checkDifferentProject(updates);
 
     try {
-      @SuppressWarnings("deprecation")
-      List<com.google.common.util.concurrent.CheckedFuture<?, IOException>> indexFutures =
-          new ArrayList<>();
+      List<ListenableFuture<?>> indexFutures = new ArrayList<>();
       List<ChangesHandle> handles = new ArrayList<>(updates.size());
       try {
         for (BatchUpdate u : updates) {
@@ -148,7 +148,7 @@ public class BatchUpdate implements AutoCloseable {
         }
       }
 
-      ChangeIndexer.allAsList(indexFutures).get();
+      ((ListenableFuture<?>) Futures.allAsList(indexFutures)).get();
 
       // Fire ref update events only after all mutations are finished, since callers may assume a
       // patch set ref being created means the change was created, or a branch advancing meaning
@@ -504,13 +504,12 @@ public class BatchUpdate implements AutoCloseable {
     }
 
     @SuppressWarnings("deprecation")
-    List<com.google.common.util.concurrent.CheckedFuture<?, IOException>> startIndexFutures() {
+    List<ListenableFuture<?>> startIndexFutures() {
       if (dryrun) {
         return ImmutableList.of();
       }
       logDebug("Reindexing %d changes", results.size());
-      List<com.google.common.util.concurrent.CheckedFuture<?, IOException>> indexFutures =
-          new ArrayList<>(results.size());
+      List<ListenableFuture<?>> indexFutures = new ArrayList<>(results.size());
       for (Map.Entry<Change.Id, ChangeResult> e : results.entrySet()) {
         Change.Id id = e.getKey();
         switch (e.getValue()) {