Skip building bloom filters by default during offline reindex

Building bloom filters can take a really long time, especially
when fully populated disk caches are used with large Gerrit
installations. Skipping it avoids unnecessary work when fully
populated caches are being used. Even when reindexing with
caches that aren't populated, bloom filters don't provide a
significant performance improvement.

Release-Notes: Bloom filters are no longer built during offline reindex
Change-Id: I33095383a13f6de4f10a21c2a215e63dfa7972d1

7 files changed, 54 insertions, 6 deletions

diff --git a/Documentation/pgm-reindex.txt b/Documentation/pgm-reindex.txt
index b74829dbde..183c13266c 100644
--- a/Documentation/pgm-reindex.txt
+++ b/Documentation/pgm-reindex.txt
@@ -39,6 +39,12 @@ Rebuilds the secondary index.
 --show-cache-stats::
 	Show cache statistics at the end of program.
 
+--build-bloom-filter::
+	Whether to build bloom filters for H2 disk caches. When using fully
+	populated disk caches on large Gerrit sites, it is recommended that
+	bloom filters are disabled to improve performance.
+
+
 == CONTEXT
 The secondary index must be enabled. See
 link:config-gerrit.html#index.type[index.type].
diff --git a/java/com/google/gerrit/pgm/Reindex.java b/java/com/google/gerrit/pgm/Reindex.java
index 7ee799f406..ecfca0dbc9 100644
--- a/java/com/google/gerrit/pgm/Reindex.java
+++ b/java/com/google/gerrit/pgm/Reindex.java
@@ -39,6 +39,7 @@ import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.index.IndexModule;
 import com.google.gerrit.server.index.change.ChangeSchemaDefinitions;
 import com.google.gerrit.server.index.options.AutoFlush;
+import com.google.gerrit.server.index.options.BuildBloomFilter;
 import com.google.gerrit.server.index.options.IsFirstInsertForEntry;
 import com.google.gerrit.server.plugins.PluginGuiceEnvironment;
 import com.google.gerrit.server.util.ReplicaUtil;
@@ -89,6 +90,9 @@ public class Reindex extends SiteProgram {
   @Option(name = "--show-cache-stats", usage = "Show cache statistics at the end.")
   private boolean showCacheStats;
 
+  @Option(name = "--build-bloom-filter", usage = "Build bloom filter for H2 disk caches.")
+  private boolean buildBloomFilter;
+
   private Injector dbInjector;
   private Injector sysInjector;
   private Injector cfgInjector;
@@ -204,6 +208,9 @@ public class Reindex extends SiteProgram {
             OptionalBinder.newOptionalBinder(binder(), IsFirstInsertForEntry.class)
                 .setBinding()
                 .toInstance(IsFirstInsertForEntry.YES);
+            OptionalBinder.newOptionalBinder(binder(), BuildBloomFilter.class)
+                .setBinding()
+                .toInstance(buildBloomFilter ? BuildBloomFilter.TRUE : BuildBloomFilter.FALSE);
           }
         });
     modules.add(new BatchProgramModule(dbInjector));
diff --git a/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java b/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java
index 893a42c4a8..961ec1488f 100644
--- a/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java
+++ b/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java
@@ -30,6 +30,7 @@ import com.google.gerrit.server.cache.h2.H2CacheImpl.SqlStore;
 import com.google.gerrit.server.cache.h2.H2CacheImpl.ValueHolder;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.index.options.BuildBloomFilter;
 import com.google.gerrit.server.index.options.IsFirstInsertForEntry;
 import com.google.gerrit.server.logging.LoggingContextAwareExecutorService;
 import com.google.gerrit.server.logging.LoggingContextAwareScheduledExecutorService;
@@ -61,6 +62,7 @@ class H2CacheFactory extends PersistentCacheBaseFactory implements LifecycleList
   private final long h2CacheSize;
   private final boolean h2AutoServer;
   private final boolean isOfflineReindex;
+  private final boolean buildBloomFilter;
 
   @Inject
   H2CacheFactory(
@@ -68,7 +70,8 @@ class H2CacheFactory extends PersistentCacheBaseFactory implements LifecycleList
       @GerritServerConfig Config cfg,
       SitePaths site,
       DynamicMap<Cache<?, ?>> cacheMap,
-      @Nullable IsFirstInsertForEntry isFirstInsertForEntry) {
+      @Nullable IsFirstInsertForEntry isFirstInsertForEntry,
+      @Nullable BuildBloomFilter buildBloomFilter) {
     super(memCacheFactory, cfg, site);
     h2CacheSize = cfg.getLong("cache", null, "h2CacheSize", -1);
     h2AutoServer = cfg.getBoolean("cache", null, "h2AutoServer", false);
@@ -76,6 +79,8 @@ class H2CacheFactory extends PersistentCacheBaseFactory implements LifecycleList
     this.cacheMap = cacheMap;
     this.isOfflineReindex =
         isFirstInsertForEntry != null && isFirstInsertForEntry.equals(IsFirstInsertForEntry.YES);
+    this.buildBloomFilter =
+        !(buildBloomFilter != null && buildBloomFilter.equals(BuildBloomFilter.FALSE));
 
     if (diskEnabled) {
       executor =
@@ -211,6 +216,7 @@ class H2CacheFactory extends PersistentCacheBaseFactory implements LifecycleList
         def.version(),
         maxSize,
         def.expireAfterWrite(),
-        def.expireFromMemoryAfterAccess());
+        def.expireFromMemoryAfterAccess(),
+        buildBloomFilter);
   }
 }
