Trigger All-Users GC in background in schema 123

New refs created under starred-changes using an atomic BatchRefUpdate
operation leaves behind empty dirs in 'refs/starred-changes'. This can
cause performance issues for subsequent schemas accessing these starred
refs. A GC will clean up such empty ref dirs. Doing the GC inline will
be a performance hit as the cleanup can be time-taking on NFS. Instead,
run the GC in background at the end of this schema, which allows others
schemas to then proceed without waiting for the GC to complete. Note
that, the request to run GC in background will be ignored if another GC
for that project is already queued.

This change allows us to either remove the GCs done in Schema 146 or at
the least run them in background.

Change-Id: I1c9cb7cccfd87db7ffce57512562372af2fadff6

3 files changed, 88 insertions, 36 deletions

diff --git a/java/com/google/gerrit/server/schema/SchemaVersion.java b/java/com/google/gerrit/server/schema/SchemaVersion.java
index 547ea81daf..da87c50691 100644
--- a/java/com/google/gerrit/server/schema/SchemaVersion.java
+++ b/java/com/google/gerrit/server/schema/SchemaVersion.java
@@ -33,9 +33,11 @@ import java.io.IOException;
 import java.sql.PreparedStatement;
 import java.sql.SQLException;
 import java.sql.Statement;
+import java.text.ParseException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Date;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Objects;
@@ -45,15 +47,20 @@ import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
+import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 import org.eclipse.jgit.internal.storage.file.FileRepository;
+import org.eclipse.jgit.internal.storage.file.GC;
 import org.eclipse.jgit.lib.CommitBuilder;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectInserter;
 import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.ProgressMonitor;
 import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.lib.TextProgressMonitor;
+import org.eclipse.jgit.storage.pack.PackConfig;
 
 /** A version of the database schema. */
 public abstract class SchemaVersion {
@@ -64,6 +71,7 @@ public abstract class SchemaVersion {
     return guessVersion(C);
   }
 
+  private static final ExecutorService backgroundGcThread = Executors.newFixedThreadPool(1);
   private final Provider<? extends SchemaVersion> prior;
   private final int versionNbr;
   private static SortedSet<Project.NameKey> projects;
@@ -317,6 +325,43 @@ public abstract class SchemaVersion {
     return cb;
   }
 
+  /**
+   * Add a task to GC a project to a single thread serving GC requests. If a GC task already exists
+   * for that project, then the current request to GC it will be ignored.
+   *
+   * @param repoManager Git repository manager to open the git repository.
+   * @param project Project name.
+   * @param ui Interface for interacting with the user.
+   */
+  protected synchronized void runGcInBackground(
+      GitRepositoryManager repoManager, Project.NameKey project, UpdateUI ui) {
+    for (Runnable task : ((ThreadPoolExecutor) backgroundGcThread).getQueue()) {
+      if (task instanceof GcTask) {
+        if (((GcTask) task).project.equals(project)) {
+          return;
+        }
+      }
+    }
+    backgroundGcThread.execute(new GcTask(project, repoManager, ui));
+  }
+
+  protected void gc(Repository repo, UpdateUI ui, ProgressMonitor pm, Stopwatch sw)
+      throws IOException, ParseException {
+    FileRepository r = (FileRepository) repo;
+    GC gc = new GC(r);
+    gc.setProgressMonitor(pm);
+    pm.beginTask("gc", ProgressMonitor.UNKNOWN);
+    // TODO(ms): Enable bitmap index when this JGit performance issue is fixed:
+    // https://bugs.eclipse.org/bugs/show_bug.cgi?id=562740
+    PackConfig pconfig = new PackConfig(repo);
+    pconfig.setBuildBitmaps(false);
+    gc.setPackConfig(pconfig);
+    ui.message(
+        String.format("... (%.3f s) gc --prune=now", sw.elapsed(TimeUnit.MILLISECONDS) / 1000d));
+    gc.setExpire(new Date());
+    gc.gc();
+  }
+
   protected static ObjectId emptyTree(ObjectInserter oi) throws IOException {
     return oi.insert(Constants.OBJ_TREE, new byte[] {});
   }
@@ -331,4 +376,36 @@ public abstract class SchemaVersion {
   private static long countDone(Collection<Future> futures) {
     return futures.stream().filter(Future::isDone).count();
   }
+
+  private class GcTask implements Runnable {
+    final GitRepositoryManager repoManager;
+    final Project.NameKey project;
+    final UpdateUI ui;
+
+    public GcTask(Project.NameKey project, GitRepositoryManager repoManager, UpdateUI ui) {
+      this.project = project;
+      this.repoManager = repoManager;
+      this.ui = ui;
+    }
+
+    @Override
+    public void run() {
+      try (Repository repo = repoManager.openRepository(project)) {
+        ProgressMonitor pm = new TextProgressMonitor();
+        try {
+          // Empty reference folders are only deleted if they have not been modified
+          // in the last 30s. Add a delay so that their deletion is not skipped.
+          Thread.sleep(30000);
+          Stopwatch sw = Stopwatch.createStarted();
+          gc(repo, ui, pm, sw);
+        } catch (IOException | ParseException | InterruptedException ex) {
+          ui.message("GC on " + project.get() + " failed with error: " + ex);
+        } finally {
+          pm.endTask();
+        }
+      } catch (IOException ex) {
+        ui.message("GC on " + project.get() + " failed with error: " + ex);
+      }
+    }
+  }
 }
diff --git a/java/com/google/gerrit/server/schema/Schema_123.java b/java/com/google/gerrit/server/schema/Schema_123.java
index 31cfd5d30d..d23c0d81a1 100644
--- a/java/com/google/gerrit/server/schema/Schema_123.java
+++ b/java/com/google/gerrit/server/schema/Schema_123.java
@@ -79,6 +79,7 @@ public class Schema_123 extends SchemaVersion {
                 ObjectId.zeroId(), id, RefNames.refsStarredChanges(e.getValue(), e.getKey())));
       }
       bru.execute(rw, new TextProgressMonitor());
+      runGcInBackground(repoManager, allUsersName, ui);
     } catch (IOException | IllegalLabelException ex) {
       throw new OrmException(ex);
     }
diff --git a/java/com/google/gerrit/server/schema/Schema_146.java b/java/com/google/gerrit/server/schema/Schema_146.java
index 32dfc83bc3..bc90c6d73d 100644
--- a/java/com/google/gerrit/server/schema/Schema_146.java
+++ b/java/com/google/gerrit/server/schema/Schema_146.java
@@ -33,7 +33,6 @@ import java.sql.SQLException;
 import java.sql.Statement;
 import java.sql.Timestamp;
 import java.text.ParseException;
-import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -45,7 +44,6 @@ import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.locks.ReentrantLock;
 import org.eclipse.jgit.internal.storage.file.FileRepository;
-import org.eclipse.jgit.internal.storage.file.GC;
 import org.eclipse.jgit.internal.storage.file.PackInserter;
 import org.eclipse.jgit.lib.BatchRefUpdate;
 import org.eclipse.jgit.lib.CommitBuilder;
@@ -61,7 +59,6 @@ import org.eclipse.jgit.lib.TextProgressMonitor;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevSort;
 import org.eclipse.jgit.revwalk.RevWalk;
-import org.eclipse.jgit.storage.pack.PackConfig;
 import org.eclipse.jgit.transport.ReceiveCommand;
 
 /**
@@ -188,42 +185,19 @@ public class Schema_146 extends SchemaVersion {
 
   private void gc(UpdateUI ui) {
     try (Repository repo = repoManager.openRepository(allUsersName)) {
-      gc(repo, false, ui);
-    } catch (IOException e) {
-      throw new UncheckedIOException(e);
-    }
-  }
-
-  private void gc(Repository repo, boolean refsOnly, UpdateUI ui) {
-    if (repo instanceof FileRepository && gcLock.tryLock()) {
-      ProgressMonitor pm = null;
-      try {
-        pm = new TextProgressMonitor();
-        FileRepository r = (FileRepository) repo;
-        GC gc = new GC(r);
-        gc.setProgressMonitor(pm);
-        pm.beginTask("gc", ProgressMonitor.UNKNOWN);
-        if (refsOnly) {
-          ui.message(String.format("... (%.3f s) pack refs", elapsed()));
-          gc.packRefs();
-        } else {
-          // TODO(ms): Enable bitmap index when this JGit performance issue is fixed:
-          // https://bugs.eclipse.org/bugs/show_bug.cgi?id=562740
-          PackConfig pconfig = new PackConfig(repo);
-          pconfig.setBuildBitmaps(false);
-          gc.setPackConfig(pconfig);
-          ui.message(String.format("... (%.3f s) gc --prune=now", elapsed()));
-          gc.setExpire(new Date());
-          gc.gc();
-        }
-      } catch (IOException | ParseException e) {
-        throw new RuntimeException(e);
-      } finally {
-        gcLock.unlock();
-        if (pm != null) {
+      if (repo instanceof FileRepository && gcLock.tryLock()) {
+        ProgressMonitor pm = new TextProgressMonitor();
+        try {
+          gc(repo, ui, pm, sw);
+        } catch (IOException | ParseException e) {
+          throw new RuntimeException(e);
+        } finally {
+          gcLock.unlock();
           pm.endTask();
         }
       }
+    } catch (IOException e) {
+      throw new UncheckedIOException(e);
     }
   }