Disable commitWithin when running Reindex

Disable the 'commitWithin' from within Reindex by overriding
the configuration with '-1'. Treat negative values as the
original behavior, auto-flushing but not auto-committing, which
is the least safe but the most efficient for reindexing the
entire site. 

Change-Id: Ifdba797bee871d2a3d8928810a6304bacb850c8c

Documentation/config-gerrit.txt

@@ -1894,18 +1894,16 @@ Determines the period at which changes are automatically committed to
 stable store on disk. This is a costly operation and may block
 additional index writes, so lower with caution.
 +
-If zero or negative, changes are committed after every write. This is
-very costly but may be useful if offline reindexing is infeasible, or
-for development servers.
+If zero, changes are committed after every write. This is very costly
+but may be useful if offline reindexing is infeasible, or for development
+servers.
 +
-Values can be specified using standard time unit abbreviations (`ms`,
-`sec`, `min`, etc.).
+Values can be specified using standard time unit abbreviations (`ms`, `sec`,
+`min`, etc.).
 +
-This setting also applies when running the reindex program.  If it is
-configured to commit on every write, this will cause reindex to take
-an unnecessarily long time to complete on sites that have a lot of
-changes.  It is recommended to temporarily set a higher value while
-running reindex.
+If negative, `commitWithin` is disabled. Changes are flushed to disk when
+the in-memory buffer fills, but only committed and guaranteed to be synced
+to disk when the process finishes.
 
 Defaults to 300000 ms (5 minutes).
 

gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java

