From 9660387c86982917b1b9fdd478ce6fb093894501 Mon Sep 17 00:00:00 2001 From: Kaushik Lingarkar Date: Wed, 7 Apr 2021 11:03:17 -0700 Subject: NoteDbMigrator: Warm PostgreSQL DB before migration Doing a 'select all' query on patch_comments, change_messages, patch_sets and patch_set_approvals warms the DB and helps make the individual queries done on these tables for each change execute faster. Note that, we only attempt to warm if the DB type is PostgreSQL as other types were untested. This change helped bring down the overall migration time of a site with ~150k changes(spread across 4 projects) from 105 mins to 45 mins. That's a ~55% improvement. If the DB was warmed externally before starting the migration, the overall time without this change is 42 mins and with this change is 43 mins. So, this change doesn't degrade the performance by much even if the DB was pre-warmed. Change-Id: I22174dce9830fe4947395e39c6a57496f77655a5 --- .../server/notedb/rebuild/NoteDbMigrator.java | 38 ++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java b/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java index 36b07e51ad..0b39a79c91 100644 --- a/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java +++ b/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java @@ -122,6 +122,10 @@ public class NoteDbMigrator implements AutoCloseable { private static final String AUTO_MIGRATE = "autoMigrate"; private static final String TRIAL = "trial"; + private static final String SECTION_DATABASE = "database"; + private static final String POSTGRESQL = "postgresql"; + private static final String URL = "url"; + private static final String TYPE = "type"; private static final int PROJECT_SLICE_MAX_REFS = 1000; private static final int GC_INTERVAL = 10000; @@ -142,6 +146,18 @@ public class NoteDbMigrator implements AutoCloseable { cfg.setBoolean(SECTION_NOTE_DB, NoteDbTable.CHANGES.key(), TRIAL, trial); } + public static boolean isDatabasePostgreSQL(Config cfg) { + String type = cfg.getString(SECTION_DATABASE, null, TYPE); + if (POSTGRESQL.equals(type)) { + return true; + } + String url = cfg.getString(SECTION_DATABASE, null, URL); + if (url != null && url.contains(POSTGRESQL)) { + return true; + } + return false; + } + private static class NoteDbMigrationLoggerOut extends OutputStream { private StringBuilder outputBuffer = new StringBuilder(); @@ -859,6 +875,26 @@ public class NoteDbMigrator implements AutoCloseable { return slices; } + public void warmReviewDb() throws OrmException { + if (isDatabasePostgreSQL(gerritConfig)) { + logger.atInfo().log("Warming PostgreSQL DB"); + Stopwatch sw = Stopwatch.createStarted(); + try (ReviewDb db = schemaFactory.open()) { + // PostgreSQL driver by default fetches all the data, even if we don't iterate over + // it, which is exactly the reason why these queries warm the DB and it is also the + // reason we restrict the warm to PostgreSQL only as drivers for other DB types will + // likely behave differently. We close the result sets after each query to free up + // memory as the entire data is fetched. + db.patchSetApprovals().all().close(); + db.changeMessages().all().close(); + db.patchSets().all().close(); + db.patchComments().all().close(); + } + logger.atInfo().log( + "PostgreSQL DB warm took %.01fs", sw.elapsed(TimeUnit.MILLISECONDS) / 1000d); + } + } + public void rebuild() throws MigrationException, OrmException { if (!globalNotesMigration.commitChangeWrites()) { throw new MigrationException("Cannot rebuild without noteDb.changes.write=true"); @@ -866,6 +902,8 @@ public class NoteDbMigrator implements AutoCloseable { Stopwatch sw = Stopwatch.createStarted(); logger.atInfo().log("Rebuilding changes in NoteDb"); + warmReviewDb(); + List slices = slice(); List> futures = new ArrayList<>(); for (ProjectSlice slice : slices) { -- cgit v1.2.3 From cff9c2a8d8e10a23e51619000efe6a02d8e00543 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sat, 5 Jun 2021 08:30:20 +0200 Subject: Update jgit to 5.1.16.202106041830-r This update contains - 8bc166b00 - BatchRefUpdate: Skip saving conflicting ref names and prefixes in memory - 294a99af2 - BatchRefUpdateTest: Accurately assert RefsChangedEvent(s) fired - 303dd019d - Optimize RefDirectory.isNameConflicting() Change-Id: I570ddc05718290380fe1b3d3dcfb92ee18335b41 --- lib/jgit/jgit.bzl | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/jgit/jgit.bzl b/lib/jgit/jgit.bzl index eafb37b18e..6ee9c29740 100644 --- a/lib/jgit/jgit.bzl +++ b/lib/jgit/jgit.bzl @@ -1,6 +1,6 @@ load("//tools/bzl:maven_jar.bzl", "MAVEN_CENTRAL", "maven_jar") -_JGIT_VERS = "5.1.15.202012011955-r" +_JGIT_VERS = "5.1.16.202106041830-r" _DOC_VERS = _JGIT_VERS # Set to _JGIT_VERS unless using a snapshot @@ -40,28 +40,28 @@ def jgit_maven_repos(): name = "jgit-lib", artifact = "org.eclipse.jgit:org.eclipse.jgit:" + _JGIT_VERS, repository = _JGIT_REPO, - sha1 = "ca4a5fbd38cfad3ad386b46464b0e753eca3baea", - src_sha1 = "39a4180b70ac3024d68ee9781198e88abf567ccf", + sha1 = "1b32273b9b8326a14355374702799b6cd4a94050", + src_sha1 = "62a010fe8e0de9c4684348f3853da3c817257165", unsign = True, ) maven_jar( name = "jgit-servlet", artifact = "org.eclipse.jgit:org.eclipse.jgit.http.server:" + _JGIT_VERS, repository = _JGIT_REPO, - sha1 = "698fa84e35787eab380efadc2df52706dbac5268", + sha1 = "f375f3c6cfe37096ee984d3a2f380817446ac5f0", unsign = True, ) maven_jar( name = "jgit-archive", artifact = "org.eclipse.jgit:org.eclipse.jgit.archive:" + _JGIT_VERS, repository = _JGIT_REPO, - sha1 = "27c731c41c8cb45e6bccdaac14311bda1d724437", + sha1 = "20378bb3138a9ab8d2e13036a2f3e4719caad7a2", ) maven_jar( name = "jgit-junit", artifact = "org.eclipse.jgit:org.eclipse.jgit.junit:" + _JGIT_VERS, repository = _JGIT_REPO, - sha1 = "6f961edbfecf5928c87d46b08acf2e1625a53089", + sha1 = "9563231cf411841a78f9bb703782b0f0a716d420", unsign = True, ) -- cgit v1.2.3 From ef8e8cfd35412f9ddd04447e9e108851faf8040d Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Tue, 13 Jul 2021 02:20:51 +0100 Subject: OnlineNoteDbMigration: allow per-project migration The per-project migration to NoteDb was missing the online reindexing option which it was making it unusable for a zero-downtime deployment. Export the same --project option on the notedb config as well so that existing sites can start converting to NoteDb on a per-project basis. Bug: Issue 14777 Change-Id: I31bcded11ac2c65c492ef7ae1c851954a86318cd --- Documentation/config-gerrit.txt | 12 +++++++ .../notedb/rebuild/OnlineNoteDbMigrator.java | 42 ++++++++++++++++------ .../server/notedb/OnlineNoteDbMigrationIT.java | 35 ++++++++++++++++++ 3 files changed, 79 insertions(+), 10 deletions(-) diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 223d905393..aff17cac82 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -3719,6 +3719,18 @@ each process retrieves at once. + By default, 1. +[[notedb.onlineMigrationProjects]] notedb.onlineMigrationProjects:: ++ +Limit the online migration from reviewDb to noteDb to a list of projects. ++ +This allows to experiment the migration to noteDb on a per-project basis +for debugging problems and without having to interrupt the service on active +projects. ++ +When not defined, all projects are migrated to noteDb. ++ +By default, undefined. + [[notedb.onlineMigrationThreads]] notedb.onlineMigrationThreads:: + The number of threads used to run the online migration from reviewDb to noteDb. diff --git a/java/com/google/gerrit/server/notedb/rebuild/OnlineNoteDbMigrator.java b/java/com/google/gerrit/server/notedb/rebuild/OnlineNoteDbMigrator.java index 46f5b22e91..fbcc8f5997 100644 --- a/java/com/google/gerrit/server/notedb/rebuild/OnlineNoteDbMigrator.java +++ b/java/com/google/gerrit/server/notedb/rebuild/OnlineNoteDbMigrator.java @@ -16,19 +16,24 @@ package com.google.gerrit.server.notedb.rebuild; import static com.google.gerrit.server.notedb.NotesMigration.SECTION_NOTE_DB; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Stopwatch; import com.google.common.flogger.FluentLogger; import com.google.gerrit.extensions.events.LifecycleListener; import com.google.gerrit.lifecycle.LifecycleModule; +import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.index.OnlineUpgrader; import com.google.gerrit.server.index.VersionManager; +import com.google.gerrit.server.notedb.rebuild.NoteDbMigrator.Builder; import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.Singleton; import com.google.inject.name.Named; import com.google.inject.name.Names; +import java.util.Arrays; import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; import org.eclipse.jgit.lib.Config; @Singleton @@ -39,6 +44,8 @@ public class OnlineNoteDbMigrator implements LifecycleListener { private static final String ONLINE_MIGRATION_THREADS = "onlineMigrationThreads"; + private static final String ONLINE_MIGRATION_PROJECTS = "onlineMigrationProjects"; + public static class Module extends LifecycleModule { private final boolean trial; @@ -59,6 +66,7 @@ public class OnlineNoteDbMigrator implements LifecycleListener { private final boolean upgradeIndex; private final boolean trial; private final int threads; + private final String[] projects; @Inject OnlineNoteDbMigrator( @@ -73,6 +81,7 @@ public class OnlineNoteDbMigrator implements LifecycleListener { this.upgradeIndex = VersionManager.getOnlineUpgrade(cfg); this.trial = trial || NoteDbMigrator.getTrialMode(cfg); this.threads = cfg.getInt(SECTION_NOTE_DB, ONLINE_MIGRATION_THREADS, 1); + this.projects = cfg.getStringList(SECTION_NOTE_DB, null, ONLINE_MIGRATION_PROJECTS); } @Override @@ -83,22 +92,35 @@ public class OnlineNoteDbMigrator implements LifecycleListener { t.start(); } - private void migrate() { + @VisibleForTesting + public void migrate() { logger.atInfo().log("Starting online NoteDb migration"); if (upgradeIndex) { logger.atInfo().log( "Online index schema upgrades will be deferred until NoteDb migration is complete"); } Stopwatch sw = Stopwatch.createStarted(); - // TODO(dborowitz): maybe expose a progress monitor somewhere. - try (NoteDbMigrator migrator = - migratorBuilderProvider - .get() - .setThreads(threads) - .setAutoMigrate(true) - .setTrialMode(trial) - .build()) { - migrator.migrate(); + + Builder migratorBuilder = + migratorBuilderProvider.get().setThreads(threads).setAutoMigrate(true).setTrialMode(trial); + + try { + // TODO(dborowitz): maybe expose a progress monitor somewhere. + if (projects.length > 0) { + try (NoteDbMigrator migrator = + migratorBuilder + .setProjects( + Arrays.asList(projects).stream() + .map(Project.NameKey::new) + .collect(Collectors.toList())) + .build()) { + migrator.rebuild(); + } + } else { + try (NoteDbMigrator migrator = migratorBuilder.build()) { + migrator.migrate(); + } + } } catch (Exception e) { logger.atSevere().withCause(e).log("Error in online NoteDb migration"); } diff --git a/javatests/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java b/javatests/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java index b9ed0f3ac7..8f0506bcf7 100644 --- a/javatests/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java +++ b/javatests/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java @@ -43,6 +43,7 @@ import com.google.gerrit.acceptance.NoHttpd; import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.acceptance.Sandboxed; import com.google.gerrit.acceptance.UseLocalDisk; +import com.google.gerrit.extensions.api.projects.ProjectInput; import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.extensions.registration.RegistrationHandle; import com.google.gerrit.reviewdb.client.Change; @@ -62,12 +63,14 @@ import com.google.gerrit.server.notedb.NotesMigrationState; import com.google.gerrit.server.notedb.rebuild.MigrationException; import com.google.gerrit.server.notedb.rebuild.NoteDbMigrator; import com.google.gerrit.server.notedb.rebuild.NotesMigrationStateListener; +import com.google.gerrit.server.notedb.rebuild.OnlineNoteDbMigrator; import com.google.gerrit.server.schema.ReviewDbFactory; import com.google.gerrit.testing.ConfigSuite; import com.google.gerrit.testing.NoteDbMode; import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.SchemaFactory; import com.google.inject.Inject; +import com.google.inject.Injector; import com.google.inject.Provider; import java.io.IOException; import java.nio.file.Files; @@ -322,6 +325,38 @@ public class OnlineNoteDbMigrationIT extends AbstractDaemonTest { assertRebuilt(id2); } + @Test + @GerritConfig(name = "noteDb.onlineMigrationProjects", value = "project2") + @UseLocalDisk + public void rebuildSubsetOfProjectsFromConfig() throws Exception { + Injector injector = + server.getTestInjector().createChildInjector(new OnlineNoteDbMigrator.Module(true)); + OnlineNoteDbMigrator onlineNoteDbMigrator = injector.getInstance(OnlineNoteDbMigrator.class); + + setNotesMigrationState(WRITE); + + ProjectInput in = new ProjectInput(); + in.name = "project2"; + in.parent = allProjects.get(); + in.createEmptyCommit = true; + gApi.projects().create(in); + Project.NameKey p2 = new Project.NameKey(in.name); + + TestRepository tr2 = cloneProject(p2, admin); + + PushOneCommit.Result r1 = createChange(); + PushOneCommit.Result r2 = pushFactory.create(db, admin.getIdent(), tr2).to("refs/for/master"); + Change.Id id1 = r1.getChange().getId(); + Change.Id id2 = r2.getChange().getId(); + + invalidateNoteDbState(id1, id2); + + onlineNoteDbMigrator.migrate(); + + assertNotRebuilt(id1); + assertRebuilt(id2); + } + @Test public void rebuildNonSkippedProjects() throws Exception { setNotesMigrationState(WRITE); -- cgit v1.2.3 From 11667a12328610224d2934ae050860a4c10e63e3 Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Mon, 30 Sep 2019 09:53:47 +0200 Subject: OutgoingEmail: Handle null accountId in getNameEmailFor(Account.Id) method If an email should be sent and a current user is not available (e.g. because the operation is executed in the background) we cannot set an account ID as the sender of the email. In this case we skip calling OutgoingEmail#setFrom(Account.Id) and the fromId stays null (e.g. see AbandonOp which can be executed in the background). If the fromId is null and we setup the soy context (ChangeEmail#setupSoyContext()) we get a NullPointerException when trying to set the fromEmail field. This is because getNameEmailFor(Account.Id) doesn't expect null as value. In contrast to this method, the getNameFor(Account.Id) method which is called before to set the fromName field does handle null as value by falling back to the Gerrit person ident. Fix the NullPointerException in getNameEmailFor(Account.Id) by doing the same, and make it also fall back to the Gerrit person ident if the given account ID is null. Signed-off-by: Edwin Kempin Change-Id: I63d50f4407b4f95eaaf73e41c5a1288ed4a16eb4 (cherry picked from commit 32cf19ad46ff5e3984e794544a9ae524fa4ab3e9) --- java/com/google/gerrit/server/mail/send/OutgoingEmail.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/java/com/google/gerrit/server/mail/send/OutgoingEmail.java b/java/com/google/gerrit/server/mail/send/OutgoingEmail.java index 664c1bf8a5..5b131692fb 100644 --- a/java/com/google/gerrit/server/mail/send/OutgoingEmail.java +++ b/java/com/google/gerrit/server/mail/send/OutgoingEmail.java @@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.ListMultimap; import com.google.common.collect.Sets; import com.google.common.flogger.FluentLogger; +import com.google.gerrit.common.Nullable; import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.api.changes.RecipientType; @@ -340,7 +341,7 @@ public abstract class OutgoingEmail { } /** Lookup a human readable name for an account, usually the "full name". */ - protected String getNameFor(Account.Id accountId) { + protected String getNameFor(@Nullable Account.Id accountId) { if (accountId == null) { return args.gerritPersonIdent.getName(); } @@ -366,7 +367,14 @@ public abstract class OutgoingEmail { * @param accountId user to fetch. * @return name/email of account, or Anonymous Coward if unset. */ - protected String getNameEmailFor(Account.Id accountId) { + protected String getNameEmailFor(@Nullable Account.Id accountId) { + if (accountId == null) { + return args.gerritPersonIdent.getName() + + " <" + + args.gerritPersonIdent.getEmailAddress() + + ">"; + } + Optional account = args.accountCache.get(accountId).map(AccountState::getAccount); if (account.isPresent()) { String name = account.get().getFullName(); -- cgit v1.2.3 From 8aad9f8339f26941bfcef379d97642bd2046d5a7 Mon Sep 17 00:00:00 2001 From: Kaushik Lingarkar Date: Thu, 8 Apr 2021 15:38:57 -0700 Subject: NoteDbMigrator: Make GC on repositories optional GC during migration can become an overhead. Making it an option helps skip it as needed. For example, in one of our sites(with 150k changes), migration with GC takes 145 mins whereas without GC it finishes in 105 mins. Change-Id: Ibd61365f87a4d7fbb5d62ffbe4f563f675e000c5 --- java/com/google/gerrit/pgm/MigrateToNoteDb.java | 4 ++++ .../server/notedb/rebuild/NoteDbMigrator.java | 23 +++++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/java/com/google/gerrit/pgm/MigrateToNoteDb.java b/java/com/google/gerrit/pgm/MigrateToNoteDb.java index 247aaf9f64..5778792f28 100644 --- a/java/com/google/gerrit/pgm/MigrateToNoteDb.java +++ b/java/com/google/gerrit/pgm/MigrateToNoteDb.java @@ -89,6 +89,9 @@ public class MigrateToNoteDb extends SiteProgram { usage = "Force state change of the migration if projects are skipped") private boolean forceStateChangeWithSkip; + @Option(name = "--gc", usage = "GC repositories regularly during the migration") + private boolean gc; + @Option(name = "--trial", usage = TRIAL_USAGE) private boolean trial; @@ -151,6 +154,7 @@ public class MigrateToNoteDb extends SiteProgram { .setTrialMode(trial) .setForceRebuild(force) .setForceStateChangeWithSkip(forceStateChangeWithSkip) + .setGC(gc) .setSequenceGap(sequenceGap) .setVerbose(verbose) .build()) { diff --git a/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java b/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java index 0b39a79c91..87a9d07bd7 100644 --- a/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java +++ b/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java @@ -200,6 +200,7 @@ public class NoteDbMigrator implements AutoCloseable { private boolean trial; private boolean forceRebuild; private boolean forceStateChangeWithSkip; + private boolean gc; private int sequenceGap = -1; private boolean autoMigrate; private boolean verbose; @@ -381,6 +382,22 @@ public class NoteDbMigrator implements AutoCloseable { return this; } + /** + * GC repositories regularly during noteDb migration. + * + *

This might help improve performance in some instances. If enabled, auto GC will be run for + * every new 10000 refs which are created during the migration of a project. Auto GC will do + * garbage collection by default, if it finds more than 6700 loose objects or more than 50 pack + * files. These defaults can be changed by updating gc.auto and gc.autoPackLimit. + * + * @param gc whether GC must be done on repositories during migration + * @return this. + */ + public Builder setGC(boolean gc) { + this.gc = gc; + return this; + } + /** * Gap between ReviewDb change sequence numbers and NoteDb. * @@ -460,6 +477,7 @@ public class NoteDbMigrator implements AutoCloseable { trial, forceRebuild, forceStateChangeWithSkip, + gc, sequenceGap >= 0 ? sequenceGap : Sequences.getChangeSequenceGap(cfg), autoMigrate, verbose); @@ -523,6 +541,7 @@ public class NoteDbMigrator implements AutoCloseable { private final boolean trial; private final boolean forceRebuild; private final boolean forceStateChangeWithSkip; + private final boolean gc; private final int sequenceGap; private final boolean autoMigrate; private final boolean verbose; @@ -555,6 +574,7 @@ public class NoteDbMigrator implements AutoCloseable { boolean trial, boolean forceRebuild, boolean forceStateChangeWithSkip, + boolean gc, int sequenceGap, boolean autoMigrate, boolean verbose) @@ -591,6 +611,7 @@ public class NoteDbMigrator implements AutoCloseable { this.trial = trial; this.forceRebuild = forceRebuild; this.forceStateChangeWithSkip = forceStateChangeWithSkip; + this.gc = gc; this.sequenceGap = sequenceGap; this.autoMigrate = autoMigrate; this.verbose = verbose; @@ -1064,7 +1085,7 @@ public class NoteDbMigrator implements AutoCloseable { c, totalChangeCount, (100.0 * c) / totalChangeCount); } pc = ctx.changesMigratedCount.incrementAndGet(); - if (pc % GC_INTERVAL == 0) { + if (gc && pc % GC_INTERVAL == 0) { gc(project, changeRepo, ctx.gcLock); } pm.update(1); -- cgit v1.2.3 From 66fb51e5932a0660b448c3dab35ca0316424a95c Mon Sep 17 00:00:00 2001 From: Kaushik Lingarkar Date: Wed, 4 Aug 2021 13:00:53 -0700 Subject: NoteDbMigrator: Make shuffling project slices optional This might help reduce memory allocation if the site has few large and many small repositories. It mixes slices from large and small repositories to reduce overall memory allocation, so that the number of migration threads can be increased without driving java GC crazy. However, it has the down-side of reducing disk cache hits. Change-Id: Ia2242d0cc1b1b305a32cf3ff7167bf398b9d81f5 --- java/com/google/gerrit/pgm/MigrateToNoteDb.java | 9 ++++++++ .../server/notedb/rebuild/NoteDbMigrator.java | 25 +++++++++++++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/java/com/google/gerrit/pgm/MigrateToNoteDb.java b/java/com/google/gerrit/pgm/MigrateToNoteDb.java index 5778792f28..e9290a2046 100644 --- a/java/com/google/gerrit/pgm/MigrateToNoteDb.java +++ b/java/com/google/gerrit/pgm/MigrateToNoteDb.java @@ -92,6 +92,14 @@ public class MigrateToNoteDb extends SiteProgram { @Option(name = "--gc", usage = "GC repositories regularly during the migration") private boolean gc; + @Option( + name = "--shuffle-project-slices", + usage = + "Shuffle project slices to reduce memory" + + " allocation. This is especially useful if the site has a few large and many small" + + " repositories.") + private boolean shuffleProjectSlices; + @Option(name = "--trial", usage = TRIAL_USAGE) private boolean trial; @@ -155,6 +163,7 @@ public class MigrateToNoteDb extends SiteProgram { .setForceRebuild(force) .setForceStateChangeWithSkip(forceStateChangeWithSkip) .setGC(gc) + .setShuffleProjectSlices(shuffleProjectSlices) .setSequenceGap(sequenceGap) .setVerbose(verbose) .build()) { diff --git a/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java b/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java index 87a9d07bd7..2f569e3399 100644 --- a/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java +++ b/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java @@ -201,6 +201,7 @@ public class NoteDbMigrator implements AutoCloseable { private boolean forceRebuild; private boolean forceStateChangeWithSkip; private boolean gc; + private boolean shuffleProjectSlices; private int sequenceGap = -1; private boolean autoMigrate; private boolean verbose; @@ -398,6 +399,22 @@ public class NoteDbMigrator implements AutoCloseable { return this; } + /** + * Shuffle project slices during the rebuild phase of noteDb migration. + * + *

This might help reduce memory allocation if the site has few large and many small + * repositories. It mixes slices from large and small repositories to reduce overall memory + * allocation, so that the number of migration threads can be increased without driving java GC + * crazy. However, it has the down-side of reducing disk cache hits. + * + * @param shuffleProjectSlices whether project slices must be shuffled during migration + * @return this. + */ + public Builder setShuffleProjectSlices(boolean shuffleProjectSlices) { + this.shuffleProjectSlices = shuffleProjectSlices; + return this; + } + /** * Gap between ReviewDb change sequence numbers and NoteDb. * @@ -478,6 +495,7 @@ public class NoteDbMigrator implements AutoCloseable { forceRebuild, forceStateChangeWithSkip, gc, + shuffleProjectSlices, sequenceGap >= 0 ? sequenceGap : Sequences.getChangeSequenceGap(cfg), autoMigrate, verbose); @@ -542,6 +560,7 @@ public class NoteDbMigrator implements AutoCloseable { private final boolean forceRebuild; private final boolean forceStateChangeWithSkip; private final boolean gc; + private final boolean shuffleProjectSlices; private final int sequenceGap; private final boolean autoMigrate; private final boolean verbose; @@ -575,6 +594,7 @@ public class NoteDbMigrator implements AutoCloseable { boolean forceRebuild, boolean forceStateChangeWithSkip, boolean gc, + boolean shuffleProjectSlices, int sequenceGap, boolean autoMigrate, boolean verbose) @@ -612,6 +632,7 @@ public class NoteDbMigrator implements AutoCloseable { this.forceRebuild = forceRebuild; this.forceStateChangeWithSkip = forceStateChangeWithSkip; this.gc = gc; + this.shuffleProjectSlices = shuffleProjectSlices; this.sequenceGap = sequenceGap; this.autoMigrate = autoMigrate; this.verbose = verbose; @@ -891,7 +912,9 @@ public class NoteDbMigrator implements AutoCloseable { slices.add(ps); } } - Collections.shuffle(slices); + if (shuffleProjectSlices) { + Collections.shuffle(slices); + } totalChangeCount = changesByProject.size(); return slices; } -- cgit v1.2.3 From 06c00956b35a34c35409d365637f54965274da1c Mon Sep 17 00:00:00 2001 From: Marcin Czech Date: Wed, 4 Aug 2021 11:21:37 +0200 Subject: OutgoingEmail: Handle null accountId in getUserNameEmailFor(Account.Id) If an email should be sent and a current user is not available (e.g. because the operation is executed in the background) we cannot set an account ID as the sender of the email. This cause NPE when retrieving account state from account cache. Add check if account id is not null before accessing account cache. Bug: Issue 14852 Change-Id: I0181164acd7668bd101b67995c9135207950f3c6 --- java/com/google/gerrit/server/mail/send/OutgoingEmail.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/java/com/google/gerrit/server/mail/send/OutgoingEmail.java b/java/com/google/gerrit/server/mail/send/OutgoingEmail.java index 5b131692fb..cd6882fc0e 100644 --- a/java/com/google/gerrit/server/mail/send/OutgoingEmail.java +++ b/java/com/google/gerrit/server/mail/send/OutgoingEmail.java @@ -395,9 +395,13 @@ public abstract class OutgoingEmail { * username. If no username is set, this function returns null. * * @param accountId user to fetch. - * @return name/email of account, username, or null if unset. + * @return name/email of account, username, or null if unset or the accountId is null. */ - protected String getUserNameEmailFor(Account.Id accountId) { + protected String getUserNameEmailFor(@Nullable Account.Id accountId) { + if (accountId == null) { + return null; + } + Optional accountState = args.accountCache.get(accountId); if (!accountState.isPresent()) { return null; -- cgit v1.2.3 From c9322bf319e6376e3ef27c87a75e189bec3dedd6 Mon Sep 17 00:00:00 2001 From: Adithya Chakilam Date: Thu, 12 Aug 2021 12:00:53 -0500 Subject: Introduce flag '--migrate-draft-to' This would help remove the manual intervention during Schema_159 migration. Change-Id: Ibb4df8be4c1d2fa9910457a6b352bb3056dc3fc5 --- gerrit-pgm/src/main/java/com/google/gerrit/pgm/Init.java | 14 ++++++++++++++ .../src/main/java/com/google/gerrit/pgm/init/BaseInit.java | 11 +++++++++++ .../java/com/google/gerrit/pgm/init/api/InitFlags.java | 4 ++++ .../java/com/google/gerrit/server/schema/Schema_159.java | 10 ++++++++-- .../java/com/google/gerrit/server/schema/UpdateUI.java | 4 ++++ .../test/java/com/google/gerrit/testutil/TestUpdateUI.java | 6 ++++++ 6 files changed, 47 insertions(+), 2 deletions(-) diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Init.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Init.java index b9c70684dd..2c034ead8a 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Init.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Init.java @@ -26,6 +26,7 @@ import com.google.gerrit.pgm.init.api.ConsoleUI; import com.google.gerrit.pgm.util.ErrorLogFile; import com.google.gerrit.server.config.GerritServerConfigModule; import com.google.gerrit.server.config.SitePath; +import com.google.gerrit.server.schema.Schema_159.DraftWorkflowMigrationStrategy; import com.google.gerrit.server.securestore.SecureStoreClassName; import com.google.gerrit.server.util.HostPlatform; import com.google.inject.AbstractModule; @@ -81,6 +82,14 @@ public class Init extends BaseInit { @Option(name = "--skip-download", usage = "Don't download given library") private List skippedDownloads; + @Option( + name = "--migrate-draft-to", + usage = + "Strategy to migrate draft changes during Schema 159 migration(private or work_in_progress)." + + " Applicable only when migrating from a version lower than 2.15") + private DraftWorkflowMigrationStrategy draftMigrationStrategy = + DraftWorkflowMigrationStrategy.WORK_IN_PROGRESS; + @Inject Browser browser; public Init() { @@ -185,6 +194,11 @@ public class Init extends BaseInit { return skippedDownloads != null ? skippedDownloads : Collections.emptyList(); } + @Override + protected DraftWorkflowMigrationStrategy getDraftMigrationStrategy() { + return draftMigrationStrategy; + } + @Override protected String getSecureStoreLib() { return secureStoreLib; diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/BaseInit.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/BaseInit.java index e707243c11..93943ed01e 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/BaseInit.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/BaseInit.java @@ -43,6 +43,7 @@ import com.google.gerrit.server.index.IndexModule; import com.google.gerrit.server.plugins.JarScanner; import com.google.gerrit.server.schema.ReviewDbFactory; import com.google.gerrit.server.schema.SchemaUpdater; +import com.google.gerrit.server.schema.Schema_159.DraftWorkflowMigrationStrategy; import com.google.gerrit.server.schema.UpdateUI; import com.google.gerrit.server.securestore.SecureStore; import com.google.gerrit.server.securestore.SecureStoreClassName; @@ -132,6 +133,7 @@ public class BaseInit extends SiteProgram { init.flags.skipPlugins = skipPlugins(); init.flags.deleteCaches = getDeleteCaches(); init.flags.isNew = init.site.isNew; + init.flags.draftMigrationStrategy = getDraftMigrationStrategy(); final SiteRun run; try { @@ -429,6 +431,11 @@ public class BaseInit extends SiteProgram { } } } + + @Override + public DraftWorkflowMigrationStrategy getDraftMigrationStrategy() { + return flags.draftMigrationStrategy; + } }); if (!pruneList.isEmpty()) { @@ -535,4 +542,8 @@ public class BaseInit extends SiteProgram { protected boolean getDeleteCaches() { return false; } + + protected DraftWorkflowMigrationStrategy getDraftMigrationStrategy() { + return DraftWorkflowMigrationStrategy.WORK_IN_PROGRESS; + } } diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/api/InitFlags.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/api/InitFlags.java index 7bb1366bb0..2b8e574590 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/api/InitFlags.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/api/InitFlags.java @@ -16,6 +16,7 @@ package com.google.gerrit.pgm.init.api; import com.google.common.annotations.VisibleForTesting; import com.google.gerrit.server.config.SitePaths; +import com.google.gerrit.server.schema.Schema_159.DraftWorkflowMigrationStrategy; import com.google.gerrit.server.securestore.SecureStore; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -46,6 +47,9 @@ public class InitFlags { /** Dev mode */ public boolean dev; + /** Used for Schema 159 Migration */ + public DraftWorkflowMigrationStrategy draftMigrationStrategy; + public final FileBasedConfig cfg; public final SecureStore sec; public final List installPlugins; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_159.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_159.java index f97453e6ee..95000e4e2c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_159.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_159.java @@ -23,7 +23,7 @@ import com.google.inject.Provider; /** Migrate draft changes to private or wip changes. */ public class Schema_159 extends SchemaVersion { - private static enum DraftWorkflowMigrationStrategy { + public static enum DraftWorkflowMigrationStrategy { PRIVATE, WORK_IN_PROGRESS } @@ -36,7 +36,13 @@ public class Schema_159 extends SchemaVersion { @Override protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException { DraftWorkflowMigrationStrategy strategy = DraftWorkflowMigrationStrategy.WORK_IN_PROGRESS; - if (ui.yesno(false, "Migrate draft changes to private changes (default is work-in-progress)")) { + boolean migrateDraftToPrivate = + ui.getDraftMigrationStrategy() == DraftWorkflowMigrationStrategy.PRIVATE; + if (ui.yesno( + migrateDraftToPrivate, + "Migrate draft changes to private changes (default is " + + (migrateDraftToPrivate ? "private" : "work-in-progress") + + ")")) { strategy = DraftWorkflowMigrationStrategy.PRIVATE; } ui.message( diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/UpdateUI.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/UpdateUI.java index 0c02607214..2d7db64842 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/UpdateUI.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/UpdateUI.java @@ -14,6 +14,7 @@ package com.google.gerrit.server.schema; +import com.google.gerrit.server.schema.Schema_159.DraftWorkflowMigrationStrategy; import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.StatementExecutor; import java.util.List; @@ -39,4 +40,7 @@ public interface UpdateUI { boolean isBatch(); void pruneSchema(StatementExecutor e, List pruneList) throws OrmException; + + /** Used for Schema_159 migration. */ + DraftWorkflowMigrationStrategy getDraftMigrationStrategy(); } diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/TestUpdateUI.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/TestUpdateUI.java index d4acbcbd61..8dea4e44c1 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/testutil/TestUpdateUI.java +++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/TestUpdateUI.java @@ -14,6 +14,7 @@ package com.google.gerrit.testutil; +import com.google.gerrit.server.schema.Schema_159.DraftWorkflowMigrationStrategy; import com.google.gerrit.server.schema.UpdateUI; import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.StatementExecutor; @@ -48,4 +49,9 @@ public class TestUpdateUI implements UpdateUI { e.execute(sql); } } + + @Override + public DraftWorkflowMigrationStrategy getDraftMigrationStrategy() { + return DraftWorkflowMigrationStrategy.WORK_IN_PROGRESS; + } } -- cgit v1.2.3 From f7007ef2665238b66fdc5f152b353814242e54b2 Mon Sep 17 00:00:00 2001 From: Kaushik Lingarkar Date: Wed, 13 Oct 2021 10:35:14 -0700 Subject: Update schemas 115,139,144 to ignore entries not in the 'accounts' table Creating user refs for accounts available in the tables 'account_diff_preferences', account_project_watches and account_external_ids tables, but not in 'accounts' table is undesirable. Such entries must be ignored as they point towards data inconsistency. Updates to schemas 115,139 and 144 impact the upgrade of Gerrit to versions 2.12, 2.14 and 2.15 respectively. Change-Id: Iaac3fb67ff7ae0ff19ad72599bab6dbfaf164a1f --- java/com/google/gerrit/server/schema/Schema_115.java | 5 ++++- java/com/google/gerrit/server/schema/Schema_139.java | 5 +++-- java/com/google/gerrit/server/schema/Schema_144.java | 5 +++-- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/java/com/google/gerrit/server/schema/Schema_115.java b/java/com/google/gerrit/server/schema/Schema_115.java index 70bc9215f4..d04723b2ac 100644 --- a/java/com/google/gerrit/server/schema/Schema_115.java +++ b/java/com/google/gerrit/server/schema/Schema_115.java @@ -72,7 +72,10 @@ public class Schema_115 extends SchemaVersion { protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException, SQLException { Map imports = new HashMap<>(); try (Statement stmt = ((JdbcSchema) db).getConnection().createStatement(); - ResultSet rs = stmt.executeQuery("SELECT * FROM account_diff_preferences")) { + ResultSet rs = + stmt.executeQuery( + "SELECT * FROM account_diff_preferences JOIN accounts ON " + + "account_diff_preferences.id=accounts.account_id")) { Set availableColumns = getColumns(rs); while (rs.next()) { Account.Id accountId = new Account.Id(rs.getInt("id")); diff --git a/java/com/google/gerrit/server/schema/Schema_139.java b/java/com/google/gerrit/server/schema/Schema_139.java index cdde7e4b22..cd018b7cbe 100644 --- a/java/com/google/gerrit/server/schema/Schema_139.java +++ b/java/com/google/gerrit/server/schema/Schema_139.java @@ -81,7 +81,7 @@ public class Schema_139 extends SchemaVersion { ResultSet rs = stmt.executeQuery( "SELECT " - + "account_id, " + + "account_project_watches.account_id, " + "project_name, " + "filter, " + "notify_abandoned_changes, " @@ -89,7 +89,8 @@ public class Schema_139 extends SchemaVersion { + "notify_new_changes, " + "notify_new_patch_sets, " + "notify_submitted_changes " - + "FROM account_project_watches")) { + + "FROM account_project_watches " + + "JOIN accounts ON account_project_watches.account_id=accounts.account_id")) { while (rs.next()) { Account.Id accountId = new Account.Id(rs.getInt(1)); ProjectWatch.Builder b = diff --git a/java/com/google/gerrit/server/schema/Schema_144.java b/java/com/google/gerrit/server/schema/Schema_144.java index bb0cbcafdb..7a00749e01 100644 --- a/java/com/google/gerrit/server/schema/Schema_144.java +++ b/java/com/google/gerrit/server/schema/Schema_144.java @@ -63,11 +63,12 @@ public class Schema_144 extends SchemaVersion { ResultSet rs = stmt.executeQuery( "SELECT " - + "account_id, " + + "account_external_ids.account_id, " + "email_address, " + "password, " + "external_id " - + "FROM account_external_ids")) { + + "FROM account_external_ids " + + "JOIN accounts ON account_external_ids.account_id=accounts.account_id")) { while (rs.next()) { Account.Id accountId = new Account.Id(rs.getInt(1)); String email = rs.getString(2); -- cgit v1.2.3 From 723f67ad2f96fe3861af21c2a913509959094f56 Mon Sep 17 00:00:00 2001 From: Kaushik Lingarkar Date: Wed, 29 Sep 2021 12:46:02 -0700 Subject: Avoid creating loose objects in Schema 124 Use a PackInserter to create a single pack with all the updates instead of creating several thousand loose objects. Doing this is especially helpful when the repositories are based on NFS. Reducing loose objects keeps All-Users.git in a good state for subsequent schemas and also builds towards removing gc in other schemas(146 for example). On a postgres based site with repositories on NFS and with ~25k accounts, this change avoids creating ~42k loose objects and also brings down the runtime of this schema from ~550s to ~160s. Updates to schema 124 impact the upgrade of Gerrit to version 2.13 Change-Id: I67c9b789186057dc17defa4271bdf0a4237bf8f7 --- .../gerrit/server/git/meta/VersionedMetaData.java | 61 ++++++++++++++++++++-- .../google/gerrit/server/schema/Schema_124.java | 12 +++-- 2 files changed, 64 insertions(+), 9 deletions(-) diff --git a/java/com/google/gerrit/server/git/meta/VersionedMetaData.java b/java/com/google/gerrit/server/git/meta/VersionedMetaData.java index d38a33e9eb..03ae1a1522 100644 --- a/java/com/google/gerrit/server/git/meta/VersionedMetaData.java +++ b/java/com/google/gerrit/server/git/meta/VersionedMetaData.java @@ -212,6 +212,27 @@ public abstract class VersionedMetaData { } } + /** + * Update this metadata branch, recording a new commit on its reference. This method mutates its + * receiver. + * + * @param update helper information to define the update that will occur. + * @param objInserter Shared object inserter. + * @param objReader Shared object reader. + * @param revWalk Shared rev walk. + * @return the commit that was created + * @throws IOException if there is a storage problem and the update cannot be executed as + * requested or if it failed because of a concurrent update to the same reference + */ + public RevCommit commit( + MetaDataUpdate update, ObjectInserter objInserter, ObjectReader objReader, RevWalk revWalk) + throws IOException { + try (BatchMetaDataUpdate batch = openUpdate(update, objInserter, objReader, revWalk)) { + batch.write(update.getCommitBuilder()); + return batch.commit(); + } + } + /** * Creates a new commit and a new ref based on this commit. This method mutates its receiver. * @@ -259,11 +280,39 @@ public abstract class VersionedMetaData { * @throws IOException if the update failed. */ public BatchMetaDataUpdate openUpdate(MetaDataUpdate update) throws IOException { + return openUpdate(update, null, null, null); + } + + /** + * Open a batch of updates to the same metadata ref. + * + *

This allows making multiple commits to a single metadata ref, at the end of which is a + * single ref update. For batching together updates to multiple refs (each consisting of one or + * more commits against their respective refs), create the {@link MetaDataUpdate} with a {@link + * BatchRefUpdate}. + * + *

A ref update produced by this {@link BatchMetaDataUpdate} is only committed if there is no + * associated {@link BatchRefUpdate}. As a result, the configured ref updated event is not fired + * if there is an associated batch. + * + *

If object inserter, reader and revwalk are provided, then the updates are not flushed, + * allowing callers the flexibility to flush only once after several updates. + * + * @param update helper info about the update. + * @param objInserter Shared object inserter. + * @param objReader Shared object reader. + * @param revWalk Shared rev walk. + * @throws IOException if the update failed. + */ + public BatchMetaDataUpdate openUpdate( + MetaDataUpdate update, ObjectInserter objInserter, ObjectReader objReader, RevWalk revWalk) + throws IOException { final Repository db = update.getRepository(); - inserter = db.newObjectInserter(); - reader = inserter.newReader(); - final RevWalk rw = new RevWalk(reader); + inserter = objInserter == null ? db.newObjectInserter() : objInserter; + reader = objReader == null ? inserter.newReader() : objReader; + final RevWalk rw = revWalk == null ? new RevWalk(reader) : revWalk; + final RevTree tree = revision != null ? rw.parseTree(revision) : null; newTree = readTree(tree); return new BatchMetaDataUpdate() { @@ -376,7 +425,7 @@ public abstract class VersionedMetaData { newTree = null; rw.close(); - if (inserter != null) { + if (objInserter == null && inserter != null) { inserter.close(); inserter = null; } @@ -392,7 +441,9 @@ public abstract class VersionedMetaData { BatchRefUpdate bru = update.getBatch(); if (bru != null) { bru.addCommand(new ReceiveCommand(oldId.toObjectId(), newId.toObjectId(), refName)); - inserter.flush(); + if (objInserter == null) { + inserter.flush(); + } revision = rw.parseCommit(newId); return revision; } diff --git a/java/com/google/gerrit/server/schema/Schema_124.java b/java/com/google/gerrit/server/schema/Schema_124.java index 6164fd149c..603cd5c0d1 100644 --- a/java/com/google/gerrit/server/schema/Schema_124.java +++ b/java/com/google/gerrit/server/schema/Schema_124.java @@ -44,8 +44,11 @@ import java.util.Collections; import java.util.List; import java.util.Map; import org.eclipse.jgit.errors.ConfigInvalidException; +import org.eclipse.jgit.internal.storage.file.FileRepository; +import org.eclipse.jgit.internal.storage.file.PackInserter; import org.eclipse.jgit.lib.BatchRefUpdate; import org.eclipse.jgit.lib.NullProgressMonitor; +import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevWalk; @@ -95,9 +98,10 @@ public class Schema_124 extends SchemaVersion { } try (Repository git = repoManager.openRepository(allUsersName); - RevWalk rw = new RevWalk(git)) { + PackInserter packInserter = ((FileRepository) git).getObjectDatabase().newPackInserter(); + ObjectReader reader = packInserter.newReader(); + RevWalk rw = new RevWalk(reader)) { BatchRefUpdate bru = git.getRefDatabase().newBatchUpdate(); - for (Map.Entry> e : imports.asMap().entrySet()) { try (MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, allUsersName, git, bru)) { @@ -108,10 +112,10 @@ public class Schema_124 extends SchemaVersion { new VersionedAuthorizedKeys(new SimpleSshKeyCreator(), e.getKey()); authorizedKeys.load(md); authorizedKeys.setKeys(fixInvalidSequenceNumbers(e.getValue())); - authorizedKeys.commit(md); + authorizedKeys.commit(md, packInserter, reader, rw); } } - + packInserter.flush(); bru.execute(rw, NullProgressMonitor.INSTANCE); } catch (ConfigInvalidException | IOException ex) { throw new OrmException(ex); -- cgit v1.2.3 From a6478515ad624f127bf46086a196f31eba1a90ad Mon Sep 17 00:00:00 2001 From: Kaushik Lingarkar Date: Thu, 14 Oct 2021 12:41:48 -0700 Subject: Avoid creating loose objects in Schema 139 Use a PackInserter to create a single pack with all the updates instead of creating several thousand loose objects. Doing this is especially helpful when the repositories are based on NFS. Reducing loose objects keeps All-Users.git in a good state for subsequent schemas and also builds towards removing gc in other schemas(146 for example). On a postgres based site with repositories on NFS and with ~4k account_project_watches entries, this change avoids creating ~4k loose objects and also brings down the runtime of this schema from ~100s to ~70s. Updates to schema 139 impact the upgrade of Gerrit to version 2.14 Change-Id: Iccf584d8f6704f74325622ee3b00a7df2e10b8ac --- java/com/google/gerrit/server/schema/Schema_139.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/java/com/google/gerrit/server/schema/Schema_139.java b/java/com/google/gerrit/server/schema/Schema_139.java index cd018b7cbe..24710a1c80 100644 --- a/java/com/google/gerrit/server/schema/Schema_139.java +++ b/java/com/google/gerrit/server/schema/Schema_139.java @@ -48,8 +48,11 @@ import java.util.HashMap; import java.util.Map; import java.util.Set; import org.eclipse.jgit.errors.ConfigInvalidException; +import org.eclipse.jgit.internal.storage.file.FileRepository; +import org.eclipse.jgit.internal.storage.file.PackInserter; import org.eclipse.jgit.lib.BatchRefUpdate; import org.eclipse.jgit.lib.NullProgressMonitor; +import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevWalk; @@ -111,7 +114,9 @@ public class Schema_139 extends SchemaVersion { } try (Repository git = repoManager.openRepository(allUsersName); - RevWalk rw = new RevWalk(git)) { + PackInserter packInserter = ((FileRepository) git).getObjectDatabase().newPackInserter(); + ObjectReader reader = packInserter.newReader(); + RevWalk rw = new RevWalk(reader)) { BatchRefUpdate bru = git.getRefDatabase().newBatchUpdate(); bru.setRefLogIdent(serverUser); bru.setRefLogMessage(MSG, false); @@ -157,9 +162,10 @@ public class Schema_139 extends SchemaVersion { .deleteProjectWatches(accountConfig.getProjectWatches().keySet()) .updateProjectWatches(projectWatches) .build()); - accountConfig.commit(md); + accountConfig.commit(md, packInserter, reader, rw); } } + packInserter.flush(); bru.execute(rw, NullProgressMonitor.INSTANCE); } catch (IOException | ConfigInvalidException ex) { throw new OrmException(ex); -- cgit v1.2.3 From d51f78529eba1a725f2a67a7394a47c0e0813569 Mon Sep 17 00:00:00 2001 From: Kaushik Lingarkar Date: Mon, 18 Oct 2021 11:41:16 -0700 Subject: Avoid creating loose objects in Schema 144 Use a PackInserter to create a single pack with all the updates instead of creating several thousand loose objects. Doing this is especially helpful when the repositories are based on NFS. Reducing loose objects keeps All-Users.git in a good state for subsequent schemas and also builds towards removing gc in other schemas(146 for example). On a postgres based site with repositories on NFS and with ~55k account_external_ids entries, this change avoids creating ~55k loose objects and also brings down the runtime of this schema from ~400s to ~5s. Updates to schema 144 impact the upgrade of Gerrit to version 2.15 Change-Id: I7f35af9cb6518e64d8f695941116a64213c12139 --- java/com/google/gerrit/server/schema/Schema_144.java | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/java/com/google/gerrit/server/schema/Schema_144.java b/java/com/google/gerrit/server/schema/Schema_144.java index 7a00749e01..224c315241 100644 --- a/java/com/google/gerrit/server/schema/Schema_144.java +++ b/java/com/google/gerrit/server/schema/Schema_144.java @@ -34,8 +34,14 @@ import java.sql.Statement; import java.util.HashSet; import java.util.Set; import org.eclipse.jgit.errors.ConfigInvalidException; +import org.eclipse.jgit.internal.storage.file.FileRepository; +import org.eclipse.jgit.internal.storage.file.PackInserter; +import org.eclipse.jgit.lib.BatchRefUpdate; +import org.eclipse.jgit.lib.NullProgressMonitor; +import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.revwalk.RevWalk; public class Schema_144 extends SchemaVersion { private static final String COMMIT_MSG = "Import external IDs from ReviewDb"; @@ -80,16 +86,23 @@ public class Schema_144 extends SchemaVersion { } try { - try (Repository repo = repoManager.openRepository(allUsersName)) { + try (Repository repo = repoManager.openRepository(allUsersName); + PackInserter packInserter = + ((FileRepository) repo).getObjectDatabase().newPackInserter(); + ObjectReader reader = packInserter.newReader(); + RevWalk rw = new RevWalk(reader)) { + BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate(); ExternalIdNotes extIdNotes = ExternalIdNotes.loadNoCacheUpdate(allUsersName, repo); extIdNotes.upsert(toAdd); try (MetaDataUpdate metaDataUpdate = - new MetaDataUpdate(GitReferenceUpdated.DISABLED, allUsersName, repo)) { + new MetaDataUpdate(GitReferenceUpdated.DISABLED, allUsersName, repo, bru)) { metaDataUpdate.getCommitBuilder().setAuthor(serverIdent); metaDataUpdate.getCommitBuilder().setCommitter(serverIdent); metaDataUpdate.getCommitBuilder().setMessage(COMMIT_MSG); - extIdNotes.commit(metaDataUpdate); + extIdNotes.commit(metaDataUpdate, packInserter, reader, rw); } + packInserter.flush(); + bru.execute(rw, NullProgressMonitor.INSTANCE); } } catch (IOException | ConfigInvalidException e) { throw new OrmException("Failed to migrate external IDs to NoteDb", e); -- cgit v1.2.3 From b98f035d73f491b27b9e0c482709142f29361226 Mon Sep 17 00:00:00 2001 From: Adithya Chakilam Date: Mon, 4 Oct 2021 17:44:42 -0500 Subject: Parallelize Schema 130 and 131 Both these schemas iterate through all repositories and perform updates. Since this is a slow operation on NFS, parallelizing them would be efficient. Default number of migration threads is equal to number of processors or can be manually configured through cache.projects.loadThreads property. This reduced runtime for migration 130 on a gerrit site with ~16k repos on NFS from ~570secs to ~140secs on a machine with 24 processors and schema 131 runtime reduced from ~150secs to ~35secs on same setup. Also, refactor Schema 106 to use common methods created to avoid boilerplate code. Change-Id: If26c90f659b3b1d537e45034f04d9f6b7825e537 --- .../google/gerrit/server/schema/SchemaVersion.java | 63 +++++++++++ .../google/gerrit/server/schema/Schema_106.java | 117 ++++++--------------- .../google/gerrit/server/schema/Schema_130.java | 47 ++++++--- .../google/gerrit/server/schema/Schema_131.java | 54 ++++++---- 4 files changed, 163 insertions(+), 118 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java index a67a8a9066..bbc7ce16a5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java @@ -24,13 +24,23 @@ import com.google.gwtorm.jdbc.JdbcSchema; import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.StatementExecutor; import com.google.inject.Provider; +import java.io.IOException; import java.sql.PreparedStatement; import java.sql.SQLException; import java.sql.Statement; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Objects; +import java.util.Set; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; /** A version of the database schema. */ public abstract class SchemaVersion { @@ -211,4 +221,57 @@ public abstract class SchemaVersion { protected static JdbcExecutor newExecutor(ReviewDb db) throws OrmException { return new JdbcExecutor(((JdbcSchema) db).getConnection()); } + + protected int getThreads() { + return Runtime.getRuntime().availableProcessors(); + } + + protected ExecutorService createExecutor(UpdateUI ui) { + int threads = getThreads(); + ui.message(String.format("... using %d threads ...", threads)); + return Executors.newFixedThreadPool(threads); + } + + @FunctionalInterface + protected interface ThrowingFunction { + default T accept(I input) { + try { + return acceptWithThrow(input); + } catch (OrmException | IOException e) { + throw new RuntimeException(e); + } + } + + T acceptWithThrow(I input) throws OrmException, IOException; + } + + protected Collection runParallelTasks( + ExecutorService executor, Collection lst, ThrowingFunction task, UpdateUI ui) { + Collection returnSet = new HashSet<>(); + Set futures = + lst.stream() + .map(each -> executor.submit(() -> task.accept(each))) + .collect(Collectors.toSet()); + for (Future each : futures) { + try { + Object rtn = each.get(); + if (Objects.nonNull(rtn)) { + returnSet.add(rtn); + } + } catch (InterruptedException e) { + ui.message( + String.format( + "Migration step was interrupted. Only %d of %d tasks done.", + countDone(futures), lst.size())); + throw new RuntimeException(e); + } catch (ExecutionException e) { + ui.message(e.getCause().getMessage()); + } + } + return returnSet; + } + + private static long countDone(Collection futures) { + return futures.stream().filter(Future::isDone).count(); + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_106.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_106.java index 5bb3669a14..26cf815f9c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_106.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_106.java @@ -28,14 +28,7 @@ import com.google.inject.Provider; import java.io.File; import java.io.IOException; import java.io.PrintWriter; -import java.util.ArrayList; -import java.util.List; import java.util.SortedSet; -import java.util.concurrent.Callable; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.Future; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; @@ -46,6 +39,7 @@ public class Schema_106 extends SchemaVersion { private static final int THREADS_PER_CPU = 4; private final GitRepositoryManager repoManager; private final PersonIdent serverUser; + private int repoCount; @Inject Schema_106( @@ -65,95 +59,50 @@ public class Schema_106 extends SchemaVersion { ui.message("listing all repositories ..."); SortedSet repoList = repoManager.list(); + repoCount = repoList.size(); ui.message("done"); ui.message(String.format("creating reflog files for %s branches ...", RefNames.REFS_CONFIG)); - ExecutorService executorPool = createExecutor(ui, repoList.size()); - List> futures = new ArrayList<>(); - - for (Project.NameKey project : repoList) { - Callable callable = new ReflogCreator(project); - futures.add(executorPool.submit(callable)); - } - - executorPool.shutdown(); - try { - for (Future future : futures) { - try { - future.get(); - } catch (ExecutionException e) { - ui.message(e.getCause().getMessage()); - } - } - ui.message("done"); - } catch (InterruptedException ex) { - String msg = - String.format( - "Migration step 106 was interrupted. " - + "Reflog created in %d of %d repositories only.", - countDone(futures), repoList.size()); - ui.message(msg); - } - } - - private static int countDone(List> futures) { - int count = 0; - for (Future future : futures) { - if (future.isDone()) { - count++; - } - } - - return count; + runParallelTasks( + createExecutor(ui), repoList, (repo) -> createRefLog((Project.NameKey) repo), ui); } - private ExecutorService createExecutor(UpdateUI ui, int repoCount) { - int procs = Runtime.getRuntime().availableProcessors(); - int threads = Math.min(procs * THREADS_PER_CPU, repoCount); - ui.message(String.format("... using %d threads ...", threads)); - return Executors.newFixedThreadPool(threads); + @Override + protected int getThreads() { + return Math.min(Runtime.getRuntime().availableProcessors() * THREADS_PER_CPU, repoCount); } - private class ReflogCreator implements Callable { - private final Project.NameKey project; - - ReflogCreator(Project.NameKey project) { - this.project = project; - } - - @Override - public Void call() throws IOException { - try (Repository repo = repoManager.openRepository(project)) { - File metaConfigLog = new File(repo.getDirectory(), "logs/" + RefNames.REFS_CONFIG); - if (metaConfigLog.exists()) { - return null; - } + public Void createRefLog(Project.NameKey project) throws IOException { + try (Repository repo = repoManager.openRepository(project)) { + File metaConfigLog = new File(repo.getDirectory(), "logs/" + RefNames.REFS_CONFIG); + if (metaConfigLog.exists()) { + return null; + } - if (!metaConfigLog.getParentFile().mkdirs() || !metaConfigLog.createNewFile()) { - throw new IOException(); - } + if (!metaConfigLog.getParentFile().mkdirs() || !metaConfigLog.createNewFile()) { + throw new IOException(); + } - ObjectId metaConfigId = repo.resolve(RefNames.REFS_CONFIG); - if (metaConfigId != null) { - try (PrintWriter writer = new PrintWriter(metaConfigLog, UTF_8.name())) { - writer.print(ObjectId.zeroId().name()); - writer.print(" "); - writer.print(metaConfigId.name()); - writer.print(" "); - writer.print(serverUser.toExternalString()); - writer.print("\t"); - writer.print("create reflog"); - writer.println(); - } + ObjectId metaConfigId = repo.resolve(RefNames.REFS_CONFIG); + if (metaConfigId != null) { + try (PrintWriter writer = new PrintWriter(metaConfigLog, UTF_8.name())) { + writer.print(ObjectId.zeroId().name()); + writer.print(" "); + writer.print(metaConfigId.name()); + writer.print(" "); + writer.print(serverUser.toExternalString()); + writer.print("\t"); + writer.print("create reflog"); + writer.println(); } - return null; - } catch (IOException e) { - throw new IOException( - String.format( - "ERROR: Failed to create reflog file for the %s branch in repository %s", - RefNames.REFS_CONFIG, project.get())); } + return null; + } catch (IOException e) { + throw new IOException( + String.format( + "ERROR: Failed to create reflog file for the %s branch in repository %s", + RefNames.REFS_CONFIG, project.get())); } } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_130.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_130.java index afb62faf3a..85adb65a77 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_130.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_130.java @@ -19,14 +19,16 @@ import static java.util.stream.Collectors.joining; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.GerritPersonIdent; +import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.MetaDataUpdate; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Provider; +import java.util.Collection; import java.util.SortedSet; -import java.util.TreeSet; +import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; @@ -39,36 +41,49 @@ public class Schema_130 extends SchemaVersion { private final GitRepositoryManager repoManager; private final PersonIdent serverUser; + private final Config cfg; @Inject Schema_130( Provider prior, GitRepositoryManager repoManager, - @GerritPersonIdent PersonIdent serverUser) { + @GerritPersonIdent PersonIdent serverUser, + @GerritServerConfig Config cfg) { super(prior); this.repoManager = repoManager; this.serverUser = serverUser; + this.cfg = cfg; } @Override protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException { SortedSet repoList = repoManager.list(); - SortedSet repoUpgraded = new TreeSet<>(); ui.message("\tMigrating " + repoList.size() + " repositories ..."); - for (Project.NameKey projectName : repoList) { - try (Repository git = repoManager.openRepository(projectName); - MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, projectName, git)) { - ProjectConfigSchemaUpdate cfg = ProjectConfigSchemaUpdate.read(md); - cfg.removeForceFromPermission("pushTag"); - if (cfg.isUpdated()) { - repoUpgraded.add(projectName); - } - cfg.save(serverUser, COMMIT_MSG); - } catch (Exception ex) { - throw new OrmException("Cannot migrate project " + projectName, ex); - } - } + Collection repoUpgraded = + (Collection) + runParallelTasks( + createExecutor(ui), + repoList, + (repo) -> removePushTagForcePerms((Project.NameKey) repo), + ui); ui.message("\tMigration completed: " + repoUpgraded.size() + " repositories updated:"); ui.message("\t" + repoUpgraded.stream().map(n -> n.get()).collect(joining(" "))); } + + @Override + protected int getThreads() { + return cfg.getInt("cache", "projects", "loadThreads", super.getThreads()); + } + + private Project.NameKey removePushTagForcePerms(Project.NameKey project) throws OrmException { + try (Repository git = repoManager.openRepository(project); + MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, project, git)) { + ProjectConfigSchemaUpdate cfg = ProjectConfigSchemaUpdate.read(md); + cfg.removeForceFromPermission("pushTag"); + cfg.save(serverUser, COMMIT_MSG); + return cfg.isUpdated() ? project : null; + } catch (Exception ex) { + throw new OrmException("Cannot migrate project " + project, ex); + } + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_131.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_131.java index 0387e357c0..0147669ceb 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_131.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_131.java @@ -19,6 +19,7 @@ import static java.util.stream.Collectors.joining; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.GerritPersonIdent; +import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.MetaDataUpdate; @@ -27,9 +28,10 @@ import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Provider; import java.io.IOException; +import java.util.Collection; import java.util.SortedSet; -import java.util.TreeSet; import org.eclipse.jgit.errors.ConfigInvalidException; +import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; @@ -39,38 +41,54 @@ public class Schema_131 extends SchemaVersion { private final GitRepositoryManager repoManager; private final PersonIdent serverUser; + private final Config cfg; @Inject Schema_131( Provider prior, GitRepositoryManager repoManager, - @GerritPersonIdent PersonIdent serverUser) { + @GerritPersonIdent PersonIdent serverUser, + @GerritServerConfig Config cfg) { super(prior); this.repoManager = repoManager; this.serverUser = serverUser; + this.cfg = cfg; } @Override protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException { SortedSet repoList = repoManager.list(); - SortedSet repoUpgraded = new TreeSet<>(); ui.message("\tMigrating " + repoList.size() + " repositories ..."); - for (Project.NameKey projectName : repoList) { - try (Repository git = repoManager.openRepository(projectName); - MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, projectName, git)) { - ProjectConfig config = ProjectConfig.read(md); - if (config.hasLegacyPermissions()) { - md.getCommitBuilder().setAuthor(serverUser); - md.getCommitBuilder().setCommitter(serverUser); - md.setMessage(COMMIT_MSG); - config.commit(md); - repoUpgraded.add(projectName); - } - } catch (ConfigInvalidException | IOException ex) { - throw new OrmException("Cannot migrate project " + projectName, ex); - } - } + Collection repoUpgraded = + (Collection) + runParallelTasks( + createExecutor(ui), + repoList, + (repo) -> renamePushTagPermissions((Project.NameKey) repo), + ui); ui.message("\tMigration completed: " + repoUpgraded.size() + " repositories updated:"); ui.message("\t" + repoUpgraded.stream().map(n -> n.get()).collect(joining(" "))); } + + @Override + protected int getThreads() { + return cfg.getInt("cache", "projects", "loadThreads", super.getThreads()); + } + + private Project.NameKey renamePushTagPermissions(Project.NameKey project) throws OrmException { + try (Repository git = repoManager.openRepository(project); + MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, project, git)) { + ProjectConfig config = ProjectConfig.read(md); + if (config.hasLegacyPermissions()) { + md.getCommitBuilder().setAuthor(serverUser); + md.getCommitBuilder().setCommitter(serverUser); + md.setMessage(COMMIT_MSG); + config.commit(md); + return project; + } + } catch (ConfigInvalidException | IOException ex) { + throw new OrmException("Cannot migrate project " + project, ex); + } + return null; + } } -- cgit v1.2.3 From d63afd8019340d4bac3e53d81be26f89dff4eecf Mon Sep 17 00:00:00 2001 From: Kaushik Lingarkar Date: Mon, 18 Oct 2021 15:51:37 -0700 Subject: Avoid creating loose objects in Schema 154 Use a PackInserter to create a single pack with all the updates instead of creating several thousand loose objects. Doing this is especially helpful when the repositories are based on NFS. Reducing loose objects keeps All-Users.git in a good state for subsequent schemas and also builds towards removing gc in other schemas(146 for example). Inline gc has been removed because we no longer flush objects and update refs for each account. PackInserter and BatchRefUpdate are used to perform all the updates at once. On a postgres based site with repositories on NFS and with ~25k accounts, this change avoids creating ~25k loose refs and ~75k loose objects and also brings down the runtime of this schema from ~800s to ~150s. Change-Id: Icb2856dce4f5d3874088626096a4f97b18a0a0a4 --- .../google/gerrit/server/schema/Schema_154.java | 70 +++++++--------------- 1 file changed, 23 insertions(+), 47 deletions(-) diff --git a/java/com/google/gerrit/server/schema/Schema_154.java b/java/com/google/gerrit/server/schema/Schema_154.java index 8a39c0d544..5b5aebe623 100644 --- a/java/com/google/gerrit/server/schema/Schema_154.java +++ b/java/com/google/gerrit/server/schema/Schema_154.java @@ -36,22 +36,22 @@ import java.sql.Connection; import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; -import java.text.ParseException; import java.util.ArrayList; -import java.util.Date; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.concurrent.TimeUnit; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.internal.storage.file.FileRepository; -import org.eclipse.jgit.internal.storage.file.GC; +import org.eclipse.jgit.internal.storage.file.PackInserter; +import org.eclipse.jgit.lib.BatchRefUpdate; +import org.eclipse.jgit.lib.ObjectInserter; +import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.ProgressMonitor; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.TextProgressMonitor; -import org.eclipse.jgit.storage.pack.PackConfig; +import org.eclipse.jgit.revwalk.RevWalk; /** Migrate accounts to NoteDb. */ public class Schema_154 extends SchemaVersion { @@ -86,20 +86,22 @@ public class Schema_154 extends SchemaVersion { @Override protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException, SQLException { try { - try (Repository repo = repoManager.openRepository(allUsersName)) { + try (Repository repo = repoManager.openRepository(allUsersName); + PackInserter packInserter = + ((FileRepository) repo).getObjectDatabase().newPackInserter(); + ObjectReader reader = packInserter.newReader(); + RevWalk rw = new RevWalk(reader)) { + BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate(); ProgressMonitor pm = new TextProgressMonitor(); pm.beginTask("Collecting accounts", ProgressMonitor.UNKNOWN); Set accounts = scanAccounts(db, pm); pm.endTask(); pm.beginTask("Migrating accounts to NoteDb", accounts.size()); - int i = 0; for (Account account : accounts) { - updateAccountInNoteDb(repo, account); - pm.update(1); - if (++i % 100000 == 0) { - gc(repo, ui); - } + updateAccountInNoteDb(repo, account, bru, packInserter, reader, rw); } + packInserter.flush(); + bru.execute(rw, pm); pm.endTask(); } } catch (IOException | ConfigInvalidException e) { @@ -142,53 +144,27 @@ public class Schema_154 extends SchemaVersion { .collect(toMap(Map.Entry::getKey, Map.Entry::getValue)); } - private void updateAccountInNoteDb(Repository allUsersRepo, Account account) + private void updateAccountInNoteDb( + Repository allUsersRepo, + Account account, + BatchRefUpdate bru, + ObjectInserter oi, + ObjectReader reader, + RevWalk rw) throws IOException, ConfigInvalidException { MetaDataUpdate md = - new MetaDataUpdate(GitReferenceUpdated.DISABLED, allUsersName, allUsersRepo); + new MetaDataUpdate(GitReferenceUpdated.DISABLED, allUsersName, allUsersRepo, bru); PersonIdent ident = serverIdent.get(); md.getCommitBuilder().setAuthor(ident); md.getCommitBuilder().setCommitter(ident); new AccountConfig(account.getId(), allUsersName, allUsersRepo) .load() .setAccount(account) - .commit(md); + .commit(md, oi, reader, rw); } @FunctionalInterface private interface AccountSetter { void set(Account a, ResultSet rs, String field) throws SQLException; } - - private double elapsed() { - return sw.elapsed(TimeUnit.MILLISECONDS) / 1000d; - } - - private void gc(Repository repo, UpdateUI ui) { - if (repo instanceof FileRepository) { - ProgressMonitor pm = null; - try { - pm = new TextProgressMonitor(); - FileRepository r = (FileRepository) repo; - GC gc = new GC(r); - // TODO(davido): Enable bitmap index when this JGit performance issue is fixed: - // https://bugs.eclipse.org/bugs/show_bug.cgi?id=562740 - PackConfig pconfig = new PackConfig(repo); - pconfig.setBuildBitmaps(false); - gc.setPackConfig(pconfig); - gc.setProgressMonitor(pm); - pm.beginTask("gc", ProgressMonitor.UNKNOWN); - ui.message(String.format("... (%.3f s) gc --prune=now", elapsed())); - gc.setExpire(new Date()); - gc.gc(); - ui.message(String.format("... (%.3f s) full gc completed", elapsed())); - } catch (IOException | ParseException e) { - throw new RuntimeException(e); - } finally { - if (pm != null) { - pm.endTask(); - } - } - } - } } -- cgit v1.2.3 From 708d36e7ddae86525df3856fb8a3528d20ccd77b Mon Sep 17 00:00:00 2001 From: Adithya Chakilam Date: Thu, 21 Oct 2021 14:09:26 -0500 Subject: Parallelize Schema 108 This schema iterates through all open changes by repository and updates project groups, parallelizing it would be efficient. Default number of migration threads is equal to number of processors or can be manually configured through cache.projects.loadThreads property. This reduced runtime for migration 108 on a gerrit site with ~16k repos from ~1800secs to ~850secs on a machine with 24 processors. Change-Id: Ie1b847c95f9021d50bfabe6c90954b84fc4528ff --- .../google/gerrit/server/schema/Schema_108.java | 49 +++++++++++++++------- 1 file changed, 34 insertions(+), 15 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_108.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_108.java index dc88f8de64..66e0d3aeb5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_108.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_108.java @@ -27,6 +27,7 @@ import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project.NameKey; import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GroupCollector; import com.google.gerrit.server.project.NoSuchChangeException; @@ -39,6 +40,7 @@ import java.util.Map; import java.util.Set; import java.util.SortedSet; import org.eclipse.jgit.errors.MissingObjectException; +import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Ref; @@ -51,37 +53,54 @@ import org.eclipse.jgit.revwalk.RevWalk; public class Schema_108 extends SchemaVersion { private final GitRepositoryManager repoManager; + private final Config cfg; + private ReviewDb db; + private UpdateUI ui; @Inject - Schema_108(Provider prior, GitRepositoryManager repoManager) { + Schema_108( + Provider prior, + GitRepositoryManager repoManager, + @GerritServerConfig Config cfg) { super(prior); this.repoManager = repoManager; + this.cfg = cfg; } @Override protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException { + this.db = db; + this.ui = ui; ui.message("Listing all changes ..."); SetMultimap openByProject = getOpenChangesByProject(db, ui); ui.message("done"); ui.message("Updating groups for open changes ..."); - int i = 0; - for (Map.Entry> e : openByProject.asMap().entrySet()) { - try (Repository repo = repoManager.openRepository(e.getKey()); - RevWalk rw = new RevWalk(repo)) { - updateProjectGroups(db, repo, rw, (Set) e.getValue(), ui); - } catch (IOException | NoSuchChangeException err) { - throw new OrmException(err); - } - if (++i % 100 == 0) { - ui.message(" done " + i + " projects ..."); - } - } + runParallelTasks( + createExecutor(ui), + openByProject.asMap().entrySet(), + (batch) -> processProjectBatch((Map.Entry>) batch), + ui); ui.message("done"); } - private void updateProjectGroups( - ReviewDb db, Repository repo, RevWalk rw, Set changes, UpdateUI ui) + private Void processProjectBatch( + Map.Entry> changesByProject) throws OrmException { + try (Repository repo = repoManager.openRepository(changesByProject.getKey()); + RevWalk rw = new RevWalk(repo)) { + updateProjectGroups(repo, rw, (Set) changesByProject.getValue()); + } catch (IOException | NoSuchChangeException err) { + throw new OrmException(err); + } + return null; + } + + @Override + protected int getThreads() { + return cfg.getInt("cache", "projects", "loadThreads", super.getThreads()); + } + + private void updateProjectGroups(Repository repo, RevWalk rw, Set changes) throws OrmException, IOException { // Match sorting in ReceiveCommits. rw.reset(); -- cgit v1.2.3 From 75ce6910b9b3756211d10b24a3b9c5a1d281dfbf Mon Sep 17 00:00:00 2001 From: Adithya Chakilam Date: Tue, 5 Oct 2021 11:29:00 -0500 Subject: Avoid listing repositories multiple times during schema migrations Listing all repositories is a costly operation especially on NFS depending on the size of site. Avoid listing multiple times, and store it on the first retrieval. This would make schema migrations efficient. Change-Id: I95ddd4649628c666c51c36e5ba161a28d870397e --- java/com/google/gerrit/server/schema/SchemaVersion.java | 13 +++++++++++++ java/com/google/gerrit/server/schema/Schema_106.java | 2 +- java/com/google/gerrit/server/schema/Schema_108.java | 2 +- java/com/google/gerrit/server/schema/Schema_130.java | 2 +- java/com/google/gerrit/server/schema/Schema_131.java | 2 +- java/com/google/gerrit/server/schema/Schema_169.java | 2 +- 6 files changed, 18 insertions(+), 5 deletions(-) diff --git a/java/com/google/gerrit/server/schema/SchemaVersion.java b/java/com/google/gerrit/server/schema/SchemaVersion.java index 8ecf95d8ad..ef43ad3c0a 100644 --- a/java/com/google/gerrit/server/schema/SchemaVersion.java +++ b/java/com/google/gerrit/server/schema/SchemaVersion.java @@ -18,9 +18,11 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Stopwatch; import com.google.common.collect.Lists; import com.google.gerrit.reviewdb.client.CurrentSchemaVersion; +import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDbUtil; import com.google.gerrit.server.UsedAt; +import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gwtorm.jdbc.JdbcExecutor; import com.google.gwtorm.jdbc.JdbcSchema; import com.google.gwtorm.server.OrmException; @@ -33,6 +35,7 @@ import java.sql.Statement; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.SortedSet; import java.util.concurrent.TimeUnit; /** A version of the database schema. */ @@ -46,6 +49,7 @@ public abstract class SchemaVersion { private final Provider prior; private final int versionNbr; + private static SortedSet projects; protected SchemaVersion(Provider prior) { this.prior = prior; @@ -228,4 +232,13 @@ public abstract class SchemaVersion { protected static JdbcExecutor newExecutor(ReviewDb db) throws OrmException { return new JdbcExecutor(((JdbcSchema) db).getConnection()); } + + protected static SortedSet getSortedProjectsFromCache( + GitRepositoryManager repoManager) { + // TODO: Use Injection/Supplier pattern + if (projects == null) { + projects = repoManager.list(); + } + return projects; + } } diff --git a/java/com/google/gerrit/server/schema/Schema_106.java b/java/com/google/gerrit/server/schema/Schema_106.java index 7b6a30a8b2..97cd8f2034 100644 --- a/java/com/google/gerrit/server/schema/Schema_106.java +++ b/java/com/google/gerrit/server/schema/Schema_106.java @@ -64,7 +64,7 @@ public class Schema_106 extends SchemaVersion { } ui.message("listing all repositories ..."); - SortedSet repoList = repoManager.list(); + SortedSet repoList = getSortedProjectsFromCache(repoManager); ui.message("done"); ui.message(String.format("creating reflog files for %s branches ...", RefNames.REFS_CONFIG)); diff --git a/java/com/google/gerrit/server/schema/Schema_108.java b/java/com/google/gerrit/server/schema/Schema_108.java index 4e62460323..be776f189b 100644 --- a/java/com/google/gerrit/server/schema/Schema_108.java +++ b/java/com/google/gerrit/server/schema/Schema_108.java @@ -145,7 +145,7 @@ public class Schema_108 extends SchemaVersion { private SetMultimap getOpenChangesByProject(ReviewDb db, UpdateUI ui) throws OrmException { - SortedSet projects = repoManager.list(); + SortedSet projects = getSortedProjectsFromCache(repoManager); SortedSet nonExistentProjects = Sets.newTreeSet(); SetMultimap openByProject = MultimapBuilder.hashKeys().hashSetValues().build(); diff --git a/java/com/google/gerrit/server/schema/Schema_130.java b/java/com/google/gerrit/server/schema/Schema_130.java index 91cdcdae4c..c813f9dc72 100644 --- a/java/com/google/gerrit/server/schema/Schema_130.java +++ b/java/com/google/gerrit/server/schema/Schema_130.java @@ -52,7 +52,7 @@ public class Schema_130 extends SchemaVersion { @Override protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException { - SortedSet repoList = repoManager.list(); + SortedSet repoList = getSortedProjectsFromCache(repoManager); SortedSet repoUpgraded = new TreeSet<>(); ui.message("\tMigrating " + repoList.size() + " repositories ..."); for (Project.NameKey projectName : repoList) { diff --git a/java/com/google/gerrit/server/schema/Schema_131.java b/java/com/google/gerrit/server/schema/Schema_131.java index 3755211664..deb551cf5f 100644 --- a/java/com/google/gerrit/server/schema/Schema_131.java +++ b/java/com/google/gerrit/server/schema/Schema_131.java @@ -52,7 +52,7 @@ public class Schema_131 extends SchemaVersion { @Override protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException { - SortedSet repoList = repoManager.list(); + SortedSet repoList = getSortedProjectsFromCache(repoManager); SortedSet repoUpgraded = new TreeSet<>(); ui.message("\tMigrating " + repoList.size() + " repositories ..."); for (Project.NameKey projectName : repoList) { diff --git a/java/com/google/gerrit/server/schema/Schema_169.java b/java/com/google/gerrit/server/schema/Schema_169.java index d0d60f8011..12efb36fa7 100644 --- a/java/com/google/gerrit/server/schema/Schema_169.java +++ b/java/com/google/gerrit/server/schema/Schema_169.java @@ -67,7 +67,7 @@ public class Schema_169 extends SchemaVersion { boolean ok = true; ProgressMonitor pm = new TextProgressMonitor(); - SortedSet projects = repoManager.list(); + SortedSet projects = getSortedProjectsFromCache(repoManager); pm.beginTask("Migrating projects", projects.size()); int skipped = 0; for (Project.NameKey project : projects) { -- cgit v1.2.3 From 4933d537448e6e3951548aa3fa588f5c6403a319 Mon Sep 17 00:00:00 2001 From: Kaushik Lingarkar Date: Wed, 6 Oct 2021 11:20:52 -0700 Subject: Parallelize inserts into accountPatchReviewDb in schema 127 On large sites, parallelizing inserts helps reduce the migration time of this schema significantly. Default number of migration threads is equal to number of CPUs or can be manually configured through system property 'schema127_threadcount'. This change reduced the time from ~600s to ~ 300s on a postgres based Gerrit site(on a machine with 24 CPUs) with ~25M records in table account_patch_reviews and DB type as postgres for accountPatchReviewDb. Around 30% improvement also was seen when DB type is H2 for accountPatchReviewDb. Updates to schema 127 impact the upgrade of Gerrit to version 2.13. Change-Id: I2e3d1aa28c94acd4278a7aa4450d302f1ce31865 --- .../google/gerrit/server/schema/Schema_127.java | 75 ++++++++++++++++++---- 1 file changed, 61 insertions(+), 14 deletions(-) diff --git a/java/com/google/gerrit/server/schema/Schema_127.java b/java/com/google/gerrit/server/schema/Schema_127.java index d246b7593c..5fd6da0448 100644 --- a/java/com/google/gerrit/server/schema/Schema_127.java +++ b/java/com/google/gerrit/server/schema/Schema_127.java @@ -14,6 +14,7 @@ package com.google.gerrit.server.schema; +import com.google.common.collect.Lists; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.SitePaths; @@ -26,9 +27,11 @@ import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; +import java.util.List; import org.eclipse.jgit.lib.Config; public class Schema_127 extends SchemaVersion { + private static final int SLICE_SIZE = 10000; private static final int MAX_BATCH_SIZE = 1000; private final SitePaths sitePaths; @@ -54,6 +57,44 @@ public class Schema_127 extends SchemaVersion { cfg, sitePaths, threadSettingsConfig); jdbcAccountPatchReviewStore.dropTableIfExists(); jdbcAccountPatchReviewStore.createTableIfNotExists(); + List accountPatchReviews = Lists.newArrayList(); + try (Statement s = newStatement(db); + ResultSet rs = s.executeQuery("SELECT * from account_patch_reviews")) { + while (rs.next()) { + accountPatchReviews.add( + new AccountPatchReview( + rs.getInt("account_id"), + rs.getInt("change_id"), + rs.getInt("patch_set_id"), + rs.getString("file_name"))); + } + } catch (SQLException e) { + throw new RuntimeException(e); + } + runParallelTasks( + createExecutor(ui), + Lists.partition(accountPatchReviews, SLICE_SIZE), + (slice) -> processSlice(jdbcAccountPatchReviewStore, (List) slice), + ui); + } + + private static final class AccountPatchReview { + private final int accountId; + private final int changeId; + private final int patchSetId; + private final String fileName; + + public AccountPatchReview(int accountId, int changeId, int patchSetId, String fileName) { + this.accountId = accountId; + this.changeId = changeId; + this.patchSetId = patchSetId; + this.fileName = fileName; + } + } + + private Void processSlice( + JdbcAccountPatchReviewStore jdbcAccountPatchReviewStore, List slice) + throws OrmException { try (Connection con = jdbcAccountPatchReviewStore.getConnection(); PreparedStatement stmt = con.prepareStatement( @@ -61,20 +102,16 @@ public class Schema_127 extends SchemaVersion { + "(account_id, change_id, patch_set_id, file_name) VALUES " + "(?, ?, ?, ?)")) { int batchCount = 0; - - try (Statement s = newStatement(db); - ResultSet rs = s.executeQuery("SELECT * from account_patch_reviews")) { - while (rs.next()) { - stmt.setInt(1, rs.getInt("account_id")); - stmt.setInt(2, rs.getInt("change_id")); - stmt.setInt(3, rs.getInt("patch_set_id")); - stmt.setString(4, rs.getString("file_name")); - stmt.addBatch(); - batchCount++; - if (batchCount >= MAX_BATCH_SIZE) { - stmt.executeBatch(); - batchCount = 0; - } + for (AccountPatchReview accountPatchReview : slice) { + stmt.setInt(1, accountPatchReview.accountId); + stmt.setInt(2, accountPatchReview.changeId); + stmt.setInt(3, accountPatchReview.patchSetId); + stmt.setString(4, accountPatchReview.fileName); + stmt.addBatch(); + batchCount++; + if (batchCount >= MAX_BATCH_SIZE) { + stmt.executeBatch(); + batchCount = 0; } } if (batchCount > 0) { @@ -83,5 +120,15 @@ public class Schema_127 extends SchemaVersion { } catch (SQLException e) { throw jdbcAccountPatchReviewStore.convertError("insert", e); } + return null; + } + + @Override + protected int getThreads() { + try { + return Integer.parseInt(System.getProperty("schema127_threadcount")); + } catch (NumberFormatException e) { + return super.getThreads(); + } } } -- cgit v1.2.3 From a6acb716bb53c0ca1354579bb075e1e8dda093b4 Mon Sep 17 00:00:00 2001 From: Kaushik Lingarkar Date: Thu, 7 Oct 2021 14:41:51 -0700 Subject: Avoid creating loose objects in schemas 115 and 119 Use a PackInserter to create a single pack with all the updates instead of creating several thousand loose objects. Doing this is especially helpful when the repositories are based on NFS. Reducing loose objects keeps All-Users.git in a good state for subsequent schemas and also builds towards removing gc in other schemas (146 for example). On a postgres based site with repositories on NFS and with ~25k accounts and ~3k entries in account_diff_preferences, this change helps reduce creation of loose objects and also brings down the run time of schema 115 from ~40s to ~20s, schema 119 from ~230s to ~130s. Change-Id: Ic0109aa3b768328c33f6efa35cd3da29a01570f4 --- java/com/google/gerrit/server/schema/Schema_115.java | 10 ++++++++-- java/com/google/gerrit/server/schema/Schema_119.java | 10 ++++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/java/com/google/gerrit/server/schema/Schema_115.java b/java/com/google/gerrit/server/schema/Schema_115.java index d04723b2ac..8682b8acf4 100644 --- a/java/com/google/gerrit/server/schema/Schema_115.java +++ b/java/com/google/gerrit/server/schema/Schema_115.java @@ -45,8 +45,11 @@ import java.util.HashSet; import java.util.Map; import java.util.Set; import org.eclipse.jgit.errors.ConfigInvalidException; +import org.eclipse.jgit.internal.storage.file.FileRepository; +import org.eclipse.jgit.internal.storage.file.PackInserter; import org.eclipse.jgit.lib.BatchRefUpdate; import org.eclipse.jgit.lib.NullProgressMonitor; +import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevWalk; @@ -151,7 +154,9 @@ public class Schema_115 extends SchemaVersion { } try (Repository git = mgr.openRepository(allUsersName); - RevWalk rw = new RevWalk(git)) { + PackInserter packInserter = ((FileRepository) git).getObjectDatabase().newPackInserter(); + ObjectReader reader = packInserter.newReader(); + RevWalk rw = new RevWalk(reader)) { BatchRefUpdate bru = git.getRefDatabase().newBatchUpdate(); for (Map.Entry e : imports.entrySet()) { try (MetaDataUpdate md = @@ -166,10 +171,11 @@ public class Schema_115 extends SchemaVersion { null, e.getValue(), DiffPreferencesInfo.defaults()); - p.commit(md); + p.commit(md, packInserter, reader, rw); } } + packInserter.flush(); bru.execute(rw, NullProgressMonitor.INSTANCE); } catch (ConfigInvalidException | IOException ex) { throw new OrmException(ex); diff --git a/java/com/google/gerrit/server/schema/Schema_119.java b/java/com/google/gerrit/server/schema/Schema_119.java index e5a6405957..a5831687c3 100644 --- a/java/com/google/gerrit/server/schema/Schema_119.java +++ b/java/com/google/gerrit/server/schema/Schema_119.java @@ -52,8 +52,11 @@ import java.util.HashMap; import java.util.Map; import java.util.Set; import org.eclipse.jgit.errors.ConfigInvalidException; +import org.eclipse.jgit.internal.storage.file.FileRepository; +import org.eclipse.jgit.internal.storage.file.PackInserter; import org.eclipse.jgit.lib.BatchRefUpdate; import org.eclipse.jgit.lib.NullProgressMonitor; +import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevWalk; @@ -141,7 +144,9 @@ public class Schema_119 extends SchemaVersion { } try (Repository git = mgr.openRepository(allUsersName); - RevWalk rw = new RevWalk(git)) { + PackInserter packInserter = ((FileRepository) git).getObjectDatabase().newPackInserter(); + ObjectReader reader = packInserter.newReader(); + RevWalk rw = new RevWalk(reader)) { BatchRefUpdate bru = git.getRefDatabase().newBatchUpdate(); for (Map.Entry e : imports.entrySet()) { try (MetaDataUpdate md = @@ -156,10 +161,11 @@ public class Schema_119 extends SchemaVersion { null, e.getValue(), GeneralPreferencesInfo.defaults()); - p.commit(md); + p.commit(md, packInserter, reader, rw); } } + packInserter.flush(); bru.execute(rw, NullProgressMonitor.INSTANCE); } catch (ConfigInvalidException | IOException ex) { throw new OrmException(ex); -- cgit v1.2.3 From 2d7d0443d9c795b53d03a68713955ec07d2cfe32 Mon Sep 17 00:00:00 2001 From: Kaushik Lingarkar Date: Mon, 11 Oct 2021 15:07:59 -0700 Subject: Create initial commit in schema 146 only when necessary Creating the commit even when the ref is not rewritten is unnecessary work. Change-Id: Ieaa85593154999e3b556bfdc558f74cfd08d208f --- java/com/google/gerrit/server/schema/Schema_146.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/java/com/google/gerrit/server/schema/Schema_146.java b/java/com/google/gerrit/server/schema/Schema_146.java index cdb65c91fd..7f31442be0 100644 --- a/java/com/google/gerrit/server/schema/Schema_146.java +++ b/java/com/google/gerrit/server/schema/Schema_146.java @@ -231,7 +231,6 @@ public class Schema_146 extends SchemaVersion { Ref ref, Timestamp registeredOn) throws IOException { - ObjectId current = createInitialEmptyCommit(oi, emptyTree, registeredOn); rw.reset(); rw.sort(RevSort.TOPO); @@ -239,11 +238,16 @@ public class Schema_146 extends SchemaVersion { rw.markStart(rw.parseCommit(ref.getObjectId())); RevCommit c; + ObjectId current = null; while ((c = rw.next()) != null) { if (isInitialEmptyCommit(emptyTree, c)) { return; } + if (current == null) { + current = createInitialEmptyCommit(oi, emptyTree, registeredOn); + } + CommitBuilder cb = new CommitBuilder(); cb.setParentId(current); cb.setTreeId(c.getTree()); -- cgit v1.2.3 From 29f8d1ce89138f5e3ce4ef42006b790bb8099bd9 Mon Sep 17 00:00:00 2001 From: Kaushik Lingarkar Date: Wed, 27 Oct 2021 09:43:11 -0700 Subject: Update schemas 115,119 to create a desired initial commit Schema 146 attempts to rewrite user refs to insert an initial commit whose commit time is account's registration time. If schemas 115,119 can create this commit when creating new user refs, then schema 146 wouldn't have to rewrite branches. Schema 146 is also refactored in this change to use the new generic utility methods. On a postgres based site with repositories on NFS and with ~25k accounts and ~3k entries in account_diff_preferences, this change slightly increases (by <10s) the run-time of schemas 115 and 119, however it greatly reduces run-time of schema 146 from ~1100s to ~360s. Change-Id: I31725c80c9688b037f75cace5f24b601b78da955 --- .../google/gerrit/server/schema/SchemaVersion.java | 18 ++++++++++++++ .../google/gerrit/server/schema/Schema_115.java | 29 ++++++++++++++++++---- .../google/gerrit/server/schema/Schema_119.java | 26 ++++++++++++++++--- .../google/gerrit/server/schema/Schema_146.java | 15 ++--------- 4 files changed, 66 insertions(+), 22 deletions(-) diff --git a/java/com/google/gerrit/server/schema/SchemaVersion.java b/java/com/google/gerrit/server/schema/SchemaVersion.java index 344c9e262d..ed30fda302 100644 --- a/java/com/google/gerrit/server/schema/SchemaVersion.java +++ b/java/com/google/gerrit/server/schema/SchemaVersion.java @@ -47,6 +47,11 @@ import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; +import org.eclipse.jgit.lib.CommitBuilder; +import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.ObjectInserter; +import org.eclipse.jgit.lib.PersonIdent; /** A version of the database schema. */ public abstract class SchemaVersion { @@ -301,6 +306,19 @@ public abstract class SchemaVersion { return returnSet; } + protected CommitBuilder buildCommit(PersonIdent ident, ObjectId tree, String message) { + CommitBuilder cb = new CommitBuilder(); + cb.setTreeId(tree); + cb.setCommitter(ident); + cb.setAuthor(ident); + cb.setMessage(message); + return cb; + } + + protected static ObjectId emptyTree(ObjectInserter oi) throws IOException { + return oi.insert(Constants.OBJ_TREE, new byte[] {}); + } + private static long countDone(Collection futures) { return futures.stream().filter(Future::isDone).count(); } diff --git a/java/com/google/gerrit/server/schema/Schema_115.java b/java/com/google/gerrit/server/schema/Schema_115.java index 8682b8acf4..f5a230348e 100644 --- a/java/com/google/gerrit/server/schema/Schema_115.java +++ b/java/com/google/gerrit/server/schema/Schema_115.java @@ -30,6 +30,7 @@ import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.UserConfigSections; import com.google.gerrit.server.git.meta.MetaDataUpdate; +import com.google.gerrit.server.git.meta.VersionedMetaData.BatchMetaDataUpdate; import com.google.gerrit.server.patch.PatchListKey; import com.google.gwtorm.jdbc.JdbcSchema; import com.google.gwtorm.server.OrmException; @@ -40,6 +41,7 @@ import java.sql.ResultSet; import java.sql.ResultSetMetaData; import java.sql.SQLException; import java.sql.Statement; +import java.sql.Timestamp; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -49,12 +51,14 @@ import org.eclipse.jgit.internal.storage.file.FileRepository; import org.eclipse.jgit.internal.storage.file.PackInserter; import org.eclipse.jgit.lib.BatchRefUpdate; import org.eclipse.jgit.lib.NullProgressMonitor; +import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevWalk; public class Schema_115 extends SchemaVersion { + private static final String CREATE_ACCOUNT_MSG = "Create Account"; private final GitRepositoryManager mgr; private final AllUsersName allUsersName; private final PersonIdent serverUser; @@ -74,11 +78,12 @@ public class Schema_115 extends SchemaVersion { @Override protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException, SQLException { Map imports = new HashMap<>(); + HashMap registeredOnByAccount = new HashMap<>(); try (Statement stmt = ((JdbcSchema) db).getConnection().createStatement(); ResultSet rs = stmt.executeQuery( - "SELECT * FROM account_diff_preferences JOIN accounts ON " - + "account_diff_preferences.id=accounts.account_id")) { + "SELECT *, accounts.registered_on FROM account_diff_preferences " + + "JOIN accounts ON account_diff_preferences.id=accounts.account_id")) { Set availableColumns = getColumns(rs); while (rs.next()) { Account.Id accountId = new Account.Id(rs.getInt("id")); @@ -145,6 +150,9 @@ public class Schema_115 extends SchemaVersion { if (availableColumns.contains("auto_hide_diff_table_header")) { prefs.autoHideDiffTableHeader = toBoolean(rs.getString("auto_hide_diff_table_header")); } + if (availableColumns.contains("registered_on")) { + registeredOnByAccount.put(accountId, rs.getTimestamp("registered_on")); + } imports.put(accountId, prefs); } } @@ -158,20 +166,31 @@ public class Schema_115 extends SchemaVersion { ObjectReader reader = packInserter.newReader(); RevWalk rw = new RevWalk(reader)) { BatchRefUpdate bru = git.getRefDatabase().newBatchUpdate(); + ObjectId emptyTree = emptyTree(packInserter); for (Map.Entry e : imports.entrySet()) { try (MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, allUsersName, git, bru)) { + Account.Id accountId = e.getKey(); + VersionedAccountPreferences p = VersionedAccountPreferences.forUser(accountId); + p.load(md); + BatchMetaDataUpdate batch = p.openUpdate(md, packInserter, reader, rw); + if (p.getRevision() == null) { + batch.write( + buildCommit( + new PersonIdent(serverUser, registeredOnByAccount.get(accountId)), + emptyTree, + CREATE_ACCOUNT_MSG)); + } md.getCommitBuilder().setAuthor(serverUser); md.getCommitBuilder().setCommitter(serverUser); - VersionedAccountPreferences p = VersionedAccountPreferences.forUser(e.getKey()); - p.load(md); storeSection( p.getConfig(), UserConfigSections.DIFF, null, e.getValue(), DiffPreferencesInfo.defaults()); - p.commit(md, packInserter, reader, rw); + batch.write(md.getCommitBuilder()); + batch.commit(); } } diff --git a/java/com/google/gerrit/server/schema/Schema_119.java b/java/com/google/gerrit/server/schema/Schema_119.java index a5831687c3..0e419f53d9 100644 --- a/java/com/google/gerrit/server/schema/Schema_119.java +++ b/java/com/google/gerrit/server/schema/Schema_119.java @@ -39,6 +39,7 @@ import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.UserConfigSections; import com.google.gerrit.server.git.meta.MetaDataUpdate; +import com.google.gerrit.server.git.meta.VersionedMetaData.BatchMetaDataUpdate; import com.google.gwtorm.jdbc.JdbcSchema; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; @@ -48,6 +49,7 @@ import java.sql.Connection; import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; +import java.sql.Timestamp; import java.util.HashMap; import java.util.Map; import java.util.Set; @@ -56,12 +58,14 @@ import org.eclipse.jgit.internal.storage.file.FileRepository; import org.eclipse.jgit.internal.storage.file.PackInserter; import org.eclipse.jgit.lib.BatchRefUpdate; import org.eclipse.jgit.lib.NullProgressMonitor; +import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevWalk; public class Schema_119 extends SchemaVersion { + private static final String CREATE_ACCOUNT_MSG = "Create Account"; private static final ImmutableMap LEGACY_DISPLAYNAME_MAP = ImmutableMap.of( "ANON_GIT", ANON_GIT, @@ -94,6 +98,7 @@ public class Schema_119 extends SchemaVersion { String emailStrategy = "email_strategy"; Set columns = schema.getDialect().listColumns(connection, tableName); Map imports = new HashMap<>(); + HashMap registeredOnByAccount = new HashMap<>(); try (Statement stmt = ((JdbcSchema) db).getConnection().createStatement(); ResultSet rs = stmt.executeQuery( @@ -114,7 +119,8 @@ public class Schema_119 extends SchemaVersion { + "size_bar_in_change_table, " + "legacycid_in_change_table, " + "review_category_strategy, " - + "mute_common_path_prefixes " + + "mute_common_path_prefixes, " + + "registered_on " + "from " + tableName)) { while (rs.next()) { @@ -136,6 +142,7 @@ public class Schema_119 extends SchemaVersion { p.muteCommonPathPrefixes = toBoolean(rs.getString(15)); p.defaultBaseForMerges = GeneralPreferencesInfo.defaults().defaultBaseForMerges; imports.put(accountId, p); + registeredOnByAccount.put(accountId, rs.getTimestamp(16)); } } @@ -148,20 +155,31 @@ public class Schema_119 extends SchemaVersion { ObjectReader reader = packInserter.newReader(); RevWalk rw = new RevWalk(reader)) { BatchRefUpdate bru = git.getRefDatabase().newBatchUpdate(); + ObjectId emptyTree = emptyTree(packInserter); for (Map.Entry e : imports.entrySet()) { try (MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, allUsersName, git, bru)) { + Account.Id accountId = e.getKey(); + VersionedAccountPreferences p = VersionedAccountPreferences.forUser(accountId); + p.load(md); + BatchMetaDataUpdate batch = p.openUpdate(md, packInserter, reader, rw); + if (p.getRevision() == null) { + batch.write( + buildCommit( + new PersonIdent(serverUser, registeredOnByAccount.get(accountId)), + emptyTree, + CREATE_ACCOUNT_MSG)); + } md.getCommitBuilder().setAuthor(serverUser); md.getCommitBuilder().setCommitter(serverUser); - VersionedAccountPreferences p = VersionedAccountPreferences.forUser(e.getKey()); - p.load(md); storeSection( p.getConfig(), UserConfigSections.GENERAL, null, e.getValue(), GeneralPreferencesInfo.defaults()); - p.commit(md, packInserter, reader, rw); + batch.write(md.getCommitBuilder()); + batch.commit(); } } diff --git a/java/com/google/gerrit/server/schema/Schema_146.java b/java/com/google/gerrit/server/schema/Schema_146.java index 0f204dd138..9df765e3f1 100644 --- a/java/com/google/gerrit/server/schema/Schema_146.java +++ b/java/com/google/gerrit/server/schema/Schema_146.java @@ -47,7 +47,6 @@ import java.util.concurrent.locks.ReentrantLock; import org.eclipse.jgit.internal.storage.file.FileRepository; import org.eclipse.jgit.internal.storage.file.GC; import org.eclipse.jgit.lib.CommitBuilder; -import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectInserter; import org.eclipse.jgit.lib.PersonIdent; @@ -293,14 +292,8 @@ public class Schema_146 extends SchemaVersion { private ObjectId createInitialEmptyCommit( ObjectInserter oi, ObjectId emptyTree, Timestamp registrationDate) throws IOException { - PersonIdent ident = new PersonIdent(serverIdent, registrationDate); - - CommitBuilder cb = new CommitBuilder(); - cb.setTreeId(emptyTree); - cb.setCommitter(ident); - cb.setAuthor(ident); - cb.setMessage(CREATE_ACCOUNT_MSG); - return oi.insert(cb); + return oi.insert( + buildCommit(new PersonIdent(serverIdent, registrationDate), emptyTree, CREATE_ACCOUNT_MSG)); } private boolean isInitialEmptyCommit(ObjectId emptyTree, RevCommit c) { @@ -309,10 +302,6 @@ public class Schema_146 extends SchemaVersion { && c.getShortMessage().equals(CREATE_ACCOUNT_MSG); } - private static ObjectId emptyTree(ObjectInserter oi) throws IOException { - return oi.insert(Constants.OBJ_TREE, new byte[] {}); - } - private Map scanAccounts(ReviewDb db, UpdateUI ui) throws SQLException { ui.message(String.format("... (%.3f s) scan accounts", elapsed())); try (Statement stmt = newStatement(db); -- cgit v1.2.3 From a79029c99180170ef3a711c22ac339ed44586309 Mon Sep 17 00:00:00 2001 From: Kaushik Lingarkar Date: Thu, 28 Oct 2021 15:53:45 -0700 Subject: Fix schemas to use the appropriate ObjectInserter Some schemas optimized to use a PackInserter assumed that the repository is a FileRepository which may not be the case. Add a helper to get the right ObjectInserter and update the schemas accordingly. Change-Id: I17ffdfc5273dedf26195c70f49e202ec0cf0cba4 --- java/com/google/gerrit/server/schema/SchemaVersion.java | 9 +++++++++ java/com/google/gerrit/server/schema/Schema_115.java | 13 ++++++------- java/com/google/gerrit/server/schema/Schema_119.java | 13 ++++++------- java/com/google/gerrit/server/schema/Schema_124.java | 11 +++++------ java/com/google/gerrit/server/schema/Schema_139.java | 11 +++++------ java/com/google/gerrit/server/schema/Schema_144.java | 12 +++++------- java/com/google/gerrit/server/schema/Schema_154.java | 11 ++++------- 7 files changed, 40 insertions(+), 40 deletions(-) diff --git a/java/com/google/gerrit/server/schema/SchemaVersion.java b/java/com/google/gerrit/server/schema/SchemaVersion.java index ed30fda302..547ea81daf 100644 --- a/java/com/google/gerrit/server/schema/SchemaVersion.java +++ b/java/com/google/gerrit/server/schema/SchemaVersion.java @@ -47,11 +47,13 @@ import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; +import org.eclipse.jgit.internal.storage.file.FileRepository; import org.eclipse.jgit.lib.CommitBuilder; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectInserter; import org.eclipse.jgit.lib.PersonIdent; +import org.eclipse.jgit.lib.Repository; /** A version of the database schema. */ public abstract class SchemaVersion { @@ -319,6 +321,13 @@ public abstract class SchemaVersion { return oi.insert(Constants.OBJ_TREE, new byte[] {}); } + protected static ObjectInserter getPackInserterFirst(Repository repo) { + if (repo instanceof FileRepository) { + return ((FileRepository) repo).getObjectDatabase().newPackInserter(); + } + return repo.getObjectDatabase().newInserter(); + } + private static long countDone(Collection futures) { return futures.stream().filter(Future::isDone).count(); } diff --git a/java/com/google/gerrit/server/schema/Schema_115.java b/java/com/google/gerrit/server/schema/Schema_115.java index f5a230348e..3d60102226 100644 --- a/java/com/google/gerrit/server/schema/Schema_115.java +++ b/java/com/google/gerrit/server/schema/Schema_115.java @@ -47,11 +47,10 @@ import java.util.HashSet; import java.util.Map; import java.util.Set; import org.eclipse.jgit.errors.ConfigInvalidException; -import org.eclipse.jgit.internal.storage.file.FileRepository; -import org.eclipse.jgit.internal.storage.file.PackInserter; import org.eclipse.jgit.lib.BatchRefUpdate; import org.eclipse.jgit.lib.NullProgressMonitor; import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.ObjectInserter; import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; @@ -162,18 +161,18 @@ public class Schema_115 extends SchemaVersion { } try (Repository git = mgr.openRepository(allUsersName); - PackInserter packInserter = ((FileRepository) git).getObjectDatabase().newPackInserter(); - ObjectReader reader = packInserter.newReader(); + ObjectInserter inserter = getPackInserterFirst(git); + ObjectReader reader = inserter.newReader(); RevWalk rw = new RevWalk(reader)) { BatchRefUpdate bru = git.getRefDatabase().newBatchUpdate(); - ObjectId emptyTree = emptyTree(packInserter); + ObjectId emptyTree = emptyTree(inserter); for (Map.Entry e : imports.entrySet()) { try (MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, allUsersName, git, bru)) { Account.Id accountId = e.getKey(); VersionedAccountPreferences p = VersionedAccountPreferences.forUser(accountId); p.load(md); - BatchMetaDataUpdate batch = p.openUpdate(md, packInserter, reader, rw); + BatchMetaDataUpdate batch = p.openUpdate(md, inserter, reader, rw); if (p.getRevision() == null) { batch.write( buildCommit( @@ -194,7 +193,7 @@ public class Schema_115 extends SchemaVersion { } } - packInserter.flush(); + inserter.flush(); bru.execute(rw, NullProgressMonitor.INSTANCE); } catch (ConfigInvalidException | IOException ex) { throw new OrmException(ex); diff --git a/java/com/google/gerrit/server/schema/Schema_119.java b/java/com/google/gerrit/server/schema/Schema_119.java index 0e419f53d9..fd755fb3c0 100644 --- a/java/com/google/gerrit/server/schema/Schema_119.java +++ b/java/com/google/gerrit/server/schema/Schema_119.java @@ -54,11 +54,10 @@ import java.util.HashMap; import java.util.Map; import java.util.Set; import org.eclipse.jgit.errors.ConfigInvalidException; -import org.eclipse.jgit.internal.storage.file.FileRepository; -import org.eclipse.jgit.internal.storage.file.PackInserter; import org.eclipse.jgit.lib.BatchRefUpdate; import org.eclipse.jgit.lib.NullProgressMonitor; import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.ObjectInserter; import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; @@ -151,18 +150,18 @@ public class Schema_119 extends SchemaVersion { } try (Repository git = mgr.openRepository(allUsersName); - PackInserter packInserter = ((FileRepository) git).getObjectDatabase().newPackInserter(); - ObjectReader reader = packInserter.newReader(); + ObjectInserter inserter = getPackInserterFirst(git); + ObjectReader reader = inserter.newReader(); RevWalk rw = new RevWalk(reader)) { BatchRefUpdate bru = git.getRefDatabase().newBatchUpdate(); - ObjectId emptyTree = emptyTree(packInserter); + ObjectId emptyTree = emptyTree(inserter); for (Map.Entry e : imports.entrySet()) { try (MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, allUsersName, git, bru)) { Account.Id accountId = e.getKey(); VersionedAccountPreferences p = VersionedAccountPreferences.forUser(accountId); p.load(md); - BatchMetaDataUpdate batch = p.openUpdate(md, packInserter, reader, rw); + BatchMetaDataUpdate batch = p.openUpdate(md, inserter, reader, rw); if (p.getRevision() == null) { batch.write( buildCommit( @@ -183,7 +182,7 @@ public class Schema_119 extends SchemaVersion { } } - packInserter.flush(); + inserter.flush(); bru.execute(rw, NullProgressMonitor.INSTANCE); } catch (ConfigInvalidException | IOException ex) { throw new OrmException(ex); diff --git a/java/com/google/gerrit/server/schema/Schema_124.java b/java/com/google/gerrit/server/schema/Schema_124.java index 603cd5c0d1..db8e4bdf57 100644 --- a/java/com/google/gerrit/server/schema/Schema_124.java +++ b/java/com/google/gerrit/server/schema/Schema_124.java @@ -44,10 +44,9 @@ import java.util.Collections; import java.util.List; import java.util.Map; import org.eclipse.jgit.errors.ConfigInvalidException; -import org.eclipse.jgit.internal.storage.file.FileRepository; -import org.eclipse.jgit.internal.storage.file.PackInserter; import org.eclipse.jgit.lib.BatchRefUpdate; import org.eclipse.jgit.lib.NullProgressMonitor; +import org.eclipse.jgit.lib.ObjectInserter; import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; @@ -98,8 +97,8 @@ public class Schema_124 extends SchemaVersion { } try (Repository git = repoManager.openRepository(allUsersName); - PackInserter packInserter = ((FileRepository) git).getObjectDatabase().newPackInserter(); - ObjectReader reader = packInserter.newReader(); + ObjectInserter inserter = getPackInserterFirst(git); + ObjectReader reader = inserter.newReader(); RevWalk rw = new RevWalk(reader)) { BatchRefUpdate bru = git.getRefDatabase().newBatchUpdate(); for (Map.Entry> e : imports.asMap().entrySet()) { @@ -112,10 +111,10 @@ public class Schema_124 extends SchemaVersion { new VersionedAuthorizedKeys(new SimpleSshKeyCreator(), e.getKey()); authorizedKeys.load(md); authorizedKeys.setKeys(fixInvalidSequenceNumbers(e.getValue())); - authorizedKeys.commit(md, packInserter, reader, rw); + authorizedKeys.commit(md, inserter, reader, rw); } } - packInserter.flush(); + inserter.flush(); bru.execute(rw, NullProgressMonitor.INSTANCE); } catch (ConfigInvalidException | IOException ex) { throw new OrmException(ex); diff --git a/java/com/google/gerrit/server/schema/Schema_139.java b/java/com/google/gerrit/server/schema/Schema_139.java index 24710a1c80..531c516ad4 100644 --- a/java/com/google/gerrit/server/schema/Schema_139.java +++ b/java/com/google/gerrit/server/schema/Schema_139.java @@ -48,10 +48,9 @@ import java.util.HashMap; import java.util.Map; import java.util.Set; import org.eclipse.jgit.errors.ConfigInvalidException; -import org.eclipse.jgit.internal.storage.file.FileRepository; -import org.eclipse.jgit.internal.storage.file.PackInserter; import org.eclipse.jgit.lib.BatchRefUpdate; import org.eclipse.jgit.lib.NullProgressMonitor; +import org.eclipse.jgit.lib.ObjectInserter; import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; @@ -114,8 +113,8 @@ public class Schema_139 extends SchemaVersion { } try (Repository git = repoManager.openRepository(allUsersName); - PackInserter packInserter = ((FileRepository) git).getObjectDatabase().newPackInserter(); - ObjectReader reader = packInserter.newReader(); + ObjectInserter inserter = getPackInserterFirst(git); + ObjectReader reader = inserter.newReader(); RevWalk rw = new RevWalk(reader)) { BatchRefUpdate bru = git.getRefDatabase().newBatchUpdate(); bru.setRefLogIdent(serverUser); @@ -162,10 +161,10 @@ public class Schema_139 extends SchemaVersion { .deleteProjectWatches(accountConfig.getProjectWatches().keySet()) .updateProjectWatches(projectWatches) .build()); - accountConfig.commit(md, packInserter, reader, rw); + accountConfig.commit(md, inserter, reader, rw); } } - packInserter.flush(); + inserter.flush(); bru.execute(rw, NullProgressMonitor.INSTANCE); } catch (IOException | ConfigInvalidException ex) { throw new OrmException(ex); diff --git a/java/com/google/gerrit/server/schema/Schema_144.java b/java/com/google/gerrit/server/schema/Schema_144.java index 224c315241..88f24c2a7c 100644 --- a/java/com/google/gerrit/server/schema/Schema_144.java +++ b/java/com/google/gerrit/server/schema/Schema_144.java @@ -34,10 +34,9 @@ import java.sql.Statement; import java.util.HashSet; import java.util.Set; import org.eclipse.jgit.errors.ConfigInvalidException; -import org.eclipse.jgit.internal.storage.file.FileRepository; -import org.eclipse.jgit.internal.storage.file.PackInserter; import org.eclipse.jgit.lib.BatchRefUpdate; import org.eclipse.jgit.lib.NullProgressMonitor; +import org.eclipse.jgit.lib.ObjectInserter; import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; @@ -87,9 +86,8 @@ public class Schema_144 extends SchemaVersion { try { try (Repository repo = repoManager.openRepository(allUsersName); - PackInserter packInserter = - ((FileRepository) repo).getObjectDatabase().newPackInserter(); - ObjectReader reader = packInserter.newReader(); + ObjectInserter inserter = getPackInserterFirst(repo); + ObjectReader reader = inserter.newReader(); RevWalk rw = new RevWalk(reader)) { BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate(); ExternalIdNotes extIdNotes = ExternalIdNotes.loadNoCacheUpdate(allUsersName, repo); @@ -99,9 +97,9 @@ public class Schema_144 extends SchemaVersion { metaDataUpdate.getCommitBuilder().setAuthor(serverIdent); metaDataUpdate.getCommitBuilder().setCommitter(serverIdent); metaDataUpdate.getCommitBuilder().setMessage(COMMIT_MSG); - extIdNotes.commit(metaDataUpdate, packInserter, reader, rw); + extIdNotes.commit(metaDataUpdate, inserter, reader, rw); } - packInserter.flush(); + inserter.flush(); bru.execute(rw, NullProgressMonitor.INSTANCE); } } catch (IOException | ConfigInvalidException e) { diff --git a/java/com/google/gerrit/server/schema/Schema_154.java b/java/com/google/gerrit/server/schema/Schema_154.java index 5b5aebe623..a47b22a2e0 100644 --- a/java/com/google/gerrit/server/schema/Schema_154.java +++ b/java/com/google/gerrit/server/schema/Schema_154.java @@ -42,8 +42,6 @@ import java.util.List; import java.util.Map; import java.util.Set; import org.eclipse.jgit.errors.ConfigInvalidException; -import org.eclipse.jgit.internal.storage.file.FileRepository; -import org.eclipse.jgit.internal.storage.file.PackInserter; import org.eclipse.jgit.lib.BatchRefUpdate; import org.eclipse.jgit.lib.ObjectInserter; import org.eclipse.jgit.lib.ObjectReader; @@ -87,9 +85,8 @@ public class Schema_154 extends SchemaVersion { protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException, SQLException { try { try (Repository repo = repoManager.openRepository(allUsersName); - PackInserter packInserter = - ((FileRepository) repo).getObjectDatabase().newPackInserter(); - ObjectReader reader = packInserter.newReader(); + ObjectInserter inserter = getPackInserterFirst(repo); + ObjectReader reader = inserter.newReader(); RevWalk rw = new RevWalk(reader)) { BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate(); ProgressMonitor pm = new TextProgressMonitor(); @@ -98,9 +95,9 @@ public class Schema_154 extends SchemaVersion { pm.endTask(); pm.beginTask("Migrating accounts to NoteDb", accounts.size()); for (Account account : accounts) { - updateAccountInNoteDb(repo, account, bru, packInserter, reader, rw); + updateAccountInNoteDb(repo, account, bru, inserter, reader, rw); } - packInserter.flush(); + inserter.flush(); bru.execute(rw, pm); pm.endTask(); } -- cgit v1.2.3 From 5e75a58dd1e4f835215c134a49a36c1ab1f5eb67 Mon Sep 17 00:00:00 2001 From: Kaushik Lingarkar Date: Fri, 22 Oct 2021 16:57:09 -0700 Subject: Avoid creating loose objects in schema 167 Use a PackInserter to create a single pack with all the updates instead of creating several thousand loose objects. Doing this is especially helpful when the repositories are based on NFS. Reducing loose objects keeps All-Users.git in a good state for subsequent schemas. On a postgres based site with repositories on NFS and with ~25k accounts and ~650 groups, this change helps reduce creation of loose objects and also brings down the run time of this schema from ~200s to ~15s. Change-Id: I8201f9cf5820c2f51bf7267282464449681a26db --- .../gerrit/server/schema/GroupRebuilder.java | 16 +++++++++- .../google/gerrit/server/schema/Schema_167.java | 34 +++++++++++++++------- 2 files changed, 39 insertions(+), 11 deletions(-) diff --git a/java/com/google/gerrit/server/schema/GroupRebuilder.java b/java/com/google/gerrit/server/schema/GroupRebuilder.java index 0b4fd0299a..609940409f 100644 --- a/java/com/google/gerrit/server/schema/GroupRebuilder.java +++ b/java/com/google/gerrit/server/schema/GroupRebuilder.java @@ -56,8 +56,11 @@ import java.util.stream.Stream; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.lib.BatchRefUpdate; import org.eclipse.jgit.lib.CommitBuilder; +import org.eclipse.jgit.lib.ObjectInserter; +import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.revwalk.RevWalk; /** Helper for rebuilding an entire group's NoteDb refs. */ class GroupRebuilder { @@ -74,6 +77,17 @@ class GroupRebuilder { public void rebuild(Repository allUsersRepo, GroupBundle bundle, @Nullable BatchRefUpdate bru) throws IOException, ConfigInvalidException, OrmDuplicateKeyException { + rebuild(allUsersRepo, bundle, bru, null, null, null); + } + + public void rebuild( + Repository allUsersRepo, + GroupBundle bundle, + @Nullable BatchRefUpdate bru, + @Nullable ObjectInserter inserter, + @Nullable ObjectReader reader, + @Nullable RevWalk rw) + throws IOException, ConfigInvalidException, OrmDuplicateKeyException { AccountGroup group = bundle.group(); InternalGroupCreation groupCreation = InternalGroupCreation.builder() @@ -105,7 +119,7 @@ class GroupRebuilder { md.getCommitBuilder().setCommitter(created); // Rebuild group ref. - try (BatchMetaDataUpdate batch = groupConfig.openUpdate(md)) { + try (BatchMetaDataUpdate batch = groupConfig.openUpdate(md, inserter, reader, rw)) { batch.write(groupConfig, md.getCommitBuilder()); for (Map.Entry> e : events.entrySet()) { diff --git a/java/com/google/gerrit/server/schema/Schema_167.java b/java/com/google/gerrit/server/schema/Schema_167.java index a5066cc988..67c2ce94fd 100644 --- a/java/com/google/gerrit/server/schema/Schema_167.java +++ b/java/com/google/gerrit/server/schema/Schema_167.java @@ -56,8 +56,10 @@ import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.lib.BatchRefUpdate; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.ObjectInserter; +import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.revwalk.RevWalk; /** Migrate groups from ReviewDb to NoteDb. */ public class Schema_167 extends SchemaVersion { @@ -95,18 +97,29 @@ public class Schema_167 extends SchemaVersion { return; } - try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) { + try (Repository allUsersRepo = repoManager.openRepository(allUsersName); + ObjectInserter inserter = getPackInserterFirst(allUsersRepo); + ObjectReader reader = inserter.newReader(); + RevWalk rw = new RevWalk(reader)) { List allGroupReferences = readGroupReferencesFromReviewDb(db); BatchRefUpdate batchRefUpdate = allUsersRepo.getRefDatabase().newBatchUpdate(); - writeAllGroupNamesToNoteDb(allUsersRepo, allGroupReferences, batchRefUpdate); + writeAllGroupNamesToNoteDb(allUsersRepo, allGroupReferences, inserter, batchRefUpdate); GroupRebuilder groupRebuilder = createGroupRebuilder(db, allUsersRepo); for (GroupReference groupReference : allGroupReferences) { migrateOneGroupToNoteDb( - db, allUsersRepo, groupRebuilder, groupReference.getUUID(), batchRefUpdate); + db, + allUsersRepo, + groupRebuilder, + groupReference.getUUID(), + batchRefUpdate, + inserter, + reader, + rw); } + inserter.flush(); RefUpdateUtil.executeChecked(batchRefUpdate, allUsersRepo); } catch (IOException | ConfigInvalidException e) { throw new OrmException( @@ -130,13 +143,11 @@ public class Schema_167 extends SchemaVersion { private void writeAllGroupNamesToNoteDb( Repository allUsersRepo, List allGroupReferences, + ObjectInserter inserter, BatchRefUpdate batchRefUpdate) throws IOException { - try (ObjectInserter inserter = allUsersRepo.newObjectInserter()) { - GroupNameNotes.updateAllGroups( - allUsersRepo, inserter, batchRefUpdate, allGroupReferences, serverIdent); - inserter.flush(); - } + GroupNameNotes.updateAllGroups( + allUsersRepo, inserter, batchRefUpdate, allGroupReferences, serverIdent); } private GroupRebuilder createGroupRebuilder(ReviewDb db, Repository allUsersRepo) @@ -169,11 +180,14 @@ public class Schema_167 extends SchemaVersion { Repository allUsersRepo, GroupRebuilder rebuilder, AccountGroup.UUID uuid, - BatchRefUpdate batchRefUpdate) + BatchRefUpdate batchRefUpdate, + ObjectInserter inserter, + ObjectReader reader, + RevWalk rw) throws ConfigInvalidException, IOException, OrmException { GroupBundle reviewDbBundle = GroupBundle.Factory.fromReviewDb(db, uuid); RefUpdateUtil.deleteChecked(allUsersRepo, RefNames.refsGroups(uuid)); - rebuilder.rebuild(allUsersRepo, reviewDbBundle, batchRefUpdate); + rebuilder.rebuild(allUsersRepo, reviewDbBundle, batchRefUpdate, inserter, reader, rw); } // The regular account cache isn't available during init. -> Use a simple replacement which tries -- cgit v1.2.3 From 3df428d0db18be310c02cbfd7aacab5e9da99297 Mon Sep 17 00:00:00 2001 From: Kaushik Lingarkar Date: Wed, 27 Oct 2021 22:12:12 -0700 Subject: Avoid creating loose objects in Schema 146 Attempt to use a PackInserter per batch to create pack files rather than creating several thousand loose objects. Doing this is especially helpful when the repositories are based on NFS. We now no longer need the inline pack-refs since all refs in a batch are updated without creating loose refs. Also, instead of performing an inline gc every 100k accounts, do it for every 50 packs created. Reducing loose objects and loose refs keeps All-Users.git in a good state for subsequent schemas. It also allows us to skip the gc being done at the end of this schema. Change-Id: I60f2be1fa7e0bfc12611a81cea9154aa5961d0aa --- .../google/gerrit/server/schema/Schema_146.java | 94 ++++++++++------------ 1 file changed, 41 insertions(+), 53 deletions(-) diff --git a/java/com/google/gerrit/server/schema/Schema_146.java b/java/com/google/gerrit/server/schema/Schema_146.java index 9df765e3f1..32dfc83bc3 100644 --- a/java/com/google/gerrit/server/schema/Schema_146.java +++ b/java/com/google/gerrit/server/schema/Schema_146.java @@ -46,20 +46,23 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.locks.ReentrantLock; import org.eclipse.jgit.internal.storage.file.FileRepository; import org.eclipse.jgit.internal.storage.file.GC; +import org.eclipse.jgit.internal.storage.file.PackInserter; +import org.eclipse.jgit.lib.BatchRefUpdate; import org.eclipse.jgit.lib.CommitBuilder; +import org.eclipse.jgit.lib.NullProgressMonitor; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectInserter; +import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.ProgressMonitor; import org.eclipse.jgit.lib.Ref; -import org.eclipse.jgit.lib.RefUpdate; -import org.eclipse.jgit.lib.RefUpdate.Result; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.TextProgressMonitor; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevSort; import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.storage.pack.PackConfig; +import org.eclipse.jgit.transport.ReceiveCommand; /** * Make sure that for every account a user branch exists that has an initial empty commit with the @@ -78,7 +81,8 @@ public class Schema_146 extends SchemaVersion { private final GitRepositoryManager repoManager; private final AllUsersName allUsersName; private final PersonIdent serverIdent; - private AtomicInteger i = new AtomicInteger(); + private AtomicInteger migratedAccounts = new AtomicInteger(); + private AtomicInteger packs = new AtomicInteger(); private Stopwatch sw = Stopwatch.createStarted(); ReentrantLock gcLock = new ReentrantLock(); private int size; @@ -118,10 +122,9 @@ public class Schema_146 extends SchemaVersion { throw new RuntimeException(e); } ui.message( - String.format("... (%.3f s) Migrated all %d accounts to schema 146", elapsed(), i.get())); - ui.message("Run full gc"); - gc(ui); - ui.message(String.format("... (%.3f s) full gc completed", elapsed())); + String.format( + "... (%.3f s) Migrated all %d accounts to schema 146", + elapsed(), migratedAccounts.get())); } @Override @@ -135,31 +138,36 @@ public class Schema_146 extends SchemaVersion { private void processBatch(List> batch, UpdateUI ui) { try (Repository repo = repoManager.openRepository(allUsersName); - RevWalk rw = new RevWalk(repo); - ObjectInserter oi = repo.newObjectInserter()) { - ObjectId emptyTree = emptyTree(oi); + ObjectInserter inserter = getPackInserterFirst(repo); + ObjectReader reader = inserter.newReader(); + RevWalk rw = new RevWalk(reader)) { + BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate(); + bru.setAllowNonFastForwards(true); + bru.setRefLogIdent(serverIdent); + bru.setRefLogMessage(getClass().getSimpleName(), true); + ObjectId emptyTree = emptyTree(inserter); for (Map.Entry e : batch) { - String refName = RefNames.refsUsers(e.getKey()); + Account.Id accountId = e.getKey(); + Timestamp registeredOn = e.getValue(); + String refName = RefNames.refsUsers(accountId); Ref ref = repo.exactRef(refName); if (ref != null) { - rewriteUserBranch(repo, rw, oi, emptyTree, ref, e.getValue()); + rewriteUserBranch(rw, inserter, emptyTree, ref, registeredOn, bru); } else { - createUserBranch(repo, oi, emptyTree, e.getKey(), e.getValue()); + createUserBranch(inserter, emptyTree, refName, registeredOn, bru); } - int count = i.incrementAndGet(); - showProgress(ui, count); - if (count % 1000 == 0) { - boolean runFullGc = count % 100000 == 0; - if (runFullGc) { - ui.message("Run full gc"); - } - gc(repo, !runFullGc, ui); - if (runFullGc) { - ui.message(String.format("... (%.3f s) full gc completed", elapsed())); - } + } + if (!bru.getCommands().isEmpty()) { + inserter.flush(); + bru.execute(rw, NullProgressMonitor.INSTANCE); + if (inserter instanceof PackInserter && packs.incrementAndGet() % 50 == 0) { + ui.message("Run full gc"); + gc(ui); + ui.message(String.format("... (%.3f s) full gc completed", elapsed())); } } + showProgress(ui, migratedAccounts.addAndGet(batch.size())); } catch (IOException e) { throw new UncheckedIOException(e); } @@ -220,12 +228,12 @@ public class Schema_146 extends SchemaVersion { } private void rewriteUserBranch( - Repository repo, RevWalk rw, ObjectInserter oi, ObjectId emptyTree, Ref ref, - Timestamp registeredOn) + Timestamp registeredOn, + BatchRefUpdate bru) throws IOException { rw.reset(); @@ -254,40 +262,20 @@ public class Schema_146 extends SchemaVersion { current = oi.insert(cb); } - oi.flush(); - - RefUpdate ru = repo.updateRef(ref.getName()); - ru.setExpectedOldObjectId(ref.getObjectId()); - ru.setNewObjectId(current); - ru.setForceUpdate(true); - ru.setRefLogIdent(serverIdent); - ru.setRefLogMessage(getClass().getSimpleName(), true); - Result result = ru.update(); - if (result != Result.FORCED) { - throw new IOException( - String.format("Failed to update ref %s: %s", ref.getName(), result.name())); - } + bru.addCommand( + new ReceiveCommand( + ref.getObjectId(), current, ref.getName(), ReceiveCommand.Type.UPDATE_NONFASTFORWARD)); } public void createUserBranch( - Repository repo, ObjectInserter oi, ObjectId emptyTree, - Account.Id accountId, - Timestamp registeredOn) + String refName, + Timestamp registeredOn, + BatchRefUpdate bru) throws IOException { ObjectId id = createInitialEmptyCommit(oi, emptyTree, registeredOn); - - String refName = RefNames.refsUsers(accountId); - RefUpdate ru = repo.updateRef(refName); - ru.setExpectedOldObjectId(ObjectId.zeroId()); - ru.setNewObjectId(id); - ru.setRefLogIdent(serverIdent); - ru.setRefLogMessage(CREATE_ACCOUNT_MSG, false); - Result result = ru.update(); - if (result != Result.NEW) { - throw new IOException(String.format("Failed to update ref %s: %s", refName, result.name())); - } + bru.addCommand(new ReceiveCommand(ObjectId.zeroId(), id, refName)); } private ObjectId createInitialEmptyCommit( -- cgit v1.2.3 From 2fd651a1a67678274bc1b8d2352d28ce8f2210d8 Mon Sep 17 00:00:00 2001 From: Adithya Chakilam Date: Wed, 20 Oct 2021 14:04:49 -0500 Subject: Disable auto flushing during offline Lucene indexing Current indexing process waits for the indexed changes to be flushed, which writes the in-memory segments to disk. Flushing makes those updates available to the searcher threads. This is helpful during online reindexing because we want searches to have up-to-date data. However, in case of offline indexing this creates frequent unnecessary flushes. It is ideal to flush all the in-memory segments at the end during offline indexing process since there are no searches happening on index and also lets us skip doing many segment merges. This change disables auto flushing during init/reindex commands and the online reindexing behaviour is unchanged. On a test site with ~160K changes, this change brought down the runtime of reindex command from ~5mins to ~3mins when disk-caches are pre-populated from previous reindex run and with no disk-caches there was a reduction from ~49mins to ~45mins. Number of flushes decreased from ~120K to 81 in both the cases. Number of segment merges decreased from ~13k and ~2k to 6 during pre-populated and empty disk-caches respectively. Flushes now only happen based on the index..ramBufferSize and index..maxBufferedDocs settings. Change-Id: I6094a98e8cdb2c3c3b1be328db19b0920c0bc895 --- Documentation/config-gerrit.txt | 5 ++++ .../com/google/gerrit/acceptance/GerritServer.java | 4 ++- .../gerrit/httpd/init/WebAppInitializer.java | 3 ++- .../google/gerrit/lucene/AbstractLuceneIndex.java | 13 ++++++++-- java/com/google/gerrit/lucene/ChangeSubIndex.java | 12 ++++++--- .../google/gerrit/lucene/LuceneAccountIndex.java | 7 ++++-- .../google/gerrit/lucene/LuceneChangeIndex.java | 29 ++++++++++++++++++---- .../com/google/gerrit/lucene/LuceneGroupIndex.java | 7 ++++-- .../google/gerrit/lucene/LuceneIndexModule.java | 26 +++++++++++++------ .../google/gerrit/lucene/LuceneProjectIndex.java | 7 ++++-- java/com/google/gerrit/pgm/Daemon.java | 3 ++- java/com/google/gerrit/pgm/Reindex.java | 5 +++- .../init/index/lucene/LuceneIndexModuleOnInit.java | 3 +++ java/com/google/gerrit/server/index/AutoFlush.java | 20 +++++++++++++++ java/com/google/gerrit/testing/InMemoryModule.java | 10 ++++++-- 15 files changed, 124 insertions(+), 30 deletions(-) create mode 100644 java/com/google/gerrit/server/index/AutoFlush.java diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index aff17cac82..f0db02ee88 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -3130,6 +3130,11 @@ Lucene documentation] for further details. + Defaults to true (throttling enabled). +During offline reindexing, setting ramBufferSize greater than the size +of index (size of specific index folder under /index) and +maxBufferedDocs as -1 avoids unnecessary flushes and triggers only a +single flush at the end of the process. + Sample Lucene index configuration: ---- [index] diff --git a/java/com/google/gerrit/acceptance/GerritServer.java b/java/com/google/gerrit/acceptance/GerritServer.java index a397923896..b18df5b3c4 100644 --- a/java/com/google/gerrit/acceptance/GerritServer.java +++ b/java/com/google/gerrit/acceptance/GerritServer.java @@ -35,6 +35,7 @@ import com.google.gerrit.server.config.GerritRuntime; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.SitePath; import com.google.gerrit.server.git.receive.AsyncReceiveCommits; +import com.google.gerrit.server.index.AutoFlush; import com.google.gerrit.server.ssh.NoSshModule; import com.google.gerrit.server.util.ManualRequestContext; import com.google.gerrit.server.util.OneOffRequestContext; @@ -353,7 +354,8 @@ public class GerritServer implements AutoCloseable { cfg.setBoolean("index", null, "onlineUpgrade", false); cfg.setString("gitweb", null, "cgi", ""); daemon.setEnableHttpd(desc.httpd()); - daemon.setLuceneModule(LuceneIndexModule.singleVersionAllLatest(0, isSlave(baseConfig))); + daemon.setLuceneModule( + LuceneIndexModule.singleVersionAllLatest(0, isSlave(baseConfig), AutoFlush.ENABLED)); daemon.setDatabaseForTesting( ImmutableList.of( new InMemoryTestingDatabaseModule( diff --git a/java/com/google/gerrit/httpd/init/WebAppInitializer.java b/java/com/google/gerrit/httpd/init/WebAppInitializer.java index c2d4a146ef..9f2ecb47b4 100644 --- a/java/com/google/gerrit/httpd/init/WebAppInitializer.java +++ b/java/com/google/gerrit/httpd/init/WebAppInitializer.java @@ -74,6 +74,7 @@ import com.google.gerrit.server.git.GarbageCollectionModule; import com.google.gerrit.server.git.GitRepositoryManagerModule; import com.google.gerrit.server.git.SearchingChangeCacheImpl; import com.google.gerrit.server.git.WorkQueue; +import com.google.gerrit.server.index.AutoFlush; import com.google.gerrit.server.index.IndexModule; import com.google.gerrit.server.index.IndexModule.IndexType; import com.google.gerrit.server.index.OnlineUpgrader; @@ -387,7 +388,7 @@ public class WebAppInitializer extends GuiceServletContextListener implements Fi private Module createIndexModule() { switch (indexType) { case LUCENE: - return LuceneIndexModule.latestVersion(false); + return LuceneIndexModule.latestVersion(false, AutoFlush.ENABLED); case ELASTICSEARCH: return ElasticIndexModule.latestVersion(false); default: diff --git a/java/com/google/gerrit/lucene/AbstractLuceneIndex.java b/java/com/google/gerrit/lucene/AbstractLuceneIndex.java index cb26488e3b..ef42fe0a5e 100644 --- a/java/com/google/gerrit/lucene/AbstractLuceneIndex.java +++ b/java/com/google/gerrit/lucene/AbstractLuceneIndex.java @@ -39,6 +39,7 @@ import com.google.gerrit.index.Schema.Values; import com.google.gerrit.index.query.DataSource; import com.google.gerrit.index.query.FieldBundle; import com.google.gerrit.server.config.SitePaths; +import com.google.gerrit.server.index.AutoFlush; import com.google.gerrit.server.index.IndexUtils; import com.google.gerrit.server.logging.LoggingContextAwareExecutorService; import com.google.gerrit.server.logging.LoggingContextAwareScheduledExecutorService; @@ -103,6 +104,7 @@ public abstract class AbstractLuceneIndex implements Index { private final ReferenceManager searcherManager; private final ControlledRealTimeReopenThread reopenThread; private final Set notDoneNrtFutures; + private final AutoFlush autoFlush; private ScheduledExecutorService autoCommitExecutor; AbstractLuceneIndex( @@ -112,12 +114,14 @@ public abstract class AbstractLuceneIndex implements Index { String name, String subIndex, GerritIndexWriterConfig writerConfig, - SearcherFactory searcherFactory) + SearcherFactory searcherFactory, + AutoFlush autoFlush) throws IOException { this.schema = schema; this.sitePaths = sitePaths; this.dir = dir; this.name = name; + this.autoFlush = autoFlush; String index = Joiner.on('_').skipNulls().join(name, subIndex); long commitPeriod = writerConfig.getCommitWithinMs(); @@ -211,7 +215,9 @@ public abstract class AbstractLuceneIndex implements Index { } }); - reopenThread.start(); + if (autoFlush.equals(AutoFlush.ENABLED)) { + reopenThread.start(); + } } @Override @@ -457,6 +463,9 @@ public abstract class AbstractLuceneIndex implements Index { } private boolean isGenAvailableNowForCurrentSearcher() { + if (autoFlush.equals(AutoFlush.DISABLED)) { + return true; + } try { return reopenThread.waitForGeneration(gen, 0); } catch (InterruptedException e) { diff --git a/java/com/google/gerrit/lucene/ChangeSubIndex.java b/java/com/google/gerrit/lucene/ChangeSubIndex.java index 7d7cbef64f..5c78c3446e 100644 --- a/java/com/google/gerrit/lucene/ChangeSubIndex.java +++ b/java/com/google/gerrit/lucene/ChangeSubIndex.java @@ -29,6 +29,7 @@ import com.google.gerrit.index.query.Predicate; import com.google.gerrit.index.query.QueryParseException; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.server.config.SitePaths; +import com.google.gerrit.server.index.AutoFlush; import com.google.gerrit.server.index.change.ChangeField; import com.google.gerrit.server.index.change.ChangeIndex; import com.google.gerrit.server.query.change.ChangeData; @@ -48,7 +49,8 @@ public class ChangeSubIndex extends AbstractLuceneIndex SitePaths sitePaths, Path path, GerritIndexWriterConfig writerConfig, - SearcherFactory searcherFactory) + SearcherFactory searcherFactory, + AutoFlush autoFlush) throws IOException { this( schema, @@ -56,7 +58,8 @@ public class ChangeSubIndex extends AbstractLuceneIndex FSDirectory.open(path), path.getFileName().toString(), writerConfig, - searcherFactory); + searcherFactory, + autoFlush); } ChangeSubIndex( @@ -65,9 +68,10 @@ public class ChangeSubIndex extends AbstractLuceneIndex Directory dir, String subIndex, GerritIndexWriterConfig writerConfig, - SearcherFactory searcherFactory) + SearcherFactory searcherFactory, + AutoFlush autoFlush) throws IOException { - super(schema, sitePaths, dir, NAME, subIndex, writerConfig, searcherFactory); + super(schema, sitePaths, dir, NAME, subIndex, writerConfig, searcherFactory, autoFlush); } @Override diff --git a/java/com/google/gerrit/lucene/LuceneAccountIndex.java b/java/com/google/gerrit/lucene/LuceneAccountIndex.java index 86a21115c5..84dd16d0a9 100644 --- a/java/com/google/gerrit/lucene/LuceneAccountIndex.java +++ b/java/com/google/gerrit/lucene/LuceneAccountIndex.java @@ -31,6 +31,7 @@ import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountState; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.SitePaths; +import com.google.gerrit.server.index.AutoFlush; import com.google.gerrit.server.index.IndexUtils; import com.google.gerrit.server.index.account.AccountIndex; import com.google.inject.Inject; @@ -86,7 +87,8 @@ public class LuceneAccountIndex extends AbstractLuceneIndex accountCache, - @Assisted Schema schema) + @Assisted Schema schema, + AutoFlush autoFlush) throws IOException { super( schema, @@ -95,7 +97,8 @@ public class LuceneAccountIndex extends AbstractLuceneIndex db, ChangeData.Factory changeDataFactory, - @Assisted Schema schema) + @Assisted Schema schema, + AutoFlush autoFlush) throws IOException { this.executor = executor; this.db = db; @@ -171,18 +173,35 @@ public class LuceneChangeIndex implements ChangeIndex { if (LuceneIndexModule.isInMemoryTest(cfg)) { openIndex = new ChangeSubIndex( - schema, sitePaths, new RAMDirectory(), "ramOpen", openConfig, searcherFactory); + schema, + sitePaths, + new RAMDirectory(), + "ramOpen", + openConfig, + searcherFactory, + autoFlush); closedIndex = new ChangeSubIndex( - schema, sitePaths, new RAMDirectory(), "ramClosed", closedConfig, searcherFactory); + schema, + sitePaths, + new RAMDirectory(), + "ramClosed", + closedConfig, + searcherFactory, + autoFlush); } else { Path dir = LuceneVersionManager.getDir(sitePaths, CHANGES, schema); openIndex = new ChangeSubIndex( - schema, sitePaths, dir.resolve(CHANGES_OPEN), openConfig, searcherFactory); + schema, sitePaths, dir.resolve(CHANGES_OPEN), openConfig, searcherFactory, autoFlush); closedIndex = new ChangeSubIndex( - schema, sitePaths, dir.resolve(CHANGES_CLOSED), closedConfig, searcherFactory); + schema, + sitePaths, + dir.resolve(CHANGES_CLOSED), + closedConfig, + searcherFactory, + autoFlush); } } diff --git a/java/com/google/gerrit/lucene/LuceneGroupIndex.java b/java/com/google/gerrit/lucene/LuceneGroupIndex.java index 95e2ab903e..40b455b53a 100644 --- a/java/com/google/gerrit/lucene/LuceneGroupIndex.java +++ b/java/com/google/gerrit/lucene/LuceneGroupIndex.java @@ -29,6 +29,7 @@ import com.google.gerrit.server.account.GroupCache; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.SitePaths; import com.google.gerrit.server.group.InternalGroup; +import com.google.gerrit.server.index.AutoFlush; import com.google.gerrit.server.index.IndexUtils; import com.google.gerrit.server.index.group.GroupIndex; import com.google.inject.Inject; @@ -82,7 +83,8 @@ public class LuceneGroupIndex extends AbstractLuceneIndex groupCache, - @Assisted Schema schema) + @Assisted Schema schema, + AutoFlush autoFlush) throws IOException { super( schema, @@ -91,7 +93,8 @@ public class LuceneGroupIndex extends AbstractLuceneIndex versions, int threads, boolean slave) { - return new LuceneIndexModule(versions, threads, slave); + Map versions, int threads, boolean slave, AutoFlush autoFlush) { + return new LuceneIndexModule(versions, threads, slave, autoFlush); } - public static LuceneIndexModule latestVersion(boolean slave) { - return new LuceneIndexModule(null, 0, slave); + public static LuceneIndexModule latestVersion(boolean slave, AutoFlush autoFlush) { + return new LuceneIndexModule(null, 0, slave, autoFlush); } static boolean isInMemoryTest(Config cfg) { return cfg.getBoolean("index", "lucene", "testInmemory", false); } - private LuceneIndexModule(Map singleVersions, int threads, boolean slave) { + private LuceneIndexModule( + Map singleVersions, int threads, boolean slave, AutoFlush autoFlush) { super(singleVersions, threads, slave); + this.autoFlush = autoFlush; + } + + @Override + protected void configure() { + super.configure(); + bind(AutoFlush.class).toInstance(autoFlush); } @Override diff --git a/java/com/google/gerrit/lucene/LuceneProjectIndex.java b/java/com/google/gerrit/lucene/LuceneProjectIndex.java index 02d86552f8..5b2a2b756a 100644 --- a/java/com/google/gerrit/lucene/LuceneProjectIndex.java +++ b/java/com/google/gerrit/lucene/LuceneProjectIndex.java @@ -29,6 +29,7 @@ import com.google.gerrit.index.query.QueryParseException; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.SitePaths; +import com.google.gerrit.server.index.AutoFlush; import com.google.gerrit.server.index.IndexUtils; import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectState; @@ -82,7 +83,8 @@ public class LuceneProjectIndex extends AbstractLuceneIndex projectCache, - @Assisted Schema schema) + @Assisted Schema schema, + AutoFlush autoFlush) throws IOException { super( schema, @@ -91,7 +93,8 @@ public class LuceneProjectIndex extends AbstractLuceneIndex clazz = Class.forName(moduleClassName); Method m = - clazz.getMethod("singleVersionWithExplicitVersions", Map.class, int.class, boolean.class); - return (Module) m.invoke(null, getSingleSchemaVersions(), 0, slave); + clazz.getMethod( + "singleVersionWithExplicitVersions", + Map.class, + int.class, + boolean.class, + AutoFlush.class); + return (Module) m.invoke(null, getSingleSchemaVersions(), 0, slave, AutoFlush.ENABLED); } catch (ClassNotFoundException | SecurityException | NoSuchMethodException -- cgit v1.2.3 From 963727cb3b33e8430f8d1795aa3a00dfb8d7b422 Mon Sep 17 00:00:00 2001 From: Adithya Chakilam Date: Thu, 4 Nov 2021 10:15:43 -0700 Subject: reindex: Use thread count specified on command line Index batch executor is not considering "--threads" flag on reindex command. Instead it considers index.batchThreads property from config if set or uses the available number of processors. Fix it to honor the "--threads" command line option when provided. Change-Id: Ia7d30b395305c102b3d21d6c848854e74d1932e9 --- Documentation/config-gerrit.txt | 2 +- Documentation/pgm-reindex.txt | 3 ++- java/com/google/gerrit/pgm/Reindex.java | 6 ++++-- java/com/google/gerrit/server/index/IndexModule.java | 8 +++++--- 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index aff17cac82..749d4c298a 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -2913,7 +2913,7 @@ by the JVM. If set to a negative value, defaults to a direct executor. [[index.batchThreads]]index.batchThreads:: + Number of threads to use for indexing in background operations, such as -online schema upgrades, and also for offline reindexing. +online schema upgrades, and also the default for offline reindexing. + If not set or set to a zero, defaults to the number of logical CPUs as returned by the JVM. If set to a negative value, defaults to a direct executor. diff --git a/Documentation/pgm-reindex.txt b/Documentation/pgm-reindex.txt index 5167277b8e..5392564df3 100644 --- a/Documentation/pgm-reindex.txt +++ b/Documentation/pgm-reindex.txt @@ -20,7 +20,8 @@ Rebuilds the secondary index. == OPTIONS --threads:: - Number of threads to use for indexing. + Number of threads to use for indexing. Default is + link:config-gerrit.html#index.batchThreads[index.batchThreads] --changes-schema-version:: Schema version to reindex; default is most recent version. diff --git a/java/com/google/gerrit/pgm/Reindex.java b/java/com/google/gerrit/pgm/Reindex.java index 85e860966f..85d1bd61ba 100644 --- a/java/com/google/gerrit/pgm/Reindex.java +++ b/java/com/google/gerrit/pgm/Reindex.java @@ -54,8 +54,10 @@ import org.eclipse.jgit.util.io.NullOutputStream; import org.kohsuke.args4j.Option; public class Reindex extends SiteProgram { - @Option(name = "--threads", usage = "Number of threads to use for indexing") - private int threads = Runtime.getRuntime().availableProcessors(); + @Option( + name = "--threads", + usage = "Number of threads to use for indexing. Default is index.batchThreads from config.") + private int threads = 0; @Option( name = "--changes-schema-version", diff --git a/java/com/google/gerrit/server/index/IndexModule.java b/java/com/google/gerrit/server/index/IndexModule.java index 3c9538c2d9..9622084489 100644 --- a/java/com/google/gerrit/server/index/IndexModule.java +++ b/java/com/google/gerrit/server/index/IndexModule.java @@ -230,11 +230,13 @@ public class IndexModule extends LifecycleModule { if (batchExecutor != null) { return batchExecutor; } - int threads = config.getInt("index", null, "batchThreads", 0); + int threads = this.threads; + if (threads == 0) { + threads = + config.getInt("index", null, "batchThreads", Runtime.getRuntime().availableProcessors()); + } if (threads < 0) { return MoreExecutors.newDirectExecutorService(); - } else if (threads == 0) { - threads = Runtime.getRuntime().availableProcessors(); } return MoreExecutors.listeningDecorator(workQueue.createQueue(threads, "Index-Batch", true)); } -- cgit v1.2.3 From b4c9c8dfe4ee362973464619581158f4b0228dff Mon Sep 17 00:00:00 2001 From: Adithya Chakilam Date: Fri, 5 Nov 2021 09:29:39 -0700 Subject: Fix buggy Index-Interactive Executor Specifying index.threads to a negative value is not using a direct executor as mentioned in docs. This change fixes that buggy behaviour. Change-Id: I924615b5e882702b3bd77973be337d40453c6d32 --- java/com/google/gerrit/server/index/IndexModule.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/java/com/google/gerrit/server/index/IndexModule.java b/java/com/google/gerrit/server/index/IndexModule.java index 9622084489..a68ce2df89 100644 --- a/java/com/google/gerrit/server/index/IndexModule.java +++ b/java/com/google/gerrit/server/index/IndexModule.java @@ -211,13 +211,14 @@ public class IndexModule extends LifecycleModule { return interactiveExecutor; } int threads = this.threads; - if (threads < 0) { - return MoreExecutors.newDirectExecutorService(); - } else if (threads == 0) { + if (threads == 0) { threads = config.getInt( "index", null, "threads", Runtime.getRuntime().availableProcessors() / 2 + 1); } + if (threads < 0) { + return MoreExecutors.newDirectExecutorService(); + } return MoreExecutors.listeningDecorator( workQueue.createQueue(threads, "Index-Interactive", true)); } -- cgit v1.2.3 From 0ed64ee206c940fccb628bdc10518cc5eb211217 Mon Sep 17 00:00:00 2001 From: Adithya Chakilam Date: Mon, 8 Nov 2021 10:00:38 -0800 Subject: Add missing "--migrate-draft-to" flag on init doc Change-Id: Ifec8e091ab0ec552139392d24f18531fcf46e881 --- Documentation/pgm-init.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Documentation/pgm-init.txt b/Documentation/pgm-init.txt index 9a16cdfa13..6b7a76db36 100644 --- a/Documentation/pgm-init.txt +++ b/Documentation/pgm-init.txt @@ -99,6 +99,11 @@ objects these SQL statements must be executed manually. The administrator must manually install the required library in the `lib/` folder. +--migrate-draft-to:: +PRIVATE | WORK_IN_PROGRESS:: + Strategy to migrate draft changes during Schema 159 migration. Applicable + only when migrating from a version lower than 2.15. + == CONTEXT This command can only be run on a server which has direct connectivity to the metadata database, and local access to the -- cgit v1.2.3