From f35bba9a19c5c13c391e8bdda3dcd000345b3178 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Mon, 19 Mar 2018 11:44:00 -0400 Subject: [PATCH] GcAllUsers: Respect gc.auto=0 in All-Users Arguably the GC operation is more like a manual run of `git gc` than it is like an automatic threshold-based GC, which is why it didn't originally inspect gc.auto. However, admins set gc.auto=0 to say that they don't trust JGit GC to be bug-free, so we would want to respect it for that reason. When this happens, at least print a warning message. Change-Id: If3e1353675302faa549d57ad4b4b681fdcf3cbdd --- .../server/notedb/rebuild/GcAllUsers.java | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/GcAllUsers.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/GcAllUsers.java index bca077205d..0653192308 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/GcAllUsers.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/GcAllUsers.java @@ -15,7 +15,10 @@ package com.google.gerrit.server.notedb.rebuild; import static com.google.common.base.Preconditions.checkNotNull; +import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_GC_SECTION; +import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_AUTO; +import com.google.common.base.Throwables; import com.google.common.collect.ImmutableList; import com.google.gerrit.common.Nullable; import com.google.gerrit.common.data.GarbageCollectionResult; @@ -25,8 +28,10 @@ import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.LocalDiskRepositoryManager; import com.google.inject.Inject; import com.google.inject.Singleton; +import java.io.IOException; import java.io.PrintWriter; import java.util.function.Consumer; +import org.eclipse.jgit.lib.Repository; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -63,6 +68,21 @@ public class GcAllUsers { logOneLine.accept("Skipping GC of " + allUsers + "; not a local disk repo"); return; } + if (!enableAutoGc(logOneLine)) { + logOneLine.accept( + "Skipping GC of " + + allUsers + + " due to disabling " + + CONFIG_GC_SECTION + + "." + + CONFIG_KEY_AUTO); + logOneLine.accept( + "If loading accounts is slow after the NoteDb migration, run `git gc` on " + + allUsers + + " manually"); + return; + } + if (progressWriter == null) { // Mimic log line from GarbageCollection. logOneLine.accept("collecting garbage for \"" + allUsers + "\":\n"); @@ -89,4 +109,14 @@ public class GcAllUsers { } } } + + private boolean enableAutoGc(Consumer logOneLine) { + try (Repository repo = repoManager.openRepository(allUsers)) { + return repo.getConfig().getInt(CONFIG_GC_SECTION, CONFIG_KEY_AUTO, -1) != 0; + } catch (IOException e) { + logOneLine.accept( + "Error reading config for " + allUsers + ":\n" + Throwables.getStackTraceAsString(e)); + return false; + } + } }