opendev/gerrit
Code Issues Proposed changes

GC All-Users after migrating to NoteDb

Browse Source 
The NoteDb migration writes many loose objects to All-Users, since this
is the simplest way to write data to All-Users with high parallelism.
However, this results in poor performance of the All-Users repo
immediately after migration, for example taking tens of seconds to load
each account on GerritForge's server.

Work around this by running GC immediately after the migration process,
both offline and online. This is not perfect, because it is not
represented by a NotesMigrationState enum value, meaning the GC will not
be resumed if the server is uncleanly shut down during the process. (As
with all GCs, this should not result in repo corruption.)

Change-Id: Iffc89e1c125ef5db57298daccabd7e1fe4af8d27
This commit is contained in:
Dave Borowitz 2018-03-19 08:34:49 -04:00
parent f4fb10467f
commit 8f1dc546c1
4 changed files with 123 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/pgm/StandaloneNoteDbMigrationIT.java
View File

@ -28,7 +28,9 @@ import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.LocalDiskRepositoryManager;
import com.google.gerrit.server.index.GerritIndexStatus;
import com.google.gerrit.server.index.change.ChangeIndexCollection;
import com.google.gerrit.server.index.change.ChangeSchemaDefinitions;
@ -41,6 +43,11 @@ import com.google.gerrit.testutil.NoteDbMode;
import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Key;
import com.google.inject.TypeLiteral;
import java.io.File;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.stream.Stream;
import org.eclipse.jgit.internal.storage.file.FileRepository;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
@ -114,11 +121,17 @@ public class StandaloneNoteDbMigrationIT extends StandaloneSiteTest {
migrate();
assertNotesMigrationState(NotesMigrationState.NOTE_DB);
File allUsersDir;
try (ServerContext ctx = startServer()) {
GitRepositoryManager repoManager = ctx.getInjector().getInstance(GitRepositoryManager.class);
try (Repository repo = repoManager.openRepository(project)) {
assertThat(repo.exactRef(RefNames.changeMetaRef(changeId))).isNotNull();
}
assertThat(repoManager).isInstanceOf(LocalDiskRepositoryManager.class);
try (Repository repo =
repoManager.openRepository(ctx.getInjector().getInstance(AllUsersName.class))) {
allUsersDir = repo.getDirectory();
}
try (ReviewDb db = openUnderlyingReviewDb(ctx)) {
Change c = db.changes().get(changeId);
@ -137,6 +150,15 @@ public class StandaloneNoteDbMigrationIT extends StandaloneSiteTest {
}
assertNoAutoMigrateConfig(gerritConfig);
assertAutoMigrateConfig(noteDbConfig, false);
try (FileRepository repo = new FileRepository(allUsersDir)) {
try (Stream<Path> paths = Files.walk(repo.getObjectsDirectory().toPath())) {
assertThat(paths.filter(p -> !p.toString().contains("pack") && Files.isRegularFile(p)))
.named("loose object files in All-Users")
.isEmpty();
}
assertThat(repo.getObjectDatabase().getPacks()).named("packfiles in All-Users").hasSize(1);
}
}
@Test

6
gerrit-pgm/src/main/java/com/google/gerrit/pgm/MigrateToNoteDb.java
View File

@ -29,13 +29,16 @@ import com.google.gerrit.pgm.util.ThreadLimiter;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.git.GarbageCollection;
import com.google.gerrit.server.index.DummyIndexModule;
import com.google.gerrit.server.index.change.ChangeSchemaDefinitions;
import com.google.gerrit.server.notedb.rebuild.GcAllUsers;
import com.google.gerrit.server.notedb.rebuild.NoteDbMigrator;
import com.google.gerrit.server.schema.DataSourceType;
import com.google.inject.Inject;
import com.google.inject.Injector;
import com.google.inject.Provider;
import java.io.PrintWriter;
import java.util.ArrayList;
import java.util.List;
import org.kohsuke.args4j.Option;
@ -99,6 +102,7 @@ public class MigrateToNoteDb extends SiteProgram {
private LifecycleManager dbManager;
private LifecycleManager sysManager;
@Inject private GcAllUsers gcAllUsers;
@Inject private Provider<NoteDbMigrator.Builder> migratorBuilderProvider;
@Override
@ -137,6 +141,7 @@ public class MigrateToNoteDb extends SiteProgram {
migrator.migrate();
}
}
gcAllUsers.run(new PrintWriter(System.out));
} finally {
stop();
}
@ -190,6 +195,7 @@ public class MigrateToNoteDb extends SiteProgram {
install(dbInjector.getInstance(BatchProgramModule.class));
install(new DummyIndexModule());
factory(ChangeResource.Factory.class);
factory(GarbageCollection.Factory.class);
}
});
}