diff --git a/java/com/google/gerrit/server/cache/h2/H2CacheImpl.java b/java/com/google/gerrit/server/cache/h2/H2CacheImpl.java
index 13b8b12313..d5e5c7f739 100644
--- a/java/com/google/gerrit/server/cache/h2/H2CacheImpl.java
+++ b/java/com/google/gerrit/server/cache/h2/H2CacheImpl.java
@@ -364,6 +364,7 @@ public class H2CacheImpl<K, V> extends AbstractLoadingCache<K, V> implements Per
     private final AtomicLong missCount = new AtomicLong();
     private volatile BloomFilter<K> bloomFilter;
     private int estimatedSize;
+    private boolean buildBloomFilter;
 
     SqlStore(
         String jdbcUrl,
@@ -373,7 +374,8 @@ public class H2CacheImpl<K, V> extends AbstractLoadingCache<K, V> implements Per
         int version,
         long maxSize,
         @Nullable Duration expireAfterWrite,
-        @Nullable Duration refreshAfterWrite) {
+        @Nullable Duration refreshAfterWrite,
+        boolean buildBloomFilter) {
       this.url = jdbcUrl;
       this.keyType = createKeyType(keyType, keySerializer);
       this.valueSerializer = valueSerializer;
@@ -381,6 +383,7 @@ public class H2CacheImpl<K, V> extends AbstractLoadingCache<K, V> implements Per
       this.maxSize = maxSize;
       this.expireAfterWrite = expireAfterWrite;
       this.refreshAfterWrite = refreshAfterWrite;
+      this.buildBloomFilter = buildBloomFilter;
 
       int cores = Runtime.getRuntime().availableProcessors();
       int keep = Math.min(cores, 16);
@@ -397,7 +400,7 @@ public class H2CacheImpl<K, V> extends AbstractLoadingCache<K, V> implements Per
     }
 
     synchronized void open() {
-      if (bloomFilter == null) {
+      if (buildBloomFilter && bloomFilter == null) {
         bloomFilter = buildBloomFilter();
       }
     }
@@ -411,7 +414,7 @@ public class H2CacheImpl<K, V> extends AbstractLoadingCache<K, V> implements Per
 
     boolean mightContain(K key) {
       BloomFilter<K> b = bloomFilter;
-      if (b == null) {
+      if (buildBloomFilter && b == null) {
         synchronized (this) {
           b = bloomFilter;
           if (b == null) {
diff --git a/java/com/google/gerrit/server/index/IndexModule.java b/java/com/google/gerrit/server/index/IndexModule.java
index a7298638c1..85f423b683 100644
--- a/java/com/google/gerrit/server/index/IndexModule.java
+++ b/java/com/google/gerrit/server/index/IndexModule.java
@@ -52,6 +52,7 @@ import com.google.gerrit.server.index.group.GroupIndexRewriter;
 import com.google.gerrit.server.index.group.GroupIndexer;
 import com.google.gerrit.server.index.group.GroupIndexerImpl;
 import com.google.gerrit.server.index.group.GroupSchemaDefinitions;
+import com.google.gerrit.server.index.options.BuildBloomFilter;
 import com.google.gerrit.server.index.options.IsFirstInsertForEntry;
 import com.google.gerrit.server.index.project.ProjectIndexDefinition;
 import com.google.gerrit.server.index.project.ProjectIndexerImpl;
@@ -150,6 +151,9 @@ public class IndexModule extends LifecycleModule {
     OptionalBinder.newOptionalBinder(binder(), IsFirstInsertForEntry.class)
         .setDefault()
         .toInstance(IsFirstInsertForEntry.NO);
+    OptionalBinder.newOptionalBinder(binder(), BuildBloomFilter.class)
+        .setDefault()
+        .toInstance(BuildBloomFilter.TRUE);
   }
 
   @Provides
diff --git a/java/com/google/gerrit/server/index/options/BuildBloomFilter.java b/java/com/google/gerrit/server/index/options/BuildBloomFilter.java
new file mode 100644
index 0000000000..021f0fe42f
--- /dev/null
+++ b/java/com/google/gerrit/server/index/options/BuildBloomFilter.java
@@ -0,0 +1,21 @@
+// Copyright (C) 2023 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.index.options;
+
+/** This enum can be used to decide if bloom filters for H2 disk caches should be built. */
+public enum BuildBloomFilter {
+  TRUE,
+  FALSE
+}
diff --git a/javatests/com/google/gerrit/server/cache/h2/H2CacheTest.java b/javatests/com/google/gerrit/server/cache/h2/H2CacheTest.java
index 14af43be57..b83365f747 100644
--- a/javatests/com/google/gerrit/server/cache/h2/H2CacheTest.java
+++ b/javatests/com/google/gerrit/server/cache/h2/H2CacheTest.java
@@ -71,7 +71,8 @@ public class H2CacheTest {
         version,
         1 << 20,
         expireAfterWrite,
-        refreshAfterWrite);
+        refreshAfterWrite,
+        true);
   }
 
   @Test