opendev/gerrit
Code Issues Proposed changes

Remove GitRepositoryManager#openMetadataRepository

Browse Source 
We were originally going to use this for a less-risky migration of
googlesource.com data. Our current migration plan no longer calls for
this, so let's get rid of the complexity. As far as
LocalDiskRepositoryManager goes, we don't expect anybody to need to
enable this before we feel pretty good about the behavior in the same
repo.

Change-Id: I99c7218a190d634ff31e8293f592f75f47a50724
This commit is contained in:
Dave Borowitz
2016-04-08 16:59:10 -07:00
parent 309f8835f3
commit 5c0f0825a3
19 changed files with 36 additions and 107 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
gerrit-server/src/main/java/com/google/gerrit/server/PatchLineCommentsUtil.java
View File

@@ -311,7 +311,7 @@ public class PatchLineCommentsUtil {
public void deleteAllDraftsFromAllUsers(Change.Id changeId)
throws IOException {
try (Repository repo = repoManager.openMetadataRepository(allUsers);
try (Repository repo = repoManager.openRepository(allUsers);
RevWalk rw = new RevWalk(repo)) {
BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate();
for (Ref ref : getDraftRefs(repo, changeId).values()) {
@@ -373,7 +373,7 @@ public class PatchLineCommentsUtil {
}
private Set<String> getRefNamesAllUsers(String prefix) throws OrmException {
try (Repository repo = repoManager.openMetadataRepository(allUsers)) {
try (Repository repo = repoManager.openRepository(allUsers)) {
RefDatabase refDb = repo.getRefDatabase();
return refDb.getRefs(prefix).keySet();
} catch (IOException e) {
@@ -383,7 +383,7 @@ public class PatchLineCommentsUtil {
public Map<String, Ref> getDraftRefs(Change.Id changeId)
throws OrmException {
try (Repository repo = repoManager.openMetadataRepository(allUsers)) {
try (Repository repo = repoManager.openRepository(allUsers)) {
return getDraftRefs(repo, changeId);
} catch (IOException e) {
throw new OrmException(e);

8
gerrit-server/src/main/java/com/google/gerrit/server/StarredChangesUtil.java
View File

@@ -87,7 +87,7 @@ public class StarredChangesUtil {
if (!migration.writeAccounts()) {
return;
}
try (Repository repo = repoManager.openMetadataRepository(allUsers);
try (Repository repo = repoManager.openRepository(allUsers);
RevWalk rw = new RevWalk(repo)) {
RefUpdate u = repo.updateRef(
RefNames.refsStarredChanges(accountId, changeId));
@@ -136,7 +136,7 @@ public class StarredChangesUtil {
if (!migration.writeAccounts()) {
return;
}
try (Repository repo = repoManager.openMetadataRepository(allUsers);
try (Repository repo = repoManager.openRepository(allUsers);
RevWalk rw = new RevWalk(repo)) {
RefUpdate u = repo.updateRef(
RefNames.refsStarredChanges(accountId, changeId));
@@ -174,7 +174,7 @@ public class StarredChangesUtil {
if (!migration.writeAccounts()) {
return;
}
try (Repository repo = repoManager.openMetadataRepository(allUsers);
try (Repository repo = repoManager.openRepository(allUsers);
RevWalk rw = new RevWalk(repo)) {
BatchRefUpdate batchUpdate = repo.getRefDatabase().newBatchUpdate();
batchUpdate.setAllowNonFastForwards(true);
@@ -251,7 +251,7 @@ public class StarredChangesUtil {
}
private Set<String> getRefNames(String prefix) throws OrmException {
try (Repository repo = repoManager.openMetadataRepository(allUsers)) {
try (Repository repo = repoManager.openRepository(allUsers)) {
RefDatabase refDb = repo.getRefDatabase();
return refDb.getRefs(prefix).keySet();
} catch (IOException e) {

20
gerrit-server/src/main/java/com/google/gerrit/server/git/GitRepositoryManager.java
View File

@@ -46,9 +46,6 @@ public interface GitRepositoryManager {
/**
* 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()}
@@ -62,23 +59,6 @@ public interface GitRepositoryManager {
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.
*/
Repository openMetadataRepository(Project.NameKey name)
throws RepositoryNotFoundException, IOException;
/** @return set of all known projects, sorted by natural NameKey order. */
SortedSet<Project.NameKey> list();

33
gerrit-server/src/main/java/com/google/gerrit/server/git/LocalDiskRepositoryManager.java
View File

@@ -14,16 +14,12 @@
package com.google.gerrit.server.git;
import static com.google.common.base.Preconditions.checkState;
import com.google.common.base.MoreObjects;
import com.google.gerrit.extensions.events.LifecycleListener;
import com.google.gerrit.lifecycle.LifecycleModule;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
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,23 +120,17 @@ public class LocalDiskRepositoryManager implements GitRepositoryManager,
}
private final Path basePath;
private final NotesMigration notesMigration;
private final Path noteDbPath;
private final Lock namesUpdateLock;
private volatile SortedSet<Project.NameKey> names = new TreeSet<>();
@Inject
LocalDiskRepositoryManager(SitePaths site,
@GerritServerConfig Config cfg,
NotesMigration notesMigration) {
this.notesMigration = notesMigration;
@GerritServerConfig Config cfg) {
basePath = site.resolve(cfg.getString("gerrit", null, "basePath"));
if (basePath == null) {
throw new IllegalStateException("gerrit.basePath must be configured");
}
noteDbPath = site.resolve(MoreObjects.firstNonNull(
cfg.getString("gerrit", null, "noteDbPath"), "notedb"));
namesUpdateLock = new ReentrantLock(true /* fair */);
}
@@ -213,15 +203,7 @@ public class LocalDiskRepositoryManager implements GitRepositoryManager,
@Override
public Repository createRepository(Project.NameKey name)
throws RepositoryNotFoundException, RepositoryCaseMismatchException {
Repository repo = createRepository(getBasePath(name), name);
if (notesMigration.writeChanges() && !noteDbPath.equals(basePath)) {
createRepository(noteDbPath, name);
}
return repo;
}
private Repository createRepository(Path path, Project.NameKey name)
throws RepositoryNotFoundException, RepositoryCaseMismatchException {
Path path = getBasePath(name);
if (isUnreasonableName(name)) {
throw new RepositoryNotFoundException("Invalid name: " + name);
}
@@ -276,17 +258,6 @@ public class LocalDiskRepositoryManager implements GitRepositoryManager,
}
}
@Override
public Repository openMetadataRepository(Project.NameKey name)
throws RepositoryNotFoundException, IOException {
checkState(notesMigration.readChanges(), "NoteDb disabled");
try {
return openRepository(noteDbPath, name);
} catch (RepositoryNotFoundException e) {
return createRepository(noteDbPath, name);
}
}
private void onCreateProject(final Project.NameKey newProjectName) {
namesUpdateLock.lock();
try {

4
gerrit-server/src/main/java/com/google/gerrit/server/git/MultiBaseLocalDiskRepositoryManager.java
View File

@@ -21,7 +21,6 @@ import com.google.gerrit.reviewdb.client.Project.NameKey;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.RepositoryConfig;
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.inject.Inject;
import org.eclipse.jgit.lib.Config;
@@ -48,9 +47,8 @@ public class MultiBaseLocalDiskRepositoryManager extends
@Inject
MultiBaseLocalDiskRepositoryManager(SitePaths site,
@GerritServerConfig Config cfg,
NotesMigration notesMigration,
RepositoryConfig config) {
super(site, cfg, notesMigration);
super(site, cfg);
this.config = config;
for (Path alternateBasePath : config.getAllBasePaths()) {

6
gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
View File

@@ -125,8 +125,7 @@ public abstract class AbstractChangeNotes<T> {
return self();
}
try (Timer1.Context timer = args.metrics.readLatency.start(CHANGES);
Repository repo =
args.repoManager.openMetadataRepository(getProjectName());
Repository repo = args.repoManager.openRepository(getProjectName());
LoadHandle handle = openHandle(repo)) {
revision = handle.id();
onLoad(handle);
@@ -155,8 +154,7 @@ public abstract class AbstractChangeNotes<T> {
} else if (!args.migration.enabled()) {
return null;
}
try (Repository repo =
args.repoManager.openMetadataRepository(getProjectName())) {
try (Repository repo = args.repoManager.openRepository(getProjectName())) {
Ref ref = repo.getRefDatabase().exactRef(getRefName());
return ref != null ? ref.getObjectId() : null;
} catch (IOException e) {

2
gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
View File

@@ -134,7 +134,7 @@ class ChangeNotesParser implements AutoCloseable {
this.id = changeId;
this.tip = tip;
this.walk = walk;
this.repo = repoManager.openMetadataRepository(project);
this.repo = repoManager.openRepository(project);
this.noteUtil = noteUtil;
this.metrics = metrics;
approvals = Maps.newHashMap();

16
gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
View File

@@ -55,8 +55,8 @@ import java.util.Set;
/**
* Object to manage a single sequence of updates to NoteDb.
* <p>
* Instances are one-time-use. Handles updating both the change meta repo and
* the All-Users meta repo for any affected changes, with proper ordering.
* Instances are one-time-use. Handles updating both the change repo and the
* All-Users repo for any affected changes, with proper ordering.
* <p>
* To see the state that would be applied prior to executing the full sequence
* of updates, use {@link #stage()}.
@@ -161,26 +161,24 @@ public class NoteDbUpdateManager {
private void initCodeRepo() throws IOException {
if (codeRepo == null) {
codeRepo = openRepo(projectName, false);
codeRepo = openRepo(projectName);
}
}
private void initChangeRepo() throws IOException {
if (changeRepo == null) {
changeRepo = openRepo(projectName, true);
changeRepo = openRepo(projectName);
}
}
private void initAllUsersRepo() throws IOException {
if (allUsersRepo == null) {
allUsersRepo = openRepo(allUsersName, true);
allUsersRepo = openRepo(allUsersName);
}
}
private OpenRepo openRepo(Project.NameKey p, boolean meta) throws IOException {
Repository repo = meta
? repoManager.openMetadataRepository(p)
: repoManager.openRepository(p);
private OpenRepo openRepo(Project.NameKey p) throws IOException {
Repository repo = repoManager.openRepository(p);
ObjectInserter ins = repo.newObjectInserter();
return new OpenRepo(repo, new RevWalk(ins.newReader()), ins,
new ChainedReceiveCommands(), true);

2
gerrit-server/src/main/java/com/google/gerrit/server/notedb/RepoSequence.java
View File

@@ -129,7 +129,7 @@ public class RepoSequence {
}
private void acquire() throws OrmException {
try (Repository repo = repoManager.openMetadataRepository(projectName);
try (Repository repo = repoManager.openRepository(projectName);
RevWalk rw = new RevWalk(repo)) {
TryAcquire attempt = new TryAcquire(repo, rw);
RefUpdate.Result result = retryer.call(attempt);