Convert ChangeBatchIndexer to more of a builder pattern
There was a single method with two required and many optional
arguments. Move the optional arguments into setters, except for the
project count, which we just cheat and extract from the input only if
the input is a Collection (which is 1 of the 2 callers).
This refactoring exposes and fixes a likely unintended behavior: we
should not recheck mergeability during online reindexing. The purpose
of online reindexing is to get a new index schema version up and
running in a timely fashion, without overloading a running server.
Checking mergeability by default was not helping here; I think it was
just an unintended quirk due to the way MergeabilityChecker was
injected into the ChangeBatchIndexer constructor.
Change-Id: Ifec986ef3c976ddeb00dd4218c5d029de9dc905e
diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/OnlineReindexer.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/OnlineReindexer.java
index d3dc963..bcf5eea 100644
--- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/OnlineReindexer.java
+++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/OnlineReindexer.java
@@ -76,8 +76,8 @@
"not an active write schema version: %s", version);
log.info("Starting online reindex from schema version {} to {}",
version(indexes.getSearchIndex()), version(index));
- ChangeBatchIndexer.Result result = batchIndexer.indexAll(
- index, projectCache.all(), -1, -1, null, null);
+ ChangeBatchIndexer.Result result =
+ batchIndexer.indexAll(index, projectCache.all());
if (!result.success()) {
log.error("Online reindex of schema version {} failed", version(index));
return;
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Reindex.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Reindex.java
index cf4fdbf..bcd34bc 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Reindex.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Reindex.java
@@ -149,9 +149,6 @@
protected void configure() {
if (recheckMergeable) {
install(new MergeabilityModule());
- } else {
- bind(MergeabilityChecker.class)
- .toProvider(Providers.<MergeabilityChecker> of(null));
}
}
});
@@ -219,9 +216,14 @@
ChangeBatchIndexer batchIndexer =
sysInjector.getInstance(ChangeBatchIndexer.class);
- ChangeBatchIndexer.Result result = batchIndexer.indexAll(
- index, projects, projects.size(), changeCount, System.err,
- verbose ? System.out : NullOutputStream.INSTANCE);
+ if (recheckMergeable) {
+ batchIndexer.setMergeabilityChecker(
+ sysInjector.getInstance(MergeabilityChecker.class));
+ }
+ ChangeBatchIndexer.Result result = batchIndexer.setNumChanges(changeCount)
+ .setProgressOut(System.err)
+ .setVerboseOut(verbose ? System.out : NullOutputStream.INSTANCE)
+ .indexAll(index, projects);
int n = result.doneCount() + result.failedCount();
double t = result.elapsed(TimeUnit.MILLISECONDS) / 1000d;
System.out.format("Reindexed %d changes in %.01fs (%.01f/s)\n", n, t, n/t);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeBatchIndexer.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeBatchIndexer.java
index 9a992cf..028d8fb 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeBatchIndexer.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeBatchIndexer.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.index;
+import static com.google.common.base.Preconditions.checkNotNull;
import static org.eclipse.jgit.lib.RefDatabase.ALL;
import com.google.common.base.Stopwatch;
@@ -27,7 +28,6 @@
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.common.util.concurrent.MoreExecutors;
-import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
@@ -65,6 +65,7 @@
import java.io.IOException;
import java.io.OutputStream;
import java.io.PrintWriter;
+import java.util.Collection;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
@@ -114,40 +115,59 @@
private final GitRepositoryManager repoManager;
private final ListeningExecutorService executor;
private final ChangeIndexer.Factory indexerFactory;
- private final MergeabilityChecker mergeabilityChecker;
private final ThreeWayMergeStrategy mergeStrategy;
+ private MergeabilityChecker mergeabilityChecker;
+ private int numChanges = -1;
+ private OutputStream progressOut = NullOutputStream.INSTANCE;
+ private PrintWriter verboseWriter =
+ new PrintWriter(NullOutputStream.INSTANCE);
+
@Inject
ChangeBatchIndexer(SchemaFactory<ReviewDb> schemaFactory,
ChangeData.Factory changeDataFactory,
GitRepositoryManager repoManager,
@IndexExecutor ListeningExecutorService executor,
ChangeIndexer.Factory indexerFactory,
- @GerritServerConfig Config config,
- @Nullable MergeabilityChecker mergeabilityChecker) {
+ @GerritServerConfig Config config) {
this.schemaFactory = schemaFactory;
this.changeDataFactory = changeDataFactory;
this.repoManager = repoManager;
this.executor = executor;
this.indexerFactory = indexerFactory;
- this.mergeabilityChecker = mergeabilityChecker;
this.mergeStrategy = MergeUtil.getMergeStrategy(config);
}
- public Result indexAll(ChangeIndex index, Iterable<Project.NameKey> projects,
- int numProjects, int numChanges, OutputStream progressOut,
- OutputStream verboseOut) {
- if (progressOut == null) {
- progressOut = NullOutputStream.INSTANCE;
- }
- PrintWriter verboseWriter = verboseOut != null ? new PrintWriter(verboseOut)
- : null;
+ public ChangeBatchIndexer setMergeabilityChecker(
+ MergeabilityChecker checker) {
+ mergeabilityChecker = checkNotNull(checker);
+ return this;
+ }
+ public ChangeBatchIndexer setNumChanges(int num) {
+ numChanges = num;
+ return this;
+ }
+
+ public ChangeBatchIndexer setProgressOut(OutputStream out) {
+ progressOut = checkNotNull(out);
+ return this;
+ }
+
+ public ChangeBatchIndexer setVerboseOut(OutputStream out) {
+ verboseWriter = new PrintWriter(checkNotNull(out));
+ return this;
+ }
+
+ public Result indexAll(ChangeIndex index,
+ Iterable<Project.NameKey> projects) {
Stopwatch sw = Stopwatch.createStarted();
final MultiProgressMonitor mpm =
new MultiProgressMonitor(progressOut, "Reindexing changes");
final Task projTask = mpm.beginSubTask("projects",
- numProjects >= 0 ? numProjects : MultiProgressMonitor.UNKNOWN);
+ (projects instanceof Collection)
+ ? ((Collection<?>) projects).size()
+ : MultiProgressMonitor.UNKNOWN);
final Task doneTask = mpm.beginSubTask(null,
numChanges >= 0 ? numChanges : MultiProgressMonitor.UNKNOWN);
final Task failedTask = mpm.beginSubTask("failed", MultiProgressMonitor.UNKNOWN);
