opendev/gerrit
Code Issues Proposed changes

Merge changes from topic 'reviewdb-primary'

Browse Source 
* changes:
  Support migrating PrimaryStorage from NoteDb -> ReviewDb
  Support read-only lease in NoteDb meta graph
  AbstractChangeUpdate: Clarify when notes can be null
  ChangeRebuilder: Add method to rebuild NoteDb -> ReviewDb
This commit is contained in:
Dave Borowitz
2017-02-17 02:08:33 +00:00
committed by Gerrit Code Review
parent 069c0a4f46 6ff2aef6f8
commit 34f51034c9
16 changed files with 580 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

135
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/NoteDbPrimaryIT.java
View File

@@ -17,6 +17,7 @@ package com.google.gerrit.acceptance.server.notedb;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assert_;
import static com.google.common.truth.TruthJUnit.assume;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.formatTime;
import static com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage.REVIEW_DB;
import static java.util.concurrent.TimeUnit.DAYS;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
@@ -27,6 +28,8 @@ import com.github.rholder.retry.Retryer;
import com.github.rholder.retry.RetryerBuilder;
import com.github.rholder.retry.StopStrategies;
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
@@ -34,6 +37,7 @@ import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.api.changes.DraftInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.ReviewInput.CommentInput;
import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.common.ApprovalInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
@@ -41,14 +45,24 @@ import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDbUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.CommentsUtil;
import com.google.gerrit.server.InternalUser;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.git.BatchUpdate;
import com.google.gerrit.server.git.RepoRefCache;
import com.google.gerrit.server.notedb.ChangeBundle;
import com.google.gerrit.server.notedb.ChangeBundleReader;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.notedb.NoteDbChangeState;
import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.notedb.PrimaryStorageMigrator;
import com.google.gerrit.server.notedb.TestChangeRebuilderWrapper;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.testutil.ConfigSuite;
import com.google.gerrit.testutil.NoteDbMode;
import com.google.gerrit.testutil.TestTimeUtil;
@@ -62,7 +76,10 @@ import java.util.Collections;
import java.util.List;
import java.util.Optional;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
@@ -78,8 +95,13 @@ public class NoteDbPrimaryIT extends AbstractDaemonTest {
}
@Inject private AllUsersName allUsers;
@Inject private BatchUpdate.Factory batchUpdateFactory;
@Inject private ChangeBundleReader bundleReader;
@Inject private CommentsUtil commentsUtil;
@Inject private TestChangeRebuilderWrapper rebuilderWrapper;
@Inject private ChangeControl.GenericFactory changeControlFactory;
@Inject private ChangeUpdate.Factory updateFactory;
@Inject private InternalUser.Factory internalUserFactory;
private PrimaryStorageMigrator migrator;
@@ -94,7 +116,17 @@ public class NoteDbPrimaryIT extends AbstractDaemonTest {
private PrimaryStorageMigrator newMigrator(
@Nullable Retryer<NoteDbChangeState> ensureRebuiltRetryer) {
return new PrimaryStorageMigrator(
cfg, Providers.of(db), repoManager, allUsers, rebuilderWrapper, ensureRebuiltRetryer);
cfg,
Providers.of(db),
repoManager,
allUsers,
rebuilderWrapper,
ensureRebuiltRetryer,
changeControlFactory,
queryProvider,
updateFactory,
internalUserFactory,
batchUpdateFactory);
}
@After
@@ -368,6 +400,105 @@ public class NoteDbPrimaryIT extends AbstractDaemonTest {
assertNoteDbPrimary(id);
}
@Test
public void rebuildReviewDb() throws Exception {
Change c = createChange().getChange().change();
Change.Id id = c.getId();
CommentInput cin = new CommentInput();
cin.line = 1;
cin.message = "Published comment";
ReviewInput rin = ReviewInput.approve();
rin.comments = ImmutableMap.of(PushOneCommit.FILE_NAME, ImmutableList.of(cin));
gApi.changes().id(id.get()).current().review(ReviewInput.approve());
DraftInput din = new DraftInput();
din.path = PushOneCommit.FILE_NAME;
din.line = 1;
din.message = "Draft comment";
gApi.changes().id(id.get()).current().createDraft(din);
gApi.changes().id(id.get()).current().review(ReviewInput.approve());
gApi.changes().id(id.get()).current().createDraft(din);
assertThat(db.changeMessages().byChange(id)).isNotEmpty();
assertThat(db.patchSets().byChange(id)).isNotEmpty();
assertThat(db.patchSetApprovals().byChange(id)).isNotEmpty();
assertThat(db.patchComments().byChange(id)).isNotEmpty();
ChangeBundle noteDbBundle =
ChangeBundle.fromNotes(commentsUtil, notesFactory.create(db, project, id));
setNoteDbPrimary(id);
db.changeMessages().delete(db.changeMessages().byChange(id));
db.patchSets().delete(db.patchSets().byChange(id));
db.patchSetApprovals().delete(db.patchSetApprovals().byChange(id));
db.patchComments().delete(db.patchComments().byChange(id));
ChangeMessage bogusMessage =
ChangeMessagesUtil.newMessage(
c.currentPatchSetId(),
identifiedUserFactory.create(admin.getId()),
TimeUtil.nowTs(),
"some message",
null);
db.changeMessages().insert(Collections.singleton(bogusMessage));
rebuilderWrapper.rebuildReviewDb(db, project, id);
assertThat(db.changeMessages().byChange(id)).isNotEmpty();
assertThat(db.patchSets().byChange(id)).isNotEmpty();
assertThat(db.patchSetApprovals().byChange(id)).isNotEmpty();
assertThat(db.patchComments().byChange(id)).isNotEmpty();
ChangeBundle reviewDbBundle = bundleReader.fromReviewDb(ReviewDbUtil.unwrapDb(db), id);
assertThat(reviewDbBundle.differencesFrom(noteDbBundle)).isEmpty();
}
@Test
public void rebuildReviewDbRequiresNoteDbPrimary() throws Exception {
Change.Id id = createChange().getChange().getId();
exception.expect(OrmException.class);
exception.expectMessage("primary storage of " + id + " is REVIEW_DB");
rebuilderWrapper.rebuildReviewDb(db, project, id);
}
@Test
public void migrateBackToReviewDbPrimary() throws Exception {
Change c = createChange().getChange().change();
Change.Id id = c.getId();
migrator.migrateToNoteDbPrimary(id);
assertNoteDbPrimary(id);
gApi.changes().id(id.get()).topic("new-topic");
assertThat(gApi.changes().id(id.get()).topic()).isEqualTo("new-topic");
assertThat(db.changes().get(id).getTopic()).isNotEqualTo("new-topic");
migrator.migrateToReviewDbPrimary(id, null);
ObjectId metaId;
try (Repository repo = repoManager.openRepository(c.getProject());
RevWalk rw = new RevWalk(repo)) {
metaId = repo.exactRef(RefNames.changeMetaRef(id)).getObjectId();
RevCommit commit = rw.parseCommit(metaId);
rw.parseBody(commit);
assertThat(commit.getFullMessage())
.contains("Read-only-until: " + formatTime(serverIdent.get(), new Timestamp(0)));
}
NoteDbChangeState state = NoteDbChangeState.parse(db.changes().get(id));
assertThat(state.getPrimaryStorage()).isEqualTo(PrimaryStorage.REVIEW_DB);
assertThat(state.getChangeMetaId()).isEqualTo(metaId);
assertThat(gApi.changes().id(id.get()).topic()).isEqualTo("new-topic");
assertThat(db.changes().get(id).getTopic()).isEqualTo("new-topic");
ChangeNotes notes = notesFactory.create(db, project, id);
assertThat(notes.getRevision()).isEqualTo(metaId); // No rebuilding, change was up to date.
assertThat(notes.getReadOnlyUntil()).isNotNull();
gApi.changes().id(id.get()).topic("reviewdb-topic");
assertThat(db.changes().get(id).getTopic()).isEqualTo("reviewdb-topic");
}
private void setNoteDbPrimary(Change.Id id) throws Exception {
Change c = db.changes().get(id);
assertThat(c).named("change " + id).isNotNull();

30
gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
View File

@@ -29,8 +29,10 @@ import com.google.gerrit.server.InternalUser;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gwtorm.server.OrmException;
import java.io.IOException;
import java.sql.Timestamp;
import java.util.Date;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
@@ -47,15 +49,17 @@ public abstract class AbstractChangeUpdate {
protected final Account.Id realAccountId;
protected final PersonIdent authorIdent;
protected final Date when;
private final long readOnlySkewMs;
@Nullable private final ChangeNotes notes;
private final Change change;
private final PersonIdent serverIdent;
protected final PersonIdent serverIdent;
protected PatchSet.Id psId;
private ObjectId result;
protected AbstractChangeUpdate(
Config cfg,
NotesMigration migration,
ChangeControl ctl,
PersonIdent serverIdent,
@@ -73,9 +77,11 @@ public abstract class AbstractChangeUpdate {
this.realAccountId = realAccountId != null ? realAccountId : accountId;
this.authorIdent = ident(noteUtil, serverIdent, anonymousCowardName, ctl.getUser(), when);
this.when = when;
this.readOnlySkewMs = NoteDbChangeState.getReadOnlySkew(cfg);
}
protected AbstractChangeUpdate(
Config cfg,
NotesMigration migration,
ChangeNoteUtil noteUtil,
PersonIdent serverIdent,
@@ -99,6 +105,7 @@ public abstract class AbstractChangeUpdate {
this.realAccountId = realAccountId;
this.authorIdent = authorIdent;
this.when = when;
this.readOnlySkewMs = NoteDbChangeState.getReadOnlySkew(cfg);
}
private static void checkUserType(CurrentUser user) {
@@ -133,6 +140,14 @@ public abstract class AbstractChangeUpdate {
return change.getId();
}
/**
* @return notes for the state of this change prior to this update. If this update is part of a
* series managed by a {@link NoteDbUpdateManager}, then this reflects the state prior to the
* first update in the series. A null return value can only happen when the change is being
* rebuilt from NoteDb. A change that is in the process of being created will result in a
* non-null return value from this method, but a null return value from {@link
* ChangeNotes#getRevision()}.
*/
@Nullable
public ChangeNotes getNotes() {
return notes;
@@ -206,6 +221,7 @@ public abstract class AbstractChangeUpdate {
// to actually store.
checkArgument(rw.getObjectReader().getCreatedFromInserter() == ins);
checkNotReadOnly();
ObjectId z = ObjectId.zeroId();
CommitBuilder cb = applyImpl(rw, ins, curr);
if (cb == null) {
@@ -233,6 +249,18 @@ public abstract class AbstractChangeUpdate {
return result;
}
protected void checkNotReadOnly() throws OrmException {
ChangeNotes notes = getNotes();
if (notes == null) {
// Can only happen during ChangeRebuilder, which will never include a read-only lease.
return;
}
Timestamp until = notes.getReadOnlyUntil();
if (until != null && NoteDbChangeState.timeForReadOnlyCheck(readOnlySkewMs).before(until)) {
throw new OrmException("change " + notes.getChangeId() + " is read-only until " + until);
}
}
/**
* Create a commit containing the contents of this update.
*

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

@@ -29,6 +29,7 @@ import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gwtorm.server.OrmException;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
@@ -42,6 +43,7 @@ import java.util.Map;
import java.util.Set;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.PersonIdent;
@@ -91,6 +93,7 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
@AssistedInject
private ChangeDraftUpdate(
@GerritServerConfig Config cfg,
@GerritPersonIdent PersonIdent serverIdent,
@AnonymousCowardName String anonymousCowardName,
NotesMigration migration,
@@ -102,6 +105,7 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
@Assisted PersonIdent authorIdent,
@Assisted Date when) {
super(
cfg,
migration,
noteUtil,
serverIdent,
@@ -117,6 +121,7 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
@AssistedInject
private ChangeDraftUpdate(
@GerritServerConfig Config cfg,
@GerritPersonIdent PersonIdent serverIdent,
@AnonymousCowardName String anonymousCowardName,
NotesMigration migration,
@@ -128,6 +133,7 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
@Assisted PersonIdent authorIdent,
@Assisted Date when) {
super(
cfg,
migration,
noteUtil,
serverIdent,

1
gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
View File

@@ -73,6 +73,7 @@ public class ChangeNoteUtil {
public static final FooterKey FOOTER_PATCH_SET = new FooterKey("Patch-set");
public static final FooterKey FOOTER_PATCH_SET_DESCRIPTION =
new FooterKey("Patch-set-description");
public static final FooterKey FOOTER_READ_ONLY_UNTIL = new FooterKey("Read-only-until");
public static final FooterKey FOOTER_REAL_USER = new FooterKey("Real-user");
public static final FooterKey FOOTER_STATUS = new FooterKey("Status");
public static final FooterKey FOOTER_SUBJECT = new FooterKey("Subject");

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

@@ -63,6 +63,7 @@ import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
@@ -558,6 +559,11 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
return checkNotNull(getPatchSets().get(psId), "missing current patch set %s", psId.get());
}
@VisibleForTesting
public Timestamp getReadOnlyUntil() {
return state.readOnlyUntil();
}
@Override
protected void onLoad(LoadHandle handle)
throws NoSuchChangeException, IOException, ConfigInvalidException {

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

@@ -24,6 +24,7 @@ import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_HASHTAGS;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_LABEL;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_PATCH_SET;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_PATCH_SET_DESCRIPTION;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_READ_ONLY_UNTIL;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_REAL_USER;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_STATUS;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_SUBJECT;
@@ -68,6 +69,7 @@ import com.google.gerrit.server.util.LabelVote;
import java.io.IOException;
import java.nio.charset.Charset;
import java.sql.Timestamp;
import java.text.ParseException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
@@ -76,6 +78,7 @@ import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
@@ -89,6 +92,7 @@ import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.notes.NoteMap;
import org.eclipse.jgit.revwalk.FooterKey;
import org.eclipse.jgit.util.GitDateParser;
import org.eclipse.jgit.util.RawParseUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -152,6 +156,7 @@ class ChangeNotesParser {
private String submissionId;
private String tag;
private RevisionNoteMap<ChangeRevisionNote> revisionNoteMap;
private Timestamp readOnlyUntil;
ChangeNotesParser(
Change.Id changeId,
@@ -232,7 +237,8 @@ class ChangeNotesParser {
submitRecords,
buildAllMessages(),
buildMessagesByPatchSet(),
comments);
comments,
readOnlyUntil);
}
private PatchSet.Id buildCurrentPatchSetId() {
@@ -369,6 +375,10 @@ class ChangeNotesParser {
// behavior.
}
if (readOnlyUntil == null) {
parseReadOnlyUntil(commit);
}
if (lastUpdatedOn == null || ts.after(lastUpdatedOn)) {
lastUpdatedOn = ts;
}
@@ -900,6 +910,20 @@ class ChangeNotesParser {
}
}
private void parseReadOnlyUntil(ChangeNotesCommit commit) throws ConfigInvalidException {
String raw = parseOneFooter(commit, FOOTER_READ_ONLY_UNTIL);
if (raw == null) {
return;
}
try {
readOnlyUntil = new Timestamp(GitDateParser.parse(raw, null, Locale.US).getTime());
} catch (ParseException e) {
ConfigInvalidException cie = invalidFooter(FOOTER_READ_ONLY_UNTIL, raw);
cie.initCause(e);
throw cie;
}
}
private void pruneReviewers() {
Iterator<Table.Cell<Account.Id, ReviewerStateInternal, Timestamp>> rit =
reviewers.cellSet().iterator();

12
gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesState.java
View File

@@ -70,7 +70,8 @@ public abstract class ChangeNotesState {
ImmutableList.of(),
ImmutableList.of(),
ImmutableListMultimap.of(),
ImmutableListMultimap.of());
ImmutableListMultimap.of(),
null);
}
static ChangeNotesState create(
@@ -98,7 +99,8 @@ public abstract class ChangeNotesState {
List<SubmitRecord> submitRecords,
List<ChangeMessage> allChangeMessages,
ListMultimap<PatchSet.Id, ChangeMessage> changeMessagesByPatchSet,
ListMultimap<RevId, Comment> publishedComments) {
ListMultimap<RevId, Comment> publishedComments,
@Nullable Timestamp readOnlyUntil) {
if (hashtags == null) {
hashtags = ImmutableSet.of();
}
@@ -128,7 +130,8 @@ public abstract class ChangeNotesState {
ImmutableList.copyOf(submitRecords),
ImmutableList.copyOf(allChangeMessages),
ImmutableListMultimap.copyOf(changeMessagesByPatchSet),
ImmutableListMultimap.copyOf(publishedComments));
ImmutableListMultimap.copyOf(publishedComments),
readOnlyUntil);
}
/**
@@ -206,6 +209,9 @@ public abstract class ChangeNotesState {
abstract ImmutableListMultimap<RevId, Comment> publishedComments();
@Nullable
abstract Timestamp readOnlyUntil();
Change newChange(Project.NameKey project) {
ChangeColumns c = checkNotNull(columns(), "columns are required");
Change change =

34
gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
View File

@@ -29,6 +29,7 @@ import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_HASHTAGS;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_LABEL;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_PATCH_SET;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_PATCH_SET_DESCRIPTION;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_READ_ONLY_UNTIL;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_REAL_USER;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_STATUS;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_SUBJECT;
@@ -58,6 +59,7 @@ import com.google.gerrit.reviewdb.client.RobotComment;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.util.LabelVote;
@@ -67,6 +69,7 @@ import com.google.gwtorm.server.OrmException;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
import java.io.IOException;
import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.Date;
@@ -79,6 +82,7 @@ import java.util.Optional;
import java.util.Set;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.PersonIdent;
@@ -144,12 +148,14 @@ public class ChangeUpdate extends AbstractChangeUpdate {
private boolean isAllowWriteToNewtRef;
private String psDescription;
private boolean currentPatchSet;
private Timestamp readOnlyUntil;
private ChangeDraftUpdate draftUpdate;
private RobotCommentUpdate robotCommentUpdate;
@AssistedInject
private ChangeUpdate(
@GerritServerConfig Config cfg,
@GerritPersonIdent PersonIdent serverIdent,
@AnonymousCowardName String anonymousCowardName,
NotesMigration migration,
@@ -161,6 +167,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
@Assisted ChangeControl ctl,
ChangeNoteUtil noteUtil) {
this(
cfg,
serverIdent,
anonymousCowardName,
migration,
@@ -176,6 +183,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
@AssistedInject
private ChangeUpdate(
@GerritServerConfig Config cfg,
@GerritPersonIdent PersonIdent serverIdent,
@AnonymousCowardName String anonymousCowardName,
NotesMigration migration,
@@ -188,6 +196,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
@Assisted Date when,
ChangeNoteUtil noteUtil) {
this(
cfg,
serverIdent,
anonymousCowardName,
migration,
@@ -212,6 +221,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
@AssistedInject
private ChangeUpdate(
@GerritServerConfig Config cfg,
@GerritPersonIdent PersonIdent serverIdent,
@AnonymousCowardName String anonymousCowardName,
NotesMigration migration,
@@ -223,7 +233,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
@Assisted Date when,
@Assisted Comparator<String> labelNameComparator,
ChangeNoteUtil noteUtil) {
super(migration, ctl, serverIdent, anonymousCowardName, noteUtil, when);
super(cfg, migration, ctl, serverIdent, anonymousCowardName, noteUtil, when);
this.accountCache = accountCache;
this.draftUpdateFactory = draftUpdateFactory;
this.robotCommentUpdateFactory = robotCommentUpdateFactory;
@@ -233,6 +243,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
@AssistedInject
private ChangeUpdate(
@GerritServerConfig Config cfg,
@GerritPersonIdent PersonIdent serverIdent,
@AnonymousCowardName String anonymousCowardName,
NotesMigration migration,
@@ -248,6 +259,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
@Assisted Date when,
@Assisted Comparator<String> labelNameComparator) {
super(
cfg,
migration,
noteUtil,
serverIdent,
@@ -695,6 +707,10 @@ public class ChangeUpdate extends AbstractChangeUpdate {
addIdent(msg, realAccountId).append('\n');
}
if (readOnlyUntil != null) {
addFooter(msg, FOOTER_READ_ONLY_UNTIL, ChangeNoteUtil.formatTime(serverIdent, readOnlyUntil));
}
cb.setMessage(msg.toString());
try {
ObjectId treeId = storeRevisionNotes(rw, ins, curr);
@@ -740,7 +756,8 @@ public class ChangeUpdate extends AbstractChangeUpdate {
&& groups == null
&& tag == null
&& psDescription == null
&& !currentPatchSet;
&& !currentPatchSet
&& readOnlyUntil == null;
}
ChangeDraftUpdate getDraftUpdate() {
@@ -760,6 +777,10 @@ public class ChangeUpdate extends AbstractChangeUpdate {
return isAllowWriteToNewtRef;
}
void setReadOnlyUntil(Timestamp readOnlyUntil) {
this.readOnlyUntil = readOnlyUntil;
}
private static StringBuilder addFooter(StringBuilder sb, FooterKey footer) {
return sb.append(footer.getName()).append(": ");
}
@@ -782,4 +803,13 @@ public class ChangeUpdate extends AbstractChangeUpdate {
sb.append('>');
return sb;
}
@Override
protected void checkNotReadOnly() throws OrmException {
// Allow setting Read-only-until to 0 to release an existing lease.
if (readOnlyUntil != null && readOnlyUntil.getTime() == 0) {
return;
}
super.checkNotReadOnly();
}
}

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

@@ -301,7 +301,7 @@ public class NoteDbChangeState {
return cfg.getTimeUnit("notedb", null, "maxTimestampSkew", 1000, TimeUnit.MILLISECONDS);
}
private static Timestamp timeForReadOnlyCheck(long skewMs) {
static Timestamp timeForReadOnlyCheck(long skewMs) {
// Subtract some slop in case the machine that set the change's read-only
// lease has a clock behind ours.
return new Timestamp(TimeUtil.nowMs() - skewMs);

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

@@ -19,6 +19,7 @@ import com.google.common.cache.CacheBuilder;
import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Change.Id;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.notedb.NoteDbUpdateManager.Result;
import com.google.gerrit.server.notedb.rebuild.ChangeRebuilder;
@@ -95,6 +96,11 @@ public class NoteDbModule extends FactoryModule {
public void buildUpdates(NoteDbUpdateManager manager, ChangeBundle bundle) {
// Do nothing.
}
@Override
public void rebuildReviewDb(ReviewDb db, Project.NameKey project, Id changeId) {
// Do nothing.
}
});
bind(new TypeLiteral<Cache<ChangeNotesCache.Key, ChangeNotesState>>() {})
.annotatedWith(Names.named(ChangeNotesCache.CACHE_NAME))

185
gerrit-server/src/main/java/com/google/gerrit/server/notedb/PrimaryStorageMigrator.java
View File

@@ -26,18 +26,32 @@ import com.github.rholder.retry.StopStrategies;
import com.github.rholder.retry.WaitStrategies;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Stopwatch;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.reviewdb.server.ReviewDbUtil;
import com.google.gerrit.server.InternalUser;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.BatchUpdate;
import com.google.gerrit.server.git.BatchUpdate.ChangeContext;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.RepoRefCache;
import com.google.gerrit.server.git.UpdateException;
import com.google.gerrit.server.index.change.ChangeField;
import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.notedb.NoteDbChangeState.RefState;
import com.google.gerrit.server.notedb.rebuild.ChangeRebuilder;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.OrmRuntimeException;
@@ -46,11 +60,17 @@ import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.sql.Timestamp;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.TreeSet;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.atomic.AtomicBoolean;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -60,10 +80,15 @@ import org.slf4j.LoggerFactory;
public class PrimaryStorageMigrator {
private static final Logger log = LoggerFactory.getLogger(PrimaryStorageMigrator.class);
private final Provider<ReviewDb> db;
private final GitRepositoryManager repoManager;
private final AllUsersName allUsers;
private final BatchUpdate.Factory batchUpdateFactory;
private final ChangeControl.GenericFactory changeControlFactory;
private final ChangeRebuilder rebuilder;
private final ChangeUpdate.Factory updateFactory;
private final GitRepositoryManager repoManager;
private final InternalUser.Factory internalUserFactory;
private final Provider<InternalChangeQuery> queryProvider;
private final Provider<ReviewDb> db;
private final long skewMs;
private final long timeoutMs;
@@ -75,8 +100,24 @@ public class PrimaryStorageMigrator {
Provider<ReviewDb> db,
GitRepositoryManager repoManager,
AllUsersName allUsers,
ChangeRebuilder rebuilder) {
this(cfg, db, repoManager, allUsers, rebuilder, null);
ChangeRebuilder rebuilder,
ChangeControl.GenericFactory changeControlFactory,
Provider<InternalChangeQuery> queryProvider,
ChangeUpdate.Factory updateFactory,
InternalUser.Factory internalUserFactory,
BatchUpdate.Factory batchUpdateFactory) {
this(
cfg,
db,
repoManager,
allUsers,
rebuilder,
null,
changeControlFactory,
queryProvider,
updateFactory,
internalUserFactory,
batchUpdateFactory);
}
@VisibleForTesting
@@ -86,12 +127,22 @@ public class PrimaryStorageMigrator {
GitRepositoryManager repoManager,
AllUsersName allUsers,
ChangeRebuilder rebuilder,
@Nullable Retryer<NoteDbChangeState> testEnsureRebuiltRetryer) {
@Nullable Retryer<NoteDbChangeState> testEnsureRebuiltRetryer,
ChangeControl.GenericFactory changeControlFactory,
Provider<InternalChangeQuery> queryProvider,
ChangeUpdate.Factory updateFactory,
InternalUser.Factory internalUserFactory,
BatchUpdate.Factory batchUpdateFactory) {
this.db = db;
this.repoManager = repoManager;
this.allUsers = allUsers;
this.rebuilder = rebuilder;
this.testEnsureRebuiltRetryer = testEnsureRebuiltRetryer;
this.changeControlFactory = changeControlFactory;
this.queryProvider = queryProvider;
this.updateFactory = updateFactory;
this.internalUserFactory = internalUserFactory;
this.batchUpdateFactory = batchUpdateFactory;
skewMs = NoteDbChangeState.getReadOnlySkew(cfg);
String s = "notedb";
@@ -189,7 +240,7 @@ public class PrimaryStorageMigrator {
// failure.
Stopwatch sw = Stopwatch.createStarted();
Change readOnlyChange = setReadOnly(id); // MRO
Change readOnlyChange = setReadOnlyInReviewDb(id); // MRO
if (readOnlyChange == null) {
return; // Already migrated.
}
@@ -217,7 +268,7 @@ public class PrimaryStorageMigrator {
log.info("Migrated change {} to NoteDb primary in {}ms", id, sw.elapsed(MILLISECONDS));
}
private Change setReadOnly(Change.Id id) throws OrmException {
private Change setReadOnlyInReviewDb(Change.Id id) throws OrmException {
AtomicBoolean alreadyMigrated = new AtomicBoolean(false);
Change result =
db().changes()
@@ -316,4 +367,124 @@ public class PrimaryStorageMigrator {
private String badState(NoteDbChangeState actual, NoteDbChangeState expected) {
return "state changed unexpectedly: " + actual + " != " + expected;
}
public void migrateToReviewDbPrimary(Change.Id id, @Nullable Project.NameKey project)
throws OrmException, IOException {
// Migrating back to ReviewDb primary is much simpler than the original migration to NoteDb
// primary, because when NoteDb is primary, each write only goes to one storage location rather
// than both. We only need to consider whether a concurrent writer (OR) conflicts with the first
// setReadOnlyInNoteDb step (MR) in this method.
//
// If OR wins, then either:
// * MR will set read-only after OR is completed, which is not a concurrent write.
// * MR will fail to set read-only with a lock failure. The caller will have to retry, but the
// change is not in a read-only state, so behavior is not degraded in the meantime.
//
// If MR wins, then either:
// * OR will fail with a read-only exception (via AbstractChangeNotes#apply).
// * OR will fail with a lock failure.
//
// In all of these scenarios, the change is read-only if and only if MR succeeds.
//
// There will be no concurrent writes to ReviewDb for this change until
// setPrimaryStorageReviewDb completes, because ReviewDb writes are not attempted when primary
// storage is NoteDb. After the primary storage changes back, it is possible for subsequent
// NoteDb writes to conflict with the releaseReadOnlyLeaseInNoteDb step, but at this point,
// since ReviewDb is primary, we are back to ignoring them.
Stopwatch sw = Stopwatch.createStarted();
if (project == null) {
project = getProject(id);
}
ObjectId newMetaId = setReadOnlyInNoteDb(project, id);
rebuilder.rebuildReviewDb(db(), project, id);
setPrimaryStorageReviewDb(id, newMetaId);
releaseReadOnlyLeaseInNoteDb(project, id);
log.info("Migrated change {} to ReviewDb primary in {}ms", id, sw.elapsed(MILLISECONDS));
}
private ObjectId setReadOnlyInNoteDb(Project.NameKey project, Change.Id id)
throws OrmException, IOException {
Timestamp now = TimeUtil.nowTs();
Timestamp until = new Timestamp(now.getTime() + timeoutMs);
ChangeUpdate update =
updateFactory.create(
changeControlFactory.controlFor(db.get(), project, id, internalUserFactory.create()));
update.setReadOnlyUntil(until);
return update.commit();
}
private void setPrimaryStorageReviewDb(Change.Id id, ObjectId newMetaId)
throws OrmException, IOException {
ImmutableMap.Builder<Account.Id, ObjectId> draftIds = ImmutableMap.builder();
try (Repository repo = repoManager.openRepository(allUsers)) {
for (Ref draftRef :
repo.getRefDatabase().getRefs(RefNames.refsDraftCommentsPrefix(id)).values()) {
Account.Id accountId = Account.Id.fromRef(draftRef.getName());
if (accountId != null) {
draftIds.put(accountId, draftRef.getObjectId().copy());
}
}
}
NoteDbChangeState newState =
new NoteDbChangeState(
id,
PrimaryStorage.REVIEW_DB,
Optional.of(RefState.create(newMetaId, draftIds.build())),
Optional.empty());
db().changes()
.atomicUpdate(
id,
new AtomicUpdate<Change>() {
@Override
public Change update(Change change) {
if (PrimaryStorage.of(change) != PrimaryStorage.NOTE_DB) {
throw new OrmRuntimeException(
"change " + id + " is not NoteDb primary: " + change.getNoteDbState());
}
change.setNoteDbState(newState.toString());
return change;
}
});
}
private void releaseReadOnlyLeaseInNoteDb(Project.NameKey project, Change.Id id)
throws OrmException {
// Use a BatchUpdate since ReviewDb is primary at this point, so it needs to reflect the update.
try (BatchUpdate bu =
batchUpdateFactory.create(
db.get(), project, internalUserFactory.create(), TimeUtil.nowTs())) {
bu.addOp(
id,
new BatchUpdate.Op() {
@Override
public boolean updateChange(ChangeContext ctx) {
ctx.getUpdate(ctx.getChange().currentPatchSetId()).setReadOnlyUntil(new Timestamp(0));
return true;
}
});
bu.execute();
} catch (RestApiException | UpdateException e) {
throw new OrmException(e);
}
}
private Project.NameKey getProject(Change.Id id) throws OrmException {
List<ChangeData> cds =
queryProvider
.get()
.setRequestedFields(ImmutableSet.of(ChangeField.PROJECT.getName()))
.byLegacyChangeId(id);
Set<Project.NameKey> projects = new TreeSet<>();
for (ChangeData cd : cds) {
projects.add(cd.project());
}
if (projects.size() != 1) {
throw new OrmException(
"zero or multiple projects found for change "
+ id
+ ", must specify project explicitly: "
+ projects);
}
return projects.iterator().next();
}
}

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

@@ -26,6 +26,7 @@ import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.client.RobotComment;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gwtorm.server.OrmException;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
@@ -38,6 +39,7 @@ import java.util.Map;
import java.util.Set;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.PersonIdent;
@@ -73,6 +75,7 @@ public class RobotCommentUpdate extends AbstractChangeUpdate {
@AssistedInject
private RobotCommentUpdate(
@GerritServerConfig Config cfg,
@GerritPersonIdent PersonIdent serverIdent,
@AnonymousCowardName String anonymousCowardName,
NotesMigration migration,
@@ -83,6 +86,7 @@ public class RobotCommentUpdate extends AbstractChangeUpdate {
@Assisted PersonIdent authorIdent,
@Assisted Date when) {
super(
cfg,
migration,
noteUtil,
serverIdent,
@@ -97,6 +101,7 @@ public class RobotCommentUpdate extends AbstractChangeUpdate {
@AssistedInject
private RobotCommentUpdate(
@GerritServerConfig Config cfg,
@GerritPersonIdent PersonIdent serverIdent,
@AnonymousCowardName String anonymousCowardName,
NotesMigration migration,
@@ -107,6 +112,7 @@ public class RobotCommentUpdate extends AbstractChangeUpdate {
@Assisted PersonIdent authorIdent,
@Assisted Date when) {
super(
cfg,
migration,
noteUtil,
serverIdent,

11
gerrit-server/src/main/java/com/google/gerrit/server/notedb/TestChangeRebuilderWrapper.java
View File

@@ -16,6 +16,8 @@ package com.google.gerrit.server.notedb;
import com.google.common.annotations.VisibleForTesting;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Change.Id;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.notedb.NoteDbUpdateManager.Result;
import com.google.gerrit.server.notedb.rebuild.ChangeRebuilder;
@@ -111,4 +113,13 @@ public class TestChangeRebuilderWrapper extends ChangeRebuilder {
// Don't check for manual failure; that happens in execute().
delegate.buildUpdates(manager, bundle);
}
@Override
public void rebuildReviewDb(ReviewDb db, Project.NameKey project, Id changeId)
throws OrmException {
if (failNextUpdate.getAndSet(false)) {
throw new OrmException("Update failed");
}
delegate.rebuildReviewDb(db, project, changeId);
}
}

12
gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilder.java
View File

@@ -17,6 +17,7 @@ package com.google.gerrit.server.notedb.rebuild;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.notedb.ChangeBundle;
import com.google.gerrit.server.notedb.NoteDbUpdateManager;
@@ -54,6 +55,17 @@ public abstract class ChangeRebuilder {
});
}
/**
* Rebuild ReviewDb contents by copying from NoteDb.
*
* <p>Requires NoteDb to be the primary storage for the change.
*/
public abstract void rebuildReviewDb(ReviewDb db, Project.NameKey project, Change.Id changeId)
throws OrmException;
// In the following methods "rebuilding" always refers to copying the state
// from ReviewDb to NoteDb, i.e. assuming ReviewDb is the primary storage.
public abstract Result rebuild(ReviewDb db, Change.Id changeId) throws IOException, OrmException;
public abstract Result rebuildEvenIfReadOnly(ReviewDb db, Change.Id changeId)

52
gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java
View File

@@ -23,6 +23,7 @@ import static java.util.concurrent.TimeUnit.SECONDS;
import static java.util.stream.Collectors.toList;
import com.google.common.base.Splitter;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ListMultimap;
@@ -41,6 +42,7 @@ import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.client.PatchLineComment.Status;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.reviewdb.server.ReviewDbUtil;
@@ -67,6 +69,8 @@ import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gwtorm.client.Key;
import com.google.gwtorm.server.Access;
import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.SchemaFactory;
@@ -74,6 +78,7 @@ import com.google.inject.Inject;
import java.io.IOException;
import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.Iterator;
@@ -112,7 +117,9 @@ public class ChangeRebuilderImpl extends ChangeRebuilder {
private final ChangeBundleReader bundleReader;
private final ChangeDraftUpdate.Factory draftUpdateFactory;
private final ChangeNoteUtil changeNoteUtil;
private final ChangeNotes.Factory notesFactory;
private final ChangeUpdate.Factory updateFactory;
private final CommentsUtil commentsUtil;
private final NoteDbUpdateManager.Factory updateManagerFactory;
private final NotesMigration migration;
private final PatchListCache patchListCache;
@@ -130,7 +137,9 @@ public class ChangeRebuilderImpl extends ChangeRebuilder {
ChangeBundleReader bundleReader,
ChangeDraftUpdate.Factory draftUpdateFactory,
ChangeNoteUtil changeNoteUtil,
ChangeNotes.Factory notesFactory,
ChangeUpdate.Factory updateFactory,
CommentsUtil commentsUtil,
NoteDbUpdateManager.Factory updateManagerFactory,
NotesMigration migration,
PatchListCache patchListCache,
@@ -143,7 +152,9 @@ public class ChangeRebuilderImpl extends ChangeRebuilder {
this.bundleReader = bundleReader;
this.draftUpdateFactory = draftUpdateFactory;
this.changeNoteUtil = changeNoteUtil;
this.notesFactory = notesFactory;
this.updateFactory = updateFactory;
this.commentsUtil = commentsUtil;
this.updateManagerFactory = updateManagerFactory;
this.migration = migration;
this.patchListCache = patchListCache;
@@ -613,4 +624,45 @@ public class ChangeRebuilderImpl extends ChangeRebuilder {
update.setBranch(change.getDest().get());
update.setSubject(change.getOriginalSubject());
}
@Override
public void rebuildReviewDb(ReviewDb db, Project.NameKey project, Change.Id changeId)
throws OrmException {
// TODO(dborowitz): Fail fast if changes tables are disabled in ReviewDb.
ChangeNotes notes = notesFactory.create(db, project, changeId);
ChangeBundle bundle = ChangeBundle.fromNotes(commentsUtil, notes);
db = ReviewDbUtil.unwrapDb(db);
db.changes().beginTransaction(changeId);
try {
Change c = db.changes().get(changeId);
PrimaryStorage ps = PrimaryStorage.of(c);
if (ps != PrimaryStorage.NOTE_DB) {
throw new OrmException("primary storage of " + changeId + " is " + ps);
}
db.changes().upsert(Collections.singleton(c));
putExactlyEntities(
db.changeMessages(), db.changeMessages().byChange(c.getId()), bundle.getChangeMessages());
putExactlyEntities(db.patchSets(), db.patchSets().byChange(c.getId()), bundle.getPatchSets());
putExactlyEntities(
db.patchSetApprovals(),
db.patchSetApprovals().byChange(c.getId()),
bundle.getPatchSetApprovals());
putExactlyEntities(
db.patchComments(),
db.patchComments().byChange(c.getId()),
bundle.getPatchLineComments());
db.commit();
} finally {
db.rollback();
}
}
private static <T, K extends Key<?>> void putExactlyEntities(
Access<T, K> access, Iterable<T> existing, Collection<T> ents) throws OrmException {
Set<K> toKeep = access.toMap(ents).keySet();
access.delete(
FluentIterable.from(existing).filter(e -> !toKeep.contains(access.primaryKey(e))));
access.upsert(ents);
}
}

73
gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
View File

@@ -15,6 +15,7 @@
package com.google.gerrit.server.notedb;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assert_;
import static com.google.gerrit.reviewdb.client.RefNames.changeMetaRef;
import static com.google.gerrit.reviewdb.client.RefNames.refsDraftComments;
import static com.google.gerrit.server.notedb.ReviewerStateInternal.CC;
@@ -51,6 +52,7 @@ import com.google.gerrit.server.config.GerritServerId;
import com.google.gerrit.server.notedb.ChangeNotesCommit.ChangeNotesRevWalk;
import com.google.gerrit.server.util.RequestId;
import com.google.gerrit.testutil.TestChanges;
import com.google.gerrit.testutil.TestTimeUtil;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import java.sql.Timestamp;
@@ -58,6 +60,7 @@ import java.util.ArrayList;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
@@ -3193,6 +3196,76 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
assertThat(newNotes(c).getChange().currentPatchSetId().get()).isEqualTo(2);
}
@Test
public void readOnlyUntilExpires() throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
Timestamp until = new Timestamp(TimeUtil.nowMs() + 10000);
update.setReadOnlyUntil(until);
update.commit();
update = newUpdate(c, changeOwner);
update.setTopic("failing-topic");
try {
update.commit();
assert_().fail("expected OrmException");
} catch (OrmException e) {
assertThat(e.getMessage()).contains("read-only until");
}
ChangeNotes notes = newNotes(c);
assertThat(notes.getChange().getTopic()).isNotEqualTo("failing-topic");
assertThat(notes.getReadOnlyUntil()).isEqualTo(until);
TestTimeUtil.incrementClock(30, TimeUnit.SECONDS);
update = newUpdate(c, changeOwner);
update.setTopic("succeeding-topic");
update.commit();
// Write succeeded; lease still exists, even though it's expired.
notes = newNotes(c);
assertThat(notes.getChange().getTopic()).isEqualTo("succeeding-topic");
assertThat(notes.getReadOnlyUntil()).isEqualTo(until);
// New lease takes precedence.
update = newUpdate(c, changeOwner);
until = new Timestamp(TimeUtil.nowMs() + 10000);
update.setReadOnlyUntil(until);
update.commit();
assertThat(newNotes(c).getReadOnlyUntil()).isEqualTo(until);
}
@Test
public void readOnlyUntilCleared() throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
Timestamp until = new Timestamp(TimeUtil.nowMs() + TimeUnit.DAYS.toMillis(30));
update.setReadOnlyUntil(until);
update.commit();
update = newUpdate(c, changeOwner);
update.setTopic("failing-topic");
try {
update.commit();
assert_().fail("expected OrmException");
} catch (OrmException e) {
assertThat(e.getMessage()).contains("read-only until");
}
// Sentinel timestamp of 0 can be written to clear lease.
update = newUpdate(c, changeOwner);
update.setReadOnlyUntil(new Timestamp(0));
update.commit();
update = newUpdate(c, changeOwner);
update.setTopic("succeeding-topic");
update.commit();
ChangeNotes notes = newNotes(c);
assertThat(notes.getChange().getTopic()).isEqualTo("succeeding-topic");
assertThat(notes.getReadOnlyUntil()).isEqualTo(new Timestamp(0));
}
private boolean testJson() {
return noteUtil.getWriteJson();
}