NoteDbMigrator: Use new pack-based inserter implementation
The migrator can potentially create many thousands of new objects for
NoteDb data, more than we want to leave loose in the repository.
Upstream JGit added a new ObjectInserter implementation that inserts
objects directly into packs instead. We can now use this to share a
single inserter for the whole change repository.
Only use this strategy during migration. We would rather make loose
objects in a live server, since object lookup is linear in the number of
packs, so we wouldn't want to continually create new packs without
compacting them.
Bug: Issue 7399
Change-Id: I3963875525e7963f209081a6881c4c77e015122e
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
index 51e2964..6b14263 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
@@ -60,8 +60,17 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
import java.io.IOException;
+import java.nio.file.FileVisitResult;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.SimpleFileVisitor;
+import java.nio.file.attribute.BasicFileAttributes;
import java.util.ArrayList;
import java.util.List;
+import java.util.Set;
+import java.util.SortedSet;
+import java.util.TreeSet;
+import org.eclipse.jgit.internal.storage.file.FileRepository;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
@@ -374,10 +383,14 @@
Change.Id id1 = r1.getChange().getId();
Change.Id id2 = r2.getChange().getId();
- migrate(b -> b.setThreads(threads));
- assertNotesMigrationState(NOTE_DB, false, false);
+ Set<String> objectFiles = getObjectFiles(project);
+ assertThat(objectFiles).isNotEmpty();
+ migrate(b -> b.setThreads(threads));
+
+ assertNotesMigrationState(NOTE_DB, false, false);
assertThat(sequences.nextChangeId()).isEqualTo(503);
+ assertThat(getObjectFiles(project)).containsExactlyElementsIn(objectFiles);
ObjectId oldMetaId = null;
int rowVersion = 0;
@@ -531,4 +544,23 @@
private void addListener(NotesMigrationStateListener listener) {
addedListeners.add(listeners.add(listener));
}
+
+ private SortedSet<String> getObjectFiles(Project.NameKey project) throws Exception {
+ SortedSet<String> files = new TreeSet<>();
+ try (Repository repo = repoManager.openRepository(project)) {
+ Files.walkFileTree(
+ ((FileRepository) repo).getObjectDatabase().getDirectory().toPath(),
+ new SimpleFileVisitor<Path>() {
+ @Override
+ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) {
+ String name = file.getFileName().toString();
+ if (!attrs.isDirectory() && !name.endsWith(".pack") && !name.endsWith(".idx")) {
+ files.add(name);
+ }
+ return FileVisitResult.CONTINUE;
+ }
+ });
+ }
+ return files;
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
index 3f20766..3e36de9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
@@ -309,14 +309,14 @@
public NoteDbUpdateManager setChangeRepo(
Repository repo, RevWalk rw, @Nullable ObjectInserter ins, ChainedReceiveCommands cmds) {
checkState(changeRepo == null, "change repo already initialized");
- changeRepo = new OpenRepo(repo, rw, ins, cmds, false, true);
+ changeRepo = new OpenRepo(repo, rw, ins, cmds, false, saveObjects);
return this;
}
public NoteDbUpdateManager setAllUsersRepo(
Repository repo, RevWalk rw, @Nullable ObjectInserter ins, ChainedReceiveCommands cmds) {
checkState(allUsersRepo == null, "All-Users repo already initialized");
- allUsersRepo = new OpenRepo(repo, rw, ins, cmds, false, true);
+ allUsersRepo = new OpenRepo(repo, rw, ins, cmds, false, saveObjects);
return this;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
index 3f72ee5..2981174 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
@@ -67,6 +67,7 @@
import com.google.gerrit.server.notedb.RepoSequence;
import com.google.gerrit.server.notedb.rebuild.ChangeRebuilder.NoPatchSetsException;
import com.google.gerrit.server.project.NoSuchChangeException;
+import com.google.gerrit.server.update.ChainedReceiveCommands;
import com.google.gerrit.server.util.ManualRequestContext;
import com.google.gerrit.server.util.ThreadLocalRequestContext;
import com.google.gwtorm.server.OrmException;
@@ -89,9 +90,14 @@
import java.util.function.Predicate;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.internal.storage.file.FileRepository;
import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.ProgressMonitor;
+import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.lib.TextProgressMonitor;
+import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.storage.file.FileBasedConfig;
import org.eclipse.jgit.util.FS;
import org.eclipse.jgit.util.io.NullOutputStream;
@@ -741,6 +747,12 @@
return ImmutableListMultimap.copyOf(out);
}
+ private static ObjectInserter newPackInserter(Repository repo) {
+ return repo instanceof FileRepository
+ ? ((FileRepository) repo).getObjectDatabase().newPackInserter()
+ : repo.newObjectInserter();
+ }
+
private boolean rebuildProject(
ReviewDb db,
ImmutableListMultimap<Project.NameKey, Change.Id> allChanges,
@@ -750,8 +762,26 @@
ProgressMonitor pm =
new TextProgressMonitor(
new PrintWriter(new BufferedWriter(new OutputStreamWriter(progressOut, UTF_8))));
- try (NoteDbUpdateManager manager =
- updateManagerFactory.create(project).setSaveObjects(false).setAtomicRefUpdates(false)) {
+ try (Repository changeRepo = repoManager.openRepository(project);
+ ObjectInserter changeIns = newPackInserter(changeRepo);
+ ObjectReader changeReader = changeIns.newReader();
+ RevWalk changeRw = new RevWalk(changeReader);
+ NoteDbUpdateManager manager =
+ updateManagerFactory
+ .create(project)
+ .setSaveObjects(false)
+ .setAtomicRefUpdates(false)
+ // Only use a PackInserter for the change repo, not All-Users.
+ //
+ // It's not possible to share a single inserter for All-Users across all project
+ // tasks, and we don't want to add one pack per project to All-Users. Adding many
+ // loose objects is preferable to many packs.
+ //
+ // Anyway, the number of objects inserted into All-Users is proportional to the
+ // number of pending draft comments, which should not be high (relative to the total
+ // number of changes), so the number of loose objects shouldn't be too unreasonable.
+ .setChangeRepo(
+ changeRepo, changeRw, changeIns, new ChainedReceiveCommands(changeRepo))) {
Set<Change.Id> skipExecute = new HashSet<>();
Collection<Change.Id> changes = allChanges.get(project);
pm.beginTask(FormatUtil.elide("Rebuilding " + project.get(), 50), changes.size());
@@ -763,8 +793,6 @@
staged = true;
} catch (NoPatchSetsException e) {
log.warn(e.getMessage());
- } catch (RepositoryNotFoundException e) {
- log.warn("Repository {} not found while rebuilding change {}", project, changeId);
} catch (Throwable t) {
log.error("Failed to rebuild change " + changeId, t);
ok = false;
@@ -812,6 +840,10 @@
} catch (OrmException | IOException e) {
log.error("Failed to save NoteDb state for " + project, e);
}
+ } catch (RepositoryNotFoundException e) {
+ log.warn("Repository {} not found", project);
+ } catch (IOException e) {
+ log.error("Failed to rebuild project " + project, e);
}
return ok;
}