@@ -137,7 +137,7 @@ public class LuceneChangeIndex implements ChangeIndex {
 
   static class GerritIndexWriterConfig {
     private final IndexWriterConfig luceneConfig;
-    private final long commitWithinMs;
+    private long commitWithinMs;
 
     private GerritIndexWriterConfig(Version version, Config cfg, String name) {
       luceneConfig = new IndexWriterConfig(version,
@@ -150,9 +150,13 @@ public class LuceneChangeIndex implements ChangeIndex {
       luceneConfig.setMaxBufferedDocs(cfg.getInt(
           "index", name, "maxBufferedDocs",
           IndexWriterConfig.DEFAULT_MAX_BUFFERED_DOCS));
-      commitWithinMs = ConfigUtil.getTimeUnit(
-          cfg, "index", name, "commitWithin",
-          MILLISECONDS.convert(5, MINUTES), MILLISECONDS);
+      try {
+        commitWithinMs =
+            ConfigUtil.getTimeUnit(cfg, "index", name, "commitWithin",
+                MILLISECONDS.convert(5, MINUTES), MILLISECONDS);
+      } catch (IllegalArgumentException e) {
+        commitWithinMs = cfg.getLong("index", name, "commitWithin", 0);
+      }
     }
 
     IndexWriterConfig getLuceneConfig() {

gerrit-lucene/src/main/java/com/google/gerrit/lucene/SubIndex.java

@@ -23,6 +23,7 @@ import com.google.common.util.concurrent.ThreadFactoryBuilder;
 import com.google.gerrit.lucene.LuceneChangeIndex.GerritIndexWriterConfig;
 
 import org.apache.lucene.document.Document;
+import org.apache.lucene.index.IndexWriter;
 import org.apache.lucene.index.Term;
 import org.apache.lucene.index.TrackingIndexWriter;
 import org.apache.lucene.search.ControlledRealTimeReopenThread;
@@ -40,7 +41,6 @@ import java.io.IOException;
 import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Executor;
-import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
@@ -55,7 +55,6 @@ class SubIndex {
   private final SearcherManager searcherManager;
   private final ControlledRealTimeReopenThread<IndexSearcher> reopenThread;
   private final ConcurrentMap<RefreshListener, Boolean> refreshListeners;
-  private final ScheduledExecutorService commitExecutor;
 
   SubIndex(File file, GerritIndexWriterConfig writerConfig) throws IOException {
     this(FSDirectory.open(file), file.getName(), writerConfig);
@@ -64,44 +63,45 @@ class SubIndex {
   SubIndex(Directory dir, final String dirName,
       GerritIndexWriterConfig writerConfig) throws IOException {
     this.dir = dir;
-
-    final AutoCommitWriter delegateWriter;
+    IndexWriter delegateWriter;
     long commitPeriod = writerConfig.getCommitWithinMs();
-    if (commitPeriod <= 0) {
-      commitExecutor = null;
+
+    if (commitPeriod < 0) {
+      delegateWriter = new IndexWriter(dir, writerConfig.getLuceneConfig());
+    } else if (commitPeriod == 0) {
       delegateWriter =
           new AutoCommitWriter(dir, writerConfig.getLuceneConfig(), true);
     } else {
-      commitExecutor = new ScheduledThreadPoolExecutor(1,
-          new ThreadFactoryBuilder()
-            .setNameFormat("Commit-%d " + dirName)
-            .setDaemon(true)
-            .build());
-      delegateWriter =
+      final AutoCommitWriter autoCommitWriter =
           new AutoCommitWriter(dir, writerConfig.getLuceneConfig(), false);
-      commitExecutor.scheduleAtFixedRate(new Runnable() {
-        @Override
-        public void run() {
-          try {
-            if (delegateWriter.hasUncommittedChanges()) {
-              delegateWriter.manualFlush();
-              delegateWriter.commit();
-            }
-          } catch (IOException e) {
-            log.error("Error committing Lucene index " + dirName, e);
-          } catch (OutOfMemoryError e) {
-            log.error("Error committing Lucene index " + dirName, e);
-            try {
-              delegateWriter.close();
-            } catch (IOException e2) {
-              log.error("SEVERE: Error closing Lucene index "
-                + dirName + " after OOM; index may be corrupted.", e);
-            }
-          }
-        }
-      }, commitPeriod, commitPeriod, MILLISECONDS);
-    }
+      delegateWriter = autoCommitWriter;
 
+      new ScheduledThreadPoolExecutor(1, new ThreadFactoryBuilder()
+          .setNameFormat("Commit-%d " + dirName)
+          .setDaemon(true)
+          .build())
+          .scheduleAtFixedRate(new Runnable() {
+            @Override
+            public void run() {
+              try {
+                if (autoCommitWriter.hasUncommittedChanges()) {
+                  autoCommitWriter.manualFlush();
+                  autoCommitWriter.commit();
+                }
+              } catch (IOException e) {
+                log.error("Error committing Lucene index " + dirName, e);
+              } catch (OutOfMemoryError e) {
+                log.error("Error committing Lucene index " + dirName, e);
+                try {
+                  autoCommitWriter.close();
+                } catch (IOException e2) {
+                  log.error("SEVERE: Error closing Lucene index " + dirName
+                      + " after OOM; index may be corrupted.", e);
+                }
+              }
+            }
+          }, commitPeriod, commitPeriod, MILLISECONDS);
+    }
     writer = new TrackingIndexWriter(delegateWriter);
     searcherManager = new SearcherManager(
         writer.getIndexWriter(), true, new SearcherFactory());

gerrit-pgm/src/main/java/com/google/gerrit/pgm/Reindex.java

@@ -95,6 +95,7 @@ public class Reindex extends SiteProgram {
       throw die("index.type must be configured (or not SQL)");
     }
     limitThreads();
+    disableLuceneAutomaticCommit();
     if (version == null) {
       version = ChangeSchemas.getLatest().getVersion();
     }
@@ -162,6 +163,15 @@ public class Reindex extends SiteProgram {
     return dbInjector.createChildInjector(modules);
   }
 
+  private void disableLuceneAutomaticCommit() {
+    Config cfg =
+        dbInjector.getInstance(Key.get(Config.class, GerritServerConfig.class));
+    if (IndexModule.getIndexType(dbInjector) == IndexType.LUCENE) {
+      cfg.setLong("index", "changes_open", "commitWithin", -1);
+      cfg.setLong("index", "changes_closed", "commitWithin", -1);
+    }
+  }
+
   private class ReviewDbModule extends LifecycleModule {
     @Override
     protected void configure() {

gerrit-server/src/main/java/com/google/gerrit/server/config/ConfigUtil.java

@@ -207,6 +207,10 @@ public class ConfigUtil {
       return defaultValue;
     }
 
+    if (s.startsWith("-")/* negative */) {
+      throw notTimeUnit(section, subsection, setting, valueString);
+    }
+
     try {
       return getTimeUnit(s, defaultValue, wantUnit);
     } catch (IllegalArgumentException notTime) {