From 13b4b5082d5150626229fd0641e5fbe138fcdb93 Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Fri, 13 Nov 2015 14:25:17 -0800 Subject: [PATCH 1/3] Allow to use GWTORM Key classes in plugins To implement dedicated databases in plugin based on GWTORM, keys are needed. Currently they can only be used from classes located in the same package. Changing the modifiers to public allow the plugin to reuse them, without keep using the same package name as in Gerrit core. Change-Id: I5f321aa89dbb9b71945c4bc53151e3d5a26cce74 --- .../main/java/com/google/gerrit/reviewdb/client/Change.java | 2 +- .../java/com/google/gerrit/reviewdb/client/LabelId.java | 4 ++-- .../java/com/google/gerrit/reviewdb/client/PatchSet.java | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java index fe2cd968ff..2b1a7cfcc7 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java @@ -106,7 +106,7 @@ public final class Change { private static final long serialVersionUID = 1L; @Column(id = 1) - protected int id; + public int id; protected Id() { } diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/LabelId.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/LabelId.java index 9a159664be..f2af5fa6cb 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/LabelId.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/LabelId.java @@ -23,9 +23,9 @@ public class LabelId extends StringKey> { public static final LabelId SUBMIT = new LabelId("SUBM"); @Column(id = 1) - protected String id; + public String id; - protected LabelId() { + public LabelId() { } public LabelId(final String n) { diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSet.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSet.java index 48623bdfd1..2361b1c91c 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSet.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSet.java @@ -32,12 +32,12 @@ public final class PatchSet { private static final long serialVersionUID = 1L; @Column(id = 1) - protected Change.Id changeId; + public Change.Id changeId; @Column(id = 2) - protected int patchSetId; + public int patchSetId; - protected Id() { + public Id() { changeId = new Change.Id(); } From 62712be1d9261ec7f3fa8268c61d7949d3e750b2 Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Fri, 13 Nov 2015 18:17:06 -0800 Subject: [PATCH 2/3] Make JdbcUtil#port() public This allows us to use it from plugins. Change-Id: I6728ef75d48af078e488cf06f886902d8963c576 --- .../src/main/java/com/google/gerrit/server/schema/JdbcUtil.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/JdbcUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/JdbcUtil.java index 90ca43d8c3..2624923a5c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/JdbcUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/JdbcUtil.java @@ -26,7 +26,7 @@ public class JdbcUtil { return hostname; } - static String port(String port) { + public static String port(String port) { if (port != null && !port.isEmpty()) { return ":" + port; } From 27542ceec5afb4053f56e7ac5bd8c5783263f34a Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Wed, 18 Nov 2015 10:25:58 -0500 Subject: [PATCH 3/3] 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 --- .../server/git/ScanningChangeCacheImpl.java | 46 ++++++++++--------- .../gerrit/server/index/SiteIndexer.java | 7 +-- 2 files changed, 27 insertions(+), 26 deletions(-) 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 65808fcda2..faf37767d9 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.reviewdb.server.ReviewDb; 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.eclipse.jgit.lib.Repository; 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 @@ public class ScanningChangeCacheImpl implements ChangeCache { @Override public List load(Project.NameKey key) throws Exception { - Repository repo = repoManager.openRepository(key); - try (ManualRequestContext ctx = requestContext.open()) { - ReviewDb db = ctx.getReviewDbProvider().get(); - Map refs = - repo.getRefDatabase().getRefs(RefNames.REFS_CHANGES); - Set ids = new LinkedHashSet<>(); - for (Ref r : refs.values()) { - Change.Id id = Change.Id.fromRef(r.getName()); - if (id != null) { - ids.add(id); - } - } - List changes = new ArrayList<>(ids.size()); - // A batch size of N may overload get(Iterable), so use something smaller, - // but still >1. - for (List 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 scan(Repository repo, ReviewDb db) + throws OrmException, IOException { + Map refs = + repo.getRefDatabase().getRefs(RefNames.REFS_CHANGES); + Set ids = new LinkedHashSet<>(); + for (Ref r : refs.values()) { + Change.Id id = Change.Id.fromRef(r.getName()); + if (id != null) { + ids.add(id); + } + } + List changes = new ArrayList<>(ids.size()); + // A batch size of N may overload get(Iterable), so use something smaller, + // but still >1. + for (List 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 0897d6001f..6dfc847c2f 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.Change; 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 @@ public class SiteIndexer { } private final SchemaFactory schemaFactory; - private final ChangeCache changeCache; private final ChangeData.Factory changeDataFactory; private final GitRepositoryManager repoManager; private final ListeningExecutorService executor; @@ -126,14 +125,12 @@ public class SiteIndexer { @Inject SiteIndexer(SchemaFactory 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 @@ public class SiteIndexer { repo = repoManager.openRepository(project); Map 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));