SiteIndexer: Always scan changes from repo/db This was changed to use ChangeCache in a2b7358d, with the intention of binding ChangeCache to ScanningChangeCacheImpl in Reindex. Unfortunately I forgot about the online reindexing case, where ChangeCache is bound to SearchingChangeCacheImpl in a running master. This had the effect of omitting from the list of changes to reindex any changes that were not already present in the index. In many cases this is fine, but if a change is missing for some reason (including a race condition during creation), it would not be found. Fix this by exposing the method in ScanningChangeCacheImpl to actually scan a repo, and using that from both its Loader and SiteIndexer directly. Change-Id: Id384ff93abbb7b6c54819b6c2672fb1f45dc55bc
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ScanningChangeCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ScanningChangeCacheImpl.java index 65808fc..faf3776 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ScanningChangeCacheImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ScanningChangeCacheImpl.java
@@ -26,6 +26,7 @@ import com.google.gerrit.server.cache.CacheModule; import com.google.gerrit.server.util.ManualRequestContext; import com.google.gerrit.server.util.OneOffRequestContext; +import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Module; import com.google.inject.Singleton; @@ -37,6 +38,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.io.IOException; import java.util.ArrayList; import java.util.Collections; import java.util.LinkedHashSet; @@ -95,29 +97,31 @@ @Override public List<Change> load(Project.NameKey key) throws Exception { - Repository repo = repoManager.openRepository(key); - try (ManualRequestContext ctx = requestContext.open()) { - ReviewDb db = ctx.getReviewDbProvider().get(); - Map<String, Ref> refs = - repo.getRefDatabase().getRefs(RefNames.REFS_CHANGES); - Set<Change.Id> ids = new LinkedHashSet<>(); - for (Ref r : refs.values()) { - Change.Id id = Change.Id.fromRef(r.getName()); - if (id != null) { - ids.add(id); - } - } - List<Change> changes = new ArrayList<>(ids.size()); - // A batch size of N may overload get(Iterable), so use something smaller, - // but still >1. - for (List<Change.Id> batch : Iterables.partition(ids, 30)) { - Iterables.addAll(changes, db.changes().get(batch)); - } - return changes; - } finally { - repo.close(); + try (Repository repo = repoManager.openRepository(key); + ManualRequestContext ctx = requestContext.open()) { + return scan(repo, ctx.getReviewDbProvider().get()); } } } + + public static List<Change> scan(Repository repo, ReviewDb db) + throws OrmException, IOException { + Map<String, Ref> refs = + repo.getRefDatabase().getRefs(RefNames.REFS_CHANGES); + Set<Change.Id> ids = new LinkedHashSet<>(); + for (Ref r : refs.values()) { + Change.Id id = Change.Id.fromRef(r.getName()); + if (id != null) { + ids.add(id); + } + } + List<Change> changes = new ArrayList<>(ids.size()); + // A batch size of N may overload get(Iterable), so use something smaller, + // but still >1. + for (List<Change.Id> batch : Iterables.partition(ids, 30)) { + Iterables.addAll(changes, db.changes().get(batch)); + } + return changes; + } }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/SiteIndexer.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/SiteIndexer.java index 0897d60..6dfc847 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/SiteIndexer.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/SiteIndexer.java
@@ -33,11 +33,11 @@ import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.config.GerritServerConfig; -import com.google.gerrit.server.git.ChangeCache; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.MergeUtil; import com.google.gerrit.server.git.MultiProgressMonitor; import com.google.gerrit.server.git.MultiProgressMonitor.Task; +import com.google.gerrit.server.git.ScanningChangeCacheImpl; import com.google.gerrit.server.patch.PatchListLoader; import com.google.gerrit.server.query.change.ChangeData; import com.google.gwtorm.server.SchemaFactory; @@ -112,7 +112,6 @@ } private final SchemaFactory<ReviewDb> schemaFactory; - private final ChangeCache changeCache; private final ChangeData.Factory changeDataFactory; private final GitRepositoryManager repoManager; private final ListeningExecutorService executor; @@ -126,14 +125,12 @@ @Inject SiteIndexer(SchemaFactory<ReviewDb> schemaFactory, - ChangeCache changeCache, ChangeData.Factory changeDataFactory, GitRepositoryManager repoManager, @IndexExecutor(BATCH) ListeningExecutorService executor, ChangeIndexer.Factory indexerFactory, @GerritServerConfig Config config) { this.schemaFactory = schemaFactory; - this.changeCache = changeCache; this.changeDataFactory = changeDataFactory; this.repoManager = repoManager; this.executor = executor; @@ -241,7 +238,7 @@ repo = repoManager.openRepository(project); Map<String, Ref> refs = repo.getRefDatabase().getRefs(ALL); db = schemaFactory.open(); - for (Change c : changeCache.get(project)) { + for (Change c : ScanningChangeCacheImpl.scan(repo, db)) { Ref r = refs.get(c.currentPatchSetId().toRefName()); if (r != null) { byId.put(r.getObjectId(), changeDataFactory.create(db, c));