Disable reading from Changes table under notedb

The only places where we still want to access the Changes table is in
ChangeNotes.Factory, BatchUpdate and RebuildNoteDb. Here we need to
unwrap ReviewDb.

Change-Id: I90e6dc89825383757f7f84507eba177445483630
This commit is contained in:
Dave Borowitz
2016-02-10 17:58:05 -05:00
parent 8fba0e255e
commit 5e46d04109
4 changed files with 73 additions and 9 deletions

View File

@@ -47,6 +47,7 @@ import com.google.gerrit.server.index.ReindexAfterUpdate;
import com.google.gerrit.server.notedb.ChangeRebuilder;
import com.google.gerrit.server.notedb.NoteDbModule;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.schema.DisabledChangesReviewDbWrapper;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.AbstractModule;
@@ -221,13 +222,20 @@ public class RebuildNotedb extends SiteProgram {
Multimap<Project.NameKey, Change.Id> changesByProject =
ArrayListMultimap.create();
try (ReviewDb db = schemaFactory.open()) {
for (Change c : db.changes().all()) {
for (Change c : unwrap(db).changes().all()) {
changesByProject.put(c.getProject(), c.getId());
}
return changesByProject;
}
}
private static ReviewDb unwrap(ReviewDb db) {
if (db instanceof DisabledChangesReviewDbWrapper) {
db = ((DisabledChangesReviewDbWrapper) db).unsafeGetDelegate();
}
return db;
}
private static class RebuildListener implements Runnable {
private Change.Id changeId;
private ListenableFuture<?> future;

View File

@@ -47,6 +47,7 @@ import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.NoSuchRefException;
import com.google.gerrit.server.schema.DisabledChangesReviewDbWrapper;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
@@ -612,7 +613,7 @@ public class BatchUpdate implements AutoCloseable {
private ChangeContext newChangeContext(Change.Id id) throws Exception {
Change c = newChanges.get(id);
if (c == null) {
c = db.changes().get(id);
c = unwrap(db).changes().get(id);
}
// Pass in preloaded change to controlFor, to avoid:
// - reading from a db that does not belong to this update
@@ -629,4 +630,11 @@ public class BatchUpdate implements AutoCloseable {
op.postUpdate(ctx);
}
}
private static ReviewDb unwrap(ReviewDb db) {
if (db instanceof DisabledChangesReviewDbWrapper) {
db = ((DisabledChangesReviewDbWrapper) db).unsafeGetDelegate();
}
return db;
}
}

View File

@@ -61,6 +61,7 @@ import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gerrit.server.schema.DisabledChangesReviewDbWrapper;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -186,7 +187,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
public ChangeNotes create(ReviewDb db, Project.NameKey project,
Change.Id changeId) throws OrmException {
Change change = db.changes().get(changeId);
Change change = unwrap(db).changes().get(changeId);
checkArgument(change.getProject().equals(project),
"passed project %s when creating ChangeNotes for %s, but actual"
+ " project is %s",
@@ -220,7 +221,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
ReviewDb db, Change.Id changeId) throws OrmException {
checkState(!migration.readChanges(), "do not call"
+ " createFromIdOnlyWhenNotedbDisabled when notedb is enabled");
Change change = db.changes().get(changeId);
Change change = unwrap(db).changes().get(changeId);
return new ChangeNotes(repoManager, migration, allUsers,
change.getProject(), change).load();
}
@@ -242,7 +243,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
final ListeningExecutorService executorService, final ReviewDb db,
final Project.NameKey project, final Change.Id changeId) {
return Futures.makeChecked(
Futures.transformAsync(db.changes().getAsync(changeId),
Futures.transformAsync(unwrap(db).changes().getAsync(changeId),
new AsyncFunction<Change, ChangeNotes>() {
@Override
public ListenableFuture<ChangeNotes> apply(
@@ -284,7 +285,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
return notes;
}
for (Change c : db.changes().get(changeIds)) {
for (Change c : unwrap(db).changes().get(changeIds)) {
notes.add(createFromChangeOnlyWhenNotedbDisabled(c));
}
return notes;
@@ -304,7 +305,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
return notes;
}
for (Change c : db.changes().get(changeIds)) {
for (Change c : unwrap(db).changes().get(changeIds)) {
if (c != null && project.equals(c.getDest().getParentKey())) {
ChangeNotes cn = createFromChangeOnlyWhenNotedbDisabled(c);
if (predicate.apply(cn)) {
@@ -330,7 +331,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
}
}
} else {
for (Change change : db.changes().all()) {
for (Change change : unwrap(db).changes().all()) {
ChangeNotes notes = createFromChangeOnlyWhenNotedbDisabled(change);
if (predicate.apply(notes)) {
m.put(change.getProject(), notes);
@@ -356,7 +357,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
// A batch size of N may overload get(Iterable), so use something smaller,
// but still >1.
for (List<Change.Id> batch : Iterables.partition(ids, 30)) {
for (Change change : db.changes().get(batch)) {
for (Change change : unwrap(db).changes().get(batch)) {
notes.add(createFromChangeOnlyWhenNotedbDisabled(change));
}
}
@@ -385,6 +386,13 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
}
return ids;
}
private static ReviewDb unwrap(ReviewDb db) {
if (db instanceof DisabledChangesReviewDbWrapper) {
db = ((DisabledChangesReviewDbWrapper) db).unsafeGetDelegate();
}
return db;
}
}
private final Project.NameKey project;

View File

@@ -21,6 +21,7 @@ import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.server.ChangeAccess;
import com.google.gerrit.reviewdb.server.ChangeMessageAccess;
import com.google.gerrit.reviewdb.server.PatchLineCommentAccess;
import com.google.gerrit.reviewdb.server.PatchSetAccess;
@@ -33,6 +34,7 @@ import com.google.gwtorm.server.ResultSet;
public class DisabledChangesReviewDbWrapper extends ReviewDbWrapper {
private static final String MSG = "This table has been migrated to notedb";
private final DisabledChangeAccess changes;
private final DisabledPatchSetApprovalAccess patchSetApprovals;
private final DisabledChangeMessageAccess changeMessages;
private final DisabledPatchSetAccess patchSets;
@@ -40,6 +42,7 @@ public class DisabledChangesReviewDbWrapper extends ReviewDbWrapper {
DisabledChangesReviewDbWrapper(ReviewDb db) {
super(db);
changes = new DisabledChangeAccess(delegate.changes());
patchSetApprovals =
new DisabledPatchSetApprovalAccess(delegate.patchSetApprovals());
changeMessages = new DisabledChangeMessageAccess(delegate.changeMessages());
@@ -52,6 +55,11 @@ public class DisabledChangesReviewDbWrapper extends ReviewDbWrapper {
return delegate;
}
@Override
public ChangeAccess changes() {
return changes;
}
@Override
public PatchSetApprovalAccess patchSetApprovals() {
return patchSetApprovals;
@@ -72,6 +80,38 @@ public class DisabledChangesReviewDbWrapper extends ReviewDbWrapper {
return patchComments;
}
private static class DisabledChangeAccess extends ChangeAccessWrapper {
protected DisabledChangeAccess(ChangeAccess delegate) {
super(delegate);
}
@Override
public ResultSet<Change> iterateAllEntities() {
throw new UnsupportedOperationException(MSG);
}
@Override
public CheckedFuture<Change, OrmException> getAsync(Change.Id key) {
throw new UnsupportedOperationException(MSG);
}
@Override
public ResultSet<Change> get(Iterable<Change.Id> keys) {
throw new UnsupportedOperationException(MSG);
}
@Override
public Change get(Change.Id id) throws OrmException {
throw new UnsupportedOperationException(MSG);
}
@Override
public ResultSet<Change> all() throws OrmException {
throw new UnsupportedOperationException(MSG);
}
}
private static class DisabledPatchSetApprovalAccess
extends PatchSetApprovalAccessWrapper {
DisabledPatchSetApprovalAccess(PatchSetApprovalAccess delegate) {