opendev/gerrit
Code Issues Proposed changes

Ensure NotesMigration method calls aren't cached

Browse Source 
This will soon no longer be an immutable singleton read at startup, so
we shouldn't store the results of method calls in other singletons'
constructors.

Change-Id: I22ef7111586a84d71413876d451611bc8d93b6c6
This commit is contained in:
Dave Borowitz
2016-02-11 16:06:27 -05:00
parent 89b9bbc4d0
commit 8f5db32405
3 changed files with 11 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -37,12 +37,8 @@ public class Sequences {
AllProjectsName allProjects) { AllProjectsName allProjects) {
this.db = db; this.db = db;
this.migration = migration; this.migration = migration;
if (migration.readChanges()) {
changeSeq = new RepoSequence( changeSeq = new RepoSequence(
repoManager, allProjects, "changes", ReviewDb.FIRST_CHANGE_ID, 100); repoManager, allProjects, "changes", ReviewDb.FIRST_CHANGE_ID, 100);
} else {
changeSeq = null;
}
} }
@SuppressWarnings("deprecation") @SuppressWarnings("deprecation")

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

@@ -122,6 +122,7 @@ public class LocalDiskRepositoryManager implements GitRepositoryManager {
} }
private final Path basePath; private final Path basePath;
private final NotesMigration notesMigration;
private final Path noteDbPath; private final Path noteDbPath;
private final Lock namesUpdateLock; private final Lock namesUpdateLock;
private volatile SortedSet<Project.NameKey> names; private volatile SortedSet<Project.NameKey> names;
@@ -130,17 +131,14 @@ public class LocalDiskRepositoryManager implements GitRepositoryManager {
LocalDiskRepositoryManager(SitePaths site, LocalDiskRepositoryManager(SitePaths site,
@GerritServerConfig Config cfg, @GerritServerConfig Config cfg,
NotesMigration notesMigration) { NotesMigration notesMigration) {
this.notesMigration = notesMigration;
basePath = site.resolve(cfg.getString("gerrit", null, "basePath")); basePath = site.resolve(cfg.getString("gerrit", null, "basePath"));
if (basePath == null) { if (basePath == null) {
throw new IllegalStateException("gerrit.basePath must be configured"); throw new IllegalStateException("gerrit.basePath must be configured");
} }
if (notesMigration.enabled()) {
noteDbPath = site.resolve(MoreObjects.firstNonNull( noteDbPath = site.resolve(MoreObjects.firstNonNull(
cfg.getString("gerrit", null, "noteDbPath"), "notedb")); cfg.getString("gerrit", null, "noteDbPath"), "notedb"));
} else {
noteDbPath = null;
}
namesUpdateLock = new ReentrantLock(true /* fair */); namesUpdateLock = new ReentrantLock(true /* fair */);
names = list(); names = list();
} }
@@ -201,7 +199,7 @@ public class LocalDiskRepositoryManager implements GitRepositoryManager {
public Repository createRepository(Project.NameKey name) public Repository createRepository(Project.NameKey name)
throws RepositoryNotFoundException, RepositoryCaseMismatchException { throws RepositoryNotFoundException, RepositoryCaseMismatchException {
Repository repo = createRepository(basePath, name); Repository repo = createRepository(basePath, name);
if (noteDbPath != null && !noteDbPath.equals(basePath)) { if (notesMigration.writeChanges() && !noteDbPath.equals(basePath)) {
createRepository(noteDbPath, name); createRepository(noteDbPath, name);
} }
return repo; return repo;
@@ -266,7 +264,7 @@ public class LocalDiskRepositoryManager implements GitRepositoryManager {
@Override @Override
public Repository openMetadataRepository(Project.NameKey name) public Repository openMetadataRepository(Project.NameKey name)
throws RepositoryNotFoundException, IOException { throws RepositoryNotFoundException, IOException {
checkState(noteDbPath != null, "notedb disabled"); checkState(notesMigration.readChanges(), "notedb disabled");
try { try {
return openRepository(noteDbPath, name); return openRepository(noteDbPath, name);
} catch (RepositoryNotFoundException e) { } catch (RepositoryNotFoundException e) {

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

@@ -24,20 +24,20 @@ import com.google.inject.Singleton;
@Singleton @Singleton
public class NotesMigrationSchemaFactory implements SchemaFactory<ReviewDb> { public class NotesMigrationSchemaFactory implements SchemaFactory<ReviewDb> {
private final SchemaFactory<ReviewDb> delegate; private final SchemaFactory<ReviewDb> delegate;
private final boolean disableChangesTables; private final NotesMigration migration;
@Inject @Inject
NotesMigrationSchemaFactory( NotesMigrationSchemaFactory(
@ReviewDbFactory SchemaFactory<ReviewDb> delegate, @ReviewDbFactory SchemaFactory<ReviewDb> delegate,
NotesMigration migration) { NotesMigration migration) {
this.delegate = delegate; this.delegate = delegate;
disableChangesTables = migration.readChanges(); this.migration = migration;
} }
@Override @Override
public ReviewDb open() throws OrmException { public ReviewDb open() throws OrmException {
ReviewDb db = delegate.open(); ReviewDb db = delegate.open();
if (!disableChangesTables) { if (!migration.readChanges()) {
return db; return db;
} }
return new DisabledChangesReviewDbWrapper(db); return new DisabledChangesReviewDbWrapper(db);