commit 3e7e6fc1c5605388d8ea97f4746d9ee6b4bbb0a3
author Edwin Kempin <ekempin@google.com> Thu Jun 07 15:45:28 2018 +0200
committer Edwin Kempin <ekempin@google.com> Mon Jun 11 10:32:15 2018 +0200
tree 358450903d2eb2921114b195ae2bc7adfc6ba1f0
parent 7c3eb65c1a0f47532bcc36660dfb6f20d2eaa1c8

Emails: Retry failed index queries when querying accounts by preferred email

Accessing the secondary index may sometimes fail due to a temporary
issue with the index.

Temporary issues with accessing the index are not common with Lucene but
at Google where we have a custom index implementation we have such
short-term failures from time to time.

Change I59aa974ced implemented retrying request validation during
auto-close because looking up accounts by preferred email could fail.
Since the retry is now done on the lower level in the Emails class,
ReceiveCommits no longer needs to retry the request validation.

Change-Id: I58147ac8d9fa837053504c1de6ff695cc8461ef3
Signed-off-by: Edwin Kempin <ekempin@google.com>

java/com/google/gerrit/server/account/Emails.java

diff --git a/java/com/google/gerrit/server/account/Emails.java b/java/com/google/gerrit/server/account/Emails.java
index 31e845b..8a48167 100644
--- a/java/com/google/gerrit/server/account/Emails.java
+++ b/java/com/google/gerrit/server/account/Emails.java
@@ -16,6 +16,7 @@
 
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 
+import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSetMultimap;
 import com.google.common.collect.Streams;
@@ -23,6 +24,9 @@
 import com.google.gerrit.server.account.externalids.ExternalId;
 import com.google.gerrit.server.account.externalids.ExternalIds;
 import com.google.gerrit.server.query.account.InternalAccountQuery;
+import com.google.gerrit.server.update.RetryHelper;
+import com.google.gerrit.server.update.RetryHelper.Action;
+import com.google.gerrit.server.update.RetryHelper.ActionType;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -34,11 +38,16 @@
 public class Emails {
   private final ExternalIds externalIds;
   private final Provider<InternalAccountQuery> queryProvider;
+  private final RetryHelper retryHelper;
 
   @Inject
-  public Emails(ExternalIds externalIds, Provider<InternalAccountQuery> queryProvider) {
+  public Emails(
+      ExternalIds externalIds,
+      Provider<InternalAccountQuery> queryProvider,
+      RetryHelper retryHelper) {
     this.externalIds = externalIds;
     this.queryProvider = queryProvider;
+    this.retryHelper = retryHelper;
   }
 
   /**
@@ -63,7 +72,8 @@
   public ImmutableSet<Account.Id> getAccountFor(String email) throws IOException, OrmException {
     return Streams.concat(
             externalIds.byEmail(email).stream().map(ExternalId::accountId),
-            queryProvider.get().byPreferredEmail(email).stream().map(a -> a.getAccount().getId()))
+            executeIndexQuery(() -> queryProvider.get().byPreferredEmail(email).stream())
+                .map(a -> a.getAccount().getId()))
         .collect(toImmutableSet());
   }
 
@@ -80,12 +90,18 @@
         .entries()
         .stream()
         .forEach(e -> builder.put(e.getKey(), e.getValue().accountId()));
-    queryProvider
-        .get()
-        .byPreferredEmail(emails)
-        .entries()
-        .stream()
+    executeIndexQuery(() -> queryProvider.get().byPreferredEmail(emails).entries().stream())
         .forEach(e -> builder.put(e.getKey(), e.getValue().getAccount().getId()));
     return builder.build();
   }
+
+  private <T> T executeIndexQuery(Action<T> action) throws OrmException {
+    try {
+      return retryHelper.execute(ActionType.INDEX_QUERY, action, OrmException.class::isInstance);
+    } catch (Exception e) {
+      Throwables.throwIfUnchecked(e);
+      Throwables.throwIfInstanceOf(e, OrmException.class);
+      throw new OrmException(e);
+    }
+  }
 }

java/com/google/gerrit/server/git/receive/ReceiveCommits.java

diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 568b966..1c3747c 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -2973,7 +2973,7 @@
 
               for (ReplaceRequest req : replaceAndClose) {
                 Change.Id id = req.notes.getChangeId();
-                if (!executeRequestValidation(() -> req.validate(true))) {
+                if (!req.validate(true)) {
                   logDebug("Not closing %s because validation failed", id);
                   continue;
                 }
@@ -3017,20 +3017,6 @@
     }
   }
 
-  private <T> T executeRequestValidation(Action<T> action)
-      throws IOException, PermissionBackendException, OrmException {
-    try {
-      // The request validation needs to do an account query to lookup accounts by preferred email,
-      // if that index query fails the request validation should be retried.
-      return retryHelper.execute(ActionType.INDEX_QUERY, action, OrmException.class::isInstance);
-    } catch (Exception t) {
-      Throwables.throwIfInstanceOf(t, IOException.class);
-      Throwables.throwIfInstanceOf(t, PermissionBackendException.class);
-      Throwables.throwIfInstanceOf(t, OrmException.class);
-      throw new OrmException(t);
-    }
-  }
-
   private void updateAccountInfo() {
     if (setFullNameTo == null) {
       return;