RepoView: Make flushing inserter a no-op

Flushing is already done automatically at the right time by the
BatchUpdate implementations, and the RevWalk provided to callers can
already read unflushed objects. A BatchUpdateOp implementation calling
flush on the inserter is doing something wrong, and flushing too much
can seriously impact DFS performance when it happens in a loop.

Make flushing (and closing) a no-op rather than slapping implementors'
wrists by throwing an exception. Even if the implementor is well-
intentioned, we can't guarantee that all downstream library functions
are going to know not to flush their inserters. For example, JGit's
Merger used to be aggressive about flushing until I fixed it[1]. This
was the most glaring example, but I would be utterly unsurprised to find
more.

Also don't use InMemoryInserter for this purpose, as it actually flushes
too little. The purpose of that class is to avoid writing a pack to DFS
when the internal buffer in DfsInserter exceeds some limit, but in
BatchUpdate, it's fine to write out an intermediate pack in this case,
we don't have to buffer the whole thing in memory.

[1] https://git.eclipse.org/r/12504

Change-Id: I2b139e50af213c4b683c25d30b190bc547304f72
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/update/BatchUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/update/BatchUpdate.java
index 281fddb..e457d9c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/update/BatchUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/update/BatchUpdate.java
@@ -332,11 +332,6 @@
     return repoView.getRevWalk();
   }
 
-  protected ObjectInserter getObjectInserter() throws IOException {
-    initRepository();
-    return repoView.getInserter();
-  }
-
   public Map<String, ReceiveCommand> getRefUpdates() {
     return repoView != null ? repoView.getCommands().getCommands() : ImmutableMap.of();
   }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/update/NoteDbBatchUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/update/NoteDbBatchUpdate.java
index 5594862..674ecb1 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/update/NoteDbBatchUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/update/NoteDbBatchUpdate.java
@@ -184,7 +184,7 @@
   private class RepoContextImpl extends ContextImpl implements RepoContext {
     @Override
     public ObjectInserter getInserter() throws IOException {
-      return getRepoView().getInserter();
+      return getRepoView().getInserterWrapper();
     }
 
     @Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/update/RepoView.java b/gerrit-server/src/main/java/com/google/gerrit/server/update/RepoView.java
index cb1abdb..8839dbe 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/update/RepoView.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/update/RepoView.java
@@ -48,12 +48,14 @@
   private final Repository repo;
   private final RevWalk rw;
   private final ObjectInserter inserter;
+  private final ObjectInserter inserterWrapper;
   private final ChainedReceiveCommands commands;
   private final boolean closeRepo;
 
   RepoView(GitRepositoryManager repoManager, Project.NameKey project) throws IOException {
     repo = repoManager.openRepository(project);
     inserter = repo.newObjectInserter();
+    inserterWrapper = new NonFlushingInserter(inserter);
     rw = new RevWalk(inserter.newReader());
     commands = new ChainedReceiveCommands(repo);
     closeRepo = true;
@@ -68,6 +70,7 @@
     this.repo = checkNotNull(repo);
     this.rw = checkNotNull(rw);
     this.inserter = checkNotNull(inserter);
+    inserterWrapper = new NonFlushingInserter(inserter);
     commands = new ChainedReceiveCommands(repo);
     closeRepo = false;
   }
@@ -198,7 +201,34 @@
     return inserter;
   }
 
+  ObjectInserter getInserterWrapper() {
+    return inserterWrapper;
+  }
+
   ChainedReceiveCommands getCommands() {
     return commands;
   }
+
+  private static class NonFlushingInserter extends ObjectInserter.Filter {
+    private final ObjectInserter delegate;
+
+    private NonFlushingInserter(ObjectInserter delegate) {
+      this.delegate = delegate;
+    }
+
+    @Override
+    protected ObjectInserter delegate() {
+      return delegate;
+    }
+
+    @Override
+    public void flush() {
+      // Do nothing.
+    }
+
+    @Override
+    public void close() {
+      // Do nothing; the delegate is closed separately.
+    }
+  }
 }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/update/ReviewDbBatchUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/update/ReviewDbBatchUpdate.java
index b6017511f6..95ed053 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/update/ReviewDbBatchUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/update/ReviewDbBatchUpdate.java
@@ -155,7 +155,7 @@
   private class RepoContextImpl extends ContextImpl implements RepoContext {
     @Override
     public ObjectInserter getInserter() throws IOException {
-      return getRepoView().getInserter();
+      return getRepoView().getInserterWrapper();
     }
 
     @Override