Support writing notedb data into a separate set of repositories
We want adventurous server admins to be able to experimentally turn on
writes for notedb data soon, long before notedb is ready for
production use. However, there are some performance concerns, such as:
- Doubling the number of refs in every repo.
- Increasing GC cost.
- Reduced object locality within packs.
- Creating lots of garbage (including loose objects) as the notedb is
regenerated.
Some of these performance issues we will need to address in JGit (e.g.
by modifying the repacker), and some might not turn out to be so bad.
For improved isolation during this transitional period, allow for
storing all metadata in a separate repository from the main one. This
can have a separate repack schedule and can be safely blown away as
necessary.
In the local disk case, metadata repositories are stored by default in
$site_path/notedb. We are not committing to this location long-term,
and there is an undocumented configuration option that may change.
Adventurous admins are responsible for rebuilding the notedb and
cleaning up garbage if this configuration changes.
Change-Id: I8158e3f46eb8378ca1726cf6bad67efa669b40d9
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/GitRepositoryManager.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/GitRepositoryManager.java
index ffb91ce..dee2df0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/GitRepositoryManager.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/GitRepositoryManager.java
@@ -46,6 +46,9 @@
/**
* Create (and open) a repository by name.
+ * <p>
+ * If the implementation supports separate metadata repositories, this method
+ * must also create the metadata repository, but does not open it.
*
* @param name the repository name, relative to the base directory.
* @return the cached Repository instance. Caller must call {@code close()}
@@ -59,6 +62,23 @@
throws RepositoryCaseMismatchException, RepositoryNotFoundException,
IOException;
+ /**
+ * Open the repository storing metadata for the given project.
+ * <p>
+ * This includes any project-specific metadata <em>except</em> what is stored
+ * in {@code refs/meta/config}. Implementations may choose to store all
+ * metadata in the original project.
+ *
+ * @param name the base project name name.
+ * @return the cached metadata Repository instance. Caller must call
+ * {@code close()} when done to decrement the resource handle.
+ * @throws RepositoryNotFoundException the name does not denote an existing
+ * repository.
+ * @throws IOException the name cannot be read as a repository.
+ */
+ public abstract Repository openMetadataRepository(Project.NameKey name)
+ throws RepositoryNotFoundException, IOException;
+
/** @return set of all known projects, sorted by natural NameKey order. */
public abstract SortedSet<Project.NameKey> list();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/LocalDiskRepositoryManager.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/LocalDiskRepositoryManager.java
index 196d3e9..f2c77cc4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/LocalDiskRepositoryManager.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/LocalDiskRepositoryManager.java
@@ -14,11 +14,15 @@
package com.google.gerrit.server.git;
+import static com.google.common.base.Preconditions.checkState;
+
+import com.google.common.base.Objects;
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.config.SitePaths;
+import com.google.gerrit.server.notedb.NotesMigration;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -124,16 +128,25 @@
}
private final File basePath;
+ private final File noteDbPath;
private final Lock namesUpdateLock;
private volatile SortedSet<Project.NameKey> names;
@Inject
- LocalDiskRepositoryManager(final SitePaths site,
- @GerritServerConfig final Config cfg) {
+ LocalDiskRepositoryManager(SitePaths site,
+ @GerritServerConfig Config cfg,
+ NotesMigration notesMigration) {
basePath = site.resolve(cfg.getString("gerrit", null, "basePath"));
if (basePath == null) {
throw new IllegalStateException("gerrit.basePath must be configured");
}
+
+ if (notesMigration.enabled()) {
+ noteDbPath = site.resolve(Objects.firstNonNull(
+ cfg.getString("gerrit", null, "noteDbPath"), "notedb"));
+ } else {
+ noteDbPath = null;
+ }
namesUpdateLock = new ReentrantLock(true /* fair */);
names = list();
}
@@ -143,28 +156,30 @@
return basePath;
}
- private File gitDirOf(Project.NameKey name) {
- return new File(getBasePath(), name.get());
+ public Repository openRepository(Project.NameKey name)
+ throws RepositoryNotFoundException {
+ return openRepository(basePath, name);
}
- public Repository openRepository(Project.NameKey name)
+ private Repository openRepository(File path, Project.NameKey name)
throws RepositoryNotFoundException {
if (isUnreasonableName(name)) {
throw new RepositoryNotFoundException("Invalid name: " + name);
}
+ File gitDir = new File(path, name.get());
if (!names.contains(name)) {
// The this.names list does not hold the project-name but it can still exist
// on disk; for instance when the project has been created directly on the
// file-system through replication.
//
if (!name.get().endsWith(Constants.DOT_GIT_EXT)) {
- if (FileKey.resolve(gitDirOf(name), FS.DETECTED) != null) {
+ if (FileKey.resolve(gitDir, FS.DETECTED) != null) {
onCreateProject(name);
} else {
- throw new RepositoryNotFoundException(gitDirOf(name));
+ throw new RepositoryNotFoundException(gitDir);
}
} else {
- final File directory = gitDirOf(name);
+ final File directory = gitDir;
if (FileKey.isGitRepository(new File(directory, Constants.DOT_GIT),
FS.DETECTED)) {
onCreateProject(name);
@@ -172,11 +187,11 @@
directory.getName() + Constants.DOT_GIT_EXT), FS.DETECTED)) {
onCreateProject(name);
} else {
- throw new RepositoryNotFoundException(gitDirOf(name));
+ throw new RepositoryNotFoundException(gitDir);
}
}
}
- final FileKey loc = FileKey.lenient(gitDirOf(name), FS.DETECTED);
+ final FileKey loc = FileKey.lenient(gitDir, FS.DETECTED);
try {
return RepositoryCache.open(loc);
} catch (IOException e1) {
@@ -187,13 +202,22 @@
}
}
- public Repository createRepository(final Project.NameKey name)
+ public Repository createRepository(Project.NameKey name)
+ throws RepositoryNotFoundException, RepositoryCaseMismatchException {
+ Repository repo = createRepository(basePath, name);
+ if (noteDbPath != null) {
+ createRepository(noteDbPath, name);
+ }
+ return repo;
+ }
+
+ private Repository createRepository(File path, Project.NameKey name)
throws RepositoryNotFoundException, RepositoryCaseMismatchException {
if (isUnreasonableName(name)) {
throw new RepositoryNotFoundException("Invalid name: " + name);
}
- File dir = FileKey.resolve(gitDirOf(name), FS.DETECTED);
+ File dir = FileKey.resolve(new File(path, name.get()), FS.DETECTED);
FileKey loc;
if (dir != null) {
// Already exists on disk, use the repository we found.
@@ -208,7 +232,7 @@
// of the repository name, so prefer the standard bare name.
//
String n = name.get() + Constants.DOT_GIT_EXT;
- loc = FileKey.exact(new File(basePath, n), FS.DETECTED);
+ loc = FileKey.exact(new File(path, n), FS.DETECTED);
}
try {
@@ -231,6 +255,17 @@
}
}
+ @Override
+ public Repository openMetadataRepository(Project.NameKey name)
+ throws RepositoryNotFoundException, IOException {
+ checkState(noteDbPath != null, "notedb disabled");
+ try {
+ return openRepository(noteDbPath, name);
+ } catch (RepositoryNotFoundException e) {
+ return createRepository(noteDbPath, name);
+ }
+ }
+
private void onCreateProject(final Project.NameKey newProjectName) {
namesUpdateLock.lock();
try {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
index dc77ada..9d93d89 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
@@ -44,7 +44,7 @@
if (!loaded) {
Repository repo;
try {
- repo = repoManager.openRepository(getProjectName());
+ repo = repoManager.openMetadataRepository(getProjectName());
} catch (IOException e) {
throw new OrmException(e);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
index 583b823..ee08692 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
@@ -93,7 +93,7 @@
private void load() throws IOException {
if (migration.write() && getRevision() == null) {
- Repository repo = repoManager.openRepository(getProjectName());
+ Repository repo = repoManager.openMetadataRepository(getProjectName());
try {
load(repo);
} catch (ConfigInvalidException e) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
index 22156c4..83cdc68 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
@@ -87,7 +87,8 @@
this.changeId = change.getId();
this.tip = tip;
this.walk = walk;
- this.repo = repoManager.openRepository(ChangeNotes.getProjectName(change));
+ this.repo =
+ repoManager.openMetadataRepository(ChangeNotes.getProjectName(change));
approvals = Maps.newHashMap();
reviewers = Maps.newLinkedHashMap();
allPastReviewers = Lists.newArrayList();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/DraftCommentNotesParser.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/DraftCommentNotesParser.java
index a55c8da..4b3fbdf 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/DraftCommentNotesParser.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/DraftCommentNotesParser.java
@@ -50,7 +50,7 @@
this.changeId = changeId;
this.walk = walk;
this.tip = tip;
- this.repo = repoManager.openRepository(draftsProject);
+ this.repo = repoManager.openMetadataRepository(draftsProject);
this.author = author;
draftBaseComments = ArrayListMultimap.create();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigration.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigration.java
index d2747bf..208ad62 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigration.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigration.java
@@ -54,6 +54,13 @@
cfg.getBoolean("notedb", "comments", "read", false);
}
+ public boolean enabled() {
+ return readChangeMessages()
+ || readComments()
+ || readPatchSetApprovals()
+ || write();
+ }
+
public boolean write() {
return write;
}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryRepositoryManager.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryRepositoryManager.java
index 328df75..0635464 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryRepositoryManager.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryRepositoryManager.java
@@ -80,6 +80,12 @@
}
@Override
+ public InMemoryRepository openMetadataRepository(Project.NameKey name)
+ throws RepositoryNotFoundException {
+ return openRepository(name);
+ }
+
+ @Override
public SortedSet<Project.NameKey> list() {
SortedSet<Project.NameKey> names = Sets.newTreeSet();
for (DfsRepository repo : repos.values()) {