88
gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/GcAllUsers.java Normal file
View File

@ -0,0 +1,88 @@
// Copyright (C) 2018 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.notedb.rebuild;
import static com.google.common.base.Preconditions.checkNotNull;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.GarbageCollectionResult;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.git.GarbageCollection;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.LocalDiskRepositoryManager;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.PrintWriter;
import java.util.function.Consumer;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@Singleton
public class GcAllUsers {
private static final Logger log = LoggerFactory.getLogger(GcAllUsers.class);
private final AllUsersName allUsers;
private final GarbageCollection.Factory gcFactory;
private final GitRepositoryManager repoManager;
@Inject
GcAllUsers(
AllUsersName allUsers,
GarbageCollection.Factory gcFactory,
GitRepositoryManager repoManager) {
this.allUsers = allUsers;
this.gcFactory = gcFactory;
this.repoManager = repoManager;
}
public void runWithLogger() {
// Print log messages using logger, and skip progress.
run(s -> log.info(s), null);
}
public void run(PrintWriter writer) {
// Print both log messages and progress to given writer.
run(checkNotNull(writer)::println, writer);
}
private void run(Consumer<String> logOneLine, @Nullable PrintWriter progressWriter) {
if (!(repoManager instanceof LocalDiskRepositoryManager)) {
logOneLine.accept("Skipping GC of " + allUsers + "; not a local disk repo");
return;
}
GarbageCollectionResult result =
gcFactory.create().run(ImmutableList.of(allUsers), progressWriter);
if (!result.hasErrors()) {
return;
}
for (GarbageCollectionResult.Error e : result.getErrors()) {
switch (e.getType()) {
case GC_ALREADY_SCHEDULED:
logOneLine.accept("GC already scheduled for " + e.getProjectName());
break;
case GC_FAILED:
logOneLine.accept("GC failed for " + e.getProjectName());
break;
case REPOSITORY_NOT_FOUND:
logOneLine.accept(e.getProjectName() + " repo not found");
break;
default:
logOneLine.accept("GC failed for " + e.getProjectName() + ": " + e.getType());
break;
}
}
}
}

10
gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/OnlineNoteDbMigrator.java
View File

@ -50,19 +50,22 @@ public class OnlineNoteDbMigrator implements LifecycleListener {
}
}
private Provider<NoteDbMigrator.Builder> migratorBuilderProvider;
private final GcAllUsers gcAllUsers;
private final OnlineUpgrader indexUpgrader;
private final Provider<NoteDbMigrator.Builder> migratorBuilderProvider;
private final boolean upgradeIndex;
private final boolean trial;
@Inject
OnlineNoteDbMigrator(
@GerritServerConfig Config cfg,
Provider<NoteDbMigrator.Builder> migratorBuilderProvider,
GcAllUsers gcAllUsers,
OnlineUpgrader indexUpgrader,
Provider<NoteDbMigrator.Builder> migratorBuilderProvider,
@Named(TRIAL) boolean trial) {
this.migratorBuilderProvider = migratorBuilderProvider;
this.gcAllUsers = gcAllUsers;
this.indexUpgrader = indexUpgrader;
this.migratorBuilderProvider = migratorBuilderProvider;
this.upgradeIndex = VersionManager.getOnlineUpgrade(cfg);
this.trial = trial || NoteDbMigrator.getTrialMode(cfg);
}
@ -88,6 +91,7 @@ public class OnlineNoteDbMigrator implements LifecycleListener {
} catch (Exception e) {
log.error("Error in online NoteDb migration", e);
}
gcAllUsers.runWithLogger();
log.info("Online NoteDb migration completed in {}s", sw.elapsed(TimeUnit.SECONDS));
if (upgradeIndex) {