Merge branch 'stable-3.5' into stable-3.6

* stable-3.5:
  GroupCacheImpl: Fix timer to include the correct method
  Make the indexing operation fail upon StorageException(s)
  ConsistencyCheckerIT: Delete calls to index broken changes that fail silently
  RefAdvertisementIT: Don't call reindex that would fail silently
  Bazel: Fix dev-bazel docs
  H2CacheImpl: Log when disk cache pruning runs
  MergeUtil: Create an optimized canFastForwardOrMerge

Release-Notes: skip
Change-Id: Ia568a9bae1a9aea84f4a60679a828771aa42afff

6 files changed, 52 insertions, 17 deletions

diff --git a/java/com/google/gerrit/index/Schema.java b/java/com/google/gerrit/index/Schema.java
index 91c3f70401..a14e583fa3 100644
--- a/java/com/google/gerrit/index/Schema.java
+++ b/java/com/google/gerrit/index/Schema.java
@@ -207,21 +207,19 @@ public class Schema<T> {
   /**
    * Build all fields in the schema from an input object.
    *
-   * <p>Null values are omitted, as are fields which cause errors, which are logged.
+   * <p>Null values are omitted, as are fields which cause errors, which are logged. If any of the
+   * fields cause a StorageException, the whole operation fails and the exception is propagated to
+   * the caller.
    *
    * @param obj input object.
    * @param skipFields set of field names to skip when indexing the document
    * @return all non-null field values from the object.
    */
   public final Iterable<Values<T>> buildFields(T obj, ImmutableSet<String> skipFields) {
-    try {
-      return fields.values().stream()
-          .map(f -> fieldValues(obj, f, skipFields))
-          .filter(Objects::nonNull)
-          .collect(toImmutableList());
-    } catch (StorageException e) {
-      return ImmutableList.of();
-    }
+    return fields.values().stream()
+        .map(f -> fieldValues(obj, f, skipFields))
+        .filter(Objects::nonNull)
+        .collect(toImmutableList());
   }
 
   @Override
diff --git a/java/com/google/gerrit/server/account/GroupCacheImpl.java b/java/com/google/gerrit/server/account/GroupCacheImpl.java
index eaec9baecd..36f725f56d 100644
--- a/java/com/google/gerrit/server/account/GroupCacheImpl.java
+++ b/java/com/google/gerrit/server/account/GroupCacheImpl.java
@@ -283,7 +283,7 @@ public class GroupCacheImpl implements GroupCache {
       List<Cache.GroupKeyProto> keyList = new ArrayList<>();
       try (TraceTimer ignored =
               TraceContext.newTimer(
-                  "Loading group from serialized cache",
+                  "Building keys to load group(s) from serialized cache",
                   Metadata.builder().cacheName(BYUUID_NAME_PERSISTED).build());
           Repository allUsers = repoManager.openRepository(allUsersName)) {
         while (uuidIterator.hasNext()) {
@@ -302,8 +302,13 @@ public class GroupCacheImpl implements GroupCache {
           keyList.add(key);
         }
       }
-      persistedCache.getAll(keyList).entrySet().stream()
-          .forEach(g -> toReturn.put(g.getKey().getUuid(), Optional.of(g.getValue())));
+      try (TraceTimer ignored =
+          TraceContext.newTimer(
+              "Loading group(s) from serialized cache",
+              Metadata.builder().cacheName(BYUUID_NAME_PERSISTED).build())) {
+        persistedCache.getAll(keyList).entrySet().stream()
+            .forEach(g -> toReturn.put(g.getKey().getUuid(), Optional.of(g.getValue())));
+      }
       return toReturn;
     }
   }
diff --git a/java/com/google/gerrit/server/cache/CacheInfo.java b/java/com/google/gerrit/server/cache/CacheInfo.java
index d6eb065339..832ca047f3 100644
--- a/java/com/google/gerrit/server/cache/CacheInfo.java
+++ b/java/com/google/gerrit/server/cache/CacheInfo.java
@@ -90,7 +90,7 @@ public class CacheInfo {
       space = bytes(value);
     }
 
-    private static String bytes(double value) {
+    public static String bytes(double value) {
       value /= 1024;
       String suffix = "k";
 
diff --git a/java/com/google/gerrit/server/cache/h2/H2CacheImpl.java b/java/com/google/gerrit/server/cache/h2/H2CacheImpl.java
index 7db4443008..cc2617805a 100644
--- a/java/com/google/gerrit/server/cache/h2/H2CacheImpl.java
+++ b/java/com/google/gerrit/server/cache/h2/H2CacheImpl.java
@@ -28,6 +28,7 @@ import com.google.common.util.concurrent.FutureCallback;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.gerrit.common.Nullable;
+import com.google.gerrit.server.cache.CacheInfo;
 import com.google.gerrit.server.cache.PersistentCache;
 import com.google.gerrit.server.cache.serialize.CacheSerializer;
 import com.google.gerrit.server.logging.Metadata;
@@ -660,12 +661,19 @@ public class H2CacheImpl<K, V> extends AbstractLoadingCache<K, V> implements Per
           try (ResultSet r = s.executeQuery("SELECT SUM(space) FROM data")) {
             used = r.next() ? r.getLong(1) : 0;
           }
+          String formattedMaxSize = CacheInfo.EntriesInfo.bytes(maxSize);
           if (used <= maxSize) {
+            logger.atInfo().log(
+                "Cache %s size (%s) is less than maxSize (%s), not pruning",
+                url, CacheInfo.EntriesInfo.bytes(used), formattedMaxSize);
             return;
           }
 
           try (ResultSet r =
               s.executeQuery("SELECT k, space, created FROM data ORDER BY accessed")) {
+            logger.atInfo().log(
+                "Cache %s size (%s) is greater than maxSize (%s), pruning",
+                url, CacheInfo.EntriesInfo.bytes(used), formattedMaxSize);
             while (maxSize < used && r.next()) {
               K key = keyType.get(r, 1);
               Timestamp created = r.getTimestamp(3);
@@ -676,6 +684,9 @@ public class H2CacheImpl<K, V> extends AbstractLoadingCache<K, V> implements Per
                 used -= r.getLong(2);
               }
             }
+            logger.atInfo().log(
+                "Done pruning cache %s, size (%s) is now less than maxSize (%s)",
+                url, CacheInfo.EntriesInfo.bytes(used), formattedMaxSize);
           }
         }
       } catch (IOException | SQLException e) {
diff --git a/java/com/google/gerrit/server/git/MergeUtil.java b/java/com/google/gerrit/server/git/MergeUtil.java
index d84ce7bfa5..3df7288c73 100644
--- a/java/com/google/gerrit/server/git/MergeUtil.java
+++ b/java/com/google/gerrit/server/git/MergeUtil.java
@@ -680,6 +680,10 @@ public class MergeUtil {
       return false;
     }
 
+    return canMerge(mergeTip, repo, toMerge);
+  }
+
+  private boolean canMerge(CodeReviewCommit mergeTip, Repository repo, CodeReviewCommit toMerge) {
     try (ObjectInserter ins = new InMemoryInserter(repo)) {
       return newThreeWayMerger(ins, repo.getConfig()).merge(mergeTip, toMerge);
     } catch (LargeObjectException e) {
@@ -701,6 +705,11 @@ public class MergeUtil {
       return false;
     }
 
+    return canFastForward(mergeTip, rw, toMerge);
+  }
+
+  private boolean canFastForward(
+      CodeReviewCommit mergeTip, CodeReviewRevWalk rw, CodeReviewCommit toMerge) {
     try {
       return mergeTip == null
           || rw.isMergedInto(mergeTip, toMerge)
@@ -710,6 +719,19 @@ public class MergeUtil {
     }
   }
 
+  public boolean canFastForwardOrMerge(
+      MergeSorter mergeSorter,
+      CodeReviewCommit mergeTip,
+      CodeReviewRevWalk rw,
+      Repository repo,
+      CodeReviewCommit toMerge) {
+    if (hasMissingDependencies(mergeSorter, toMerge)) {
+      return false;
+    }
+
+    return canFastForward(mergeTip, rw, toMerge) || canMerge(mergeTip, repo, toMerge);
+  }
+
   public boolean canCherryPick(
       MergeSorter mergeSorter,
       Repository repo,
@@ -752,8 +774,7 @@ public class MergeUtil {
     // by an equivalent merge with a different first parent. So
     // instead behave as though MERGE_IF_NECESSARY was configured.
     //
-    return canFastForward(mergeSorter, mergeTip, rw, toMerge)
-        || canMerge(mergeSorter, repo, mergeTip, toMerge);
+    return canFastForwardOrMerge(mergeSorter, mergeTip, rw, repo, toMerge);
   }
 
   public boolean hasMissingDependencies(MergeSorter mergeSorter, CodeReviewCommit toMerge) {
diff --git a/java/com/google/gerrit/server/submit/MergeIfNecessary.java b/java/com/google/gerrit/server/submit/MergeIfNecessary.java
index 75136f50f5..b8417b8bc1 100644
--- a/java/com/google/gerrit/server/submit/MergeIfNecessary.java
+++ b/java/com/google/gerrit/server/submit/MergeIfNecessary.java
@@ -49,7 +49,7 @@ public class MergeIfNecessary extends SubmitStrategy {
 
   static boolean dryRun(
       SubmitDryRun.Arguments args, CodeReviewCommit mergeTip, CodeReviewCommit toMerge) {
-    return args.mergeUtil.canFastForward(args.mergeSorter, mergeTip, args.rw, toMerge)
-        || args.mergeUtil.canMerge(args.mergeSorter, args.repo, mergeTip, toMerge);
+    return args.mergeUtil.canFastForwardOrMerge(
+        args.mergeSorter, mergeTip, args.rw, args.repo, toMerge);
   }
 }