Fuse code and meta ref updates in NoteDb
JGit now supports atomic multi-ref BatchRefUpdates natively in FileRepository, so we can eliminate the distinction between fused and unfused NoteDb modes. Now, disableChangeReviewDb=true means NoteDb is fully enabled. Change-Id: I085502687c5e9a1f426c55334fe8bf59fa36d9e8
This commit is contained in:
Dave Borowitz
parent
6866eef95d
commit
24b3e672a9
@ -16,7 +16,6 @@ package com.google.gerrit.acceptance;
|
||||
|
||||
import static com.google.common.base.Preconditions.checkArgument;
|
||||
import static com.google.common.base.Preconditions.checkNotNull;
|
||||
import static com.google.common.truth.TruthJUnit.assume;
|
||||
|
||||
import com.google.auto.value.AutoValue;
|
||||
import com.google.common.base.MoreObjects;
|
||||
@ -208,10 +207,6 @@ public class GerritServer implements AutoCloseable {
|
||||
*/
|
||||
public static void init(Description desc, Config baseConfig, Path site) throws Exception {
|
||||
checkArgument(!desc.memory(), "can't initialize site path for in-memory test: %s", desc);
|
||||
assume()
|
||||
.withMessage("FUSED mode not yet supported for on-disk sites")
|
||||
.that(NoteDbMode.get())
|
||||
.isNotEqualTo(NoteDbMode.FUSED);
|
||||
Config cfg = desc.buildConfig(baseConfig);
|
||||
Map<String, Config> pluginConfigs = desc.buildPluginConfigs();
|
||||
|
||||
|
||||
@ -748,7 +748,7 @@ public class SubmoduleSubscriptionsWholeTopicMergeIT extends AbstractSubmoduleSu
|
||||
|
||||
@Test
|
||||
public void retrySubmitAfterTornTopicOnLockFailure() throws Exception {
|
||||
assume().that(notesMigration.fuseUpdates()).isTrue();
|
||||
assume().that(notesMigration.disableChangeReviewDb()).isTrue();
|
||||
|
||||
TestRepository<?> superRepo = createProjectWithPush("super-project");
|
||||
TestRepository<?> sub1 = createProjectWithPush("sub1");
|
||||
|
||||
@ -105,7 +105,7 @@ public class StandaloneNoteDbMigrationIT extends StandaloneSiteTest {
|
||||
setUpOneChange();
|
||||
|
||||
migrate("--trial", "false");
|
||||
assertNotesMigrationState(NotesMigrationState.NOTE_DB_UNFUSED);
|
||||
assertNotesMigrationState(NotesMigrationState.NOTE_DB);
|
||||
|
||||
try (ServerContext ctx = startServer()) {
|
||||
GitRepositoryManager repoManager = ctx.getInjector().getInstance(GitRepositoryManager.class);
|
||||
@ -143,7 +143,7 @@ public class StandaloneNoteDbMigrationIT extends StandaloneSiteTest {
|
||||
assertServerStartupFails();
|
||||
|
||||
migrate("--trial", "false");
|
||||
assertNotesMigrationState(NotesMigrationState.NOTE_DB_UNFUSED);
|
||||
assertNotesMigrationState(NotesMigrationState.NOTE_DB);
|
||||
|
||||
status = new GerritIndexStatus(sitePaths);
|
||||
assertThat(status.getReady(ChangeSchemaDefinitions.NAME, version)).isTrue();
|
||||
@ -175,7 +175,7 @@ public class StandaloneNoteDbMigrationIT extends StandaloneSiteTest {
|
||||
u.runUpgrades();
|
||||
|
||||
assertThat(indexes.getSearchIndex().getSchema().getVersion()).isEqualTo(currVersion);
|
||||
assertNotesMigrationState(NotesMigrationState.NOTE_DB_UNFUSED);
|
||||
assertNotesMigrationState(NotesMigrationState.NOTE_DB);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@ -916,7 +916,7 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
|
||||
|
||||
@Test
|
||||
public void retrySubmitSingleChangeOnLockFailure() throws Exception {
|
||||
assume().that(notesMigration.fuseUpdates()).isTrue();
|
||||
assume().that(notesMigration.disableChangeReviewDb()).isTrue();
|
||||
|
||||
PushOneCommit.Result change = createChange();
|
||||
String id = change.getChangeId();
|
||||
@ -943,7 +943,7 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
|
||||
|
||||
@Test
|
||||
public void retrySubmitAfterTornTopicOnLockFailure() throws Exception {
|
||||
assume().that(notesMigration.fuseUpdates()).isTrue();
|
||||
assume().that(notesMigration.disableChangeReviewDb()).isTrue();
|
||||
assume().that(isSubmitWholeTopicEnabled()).isTrue();
|
||||
|
||||
String topic = "test-topic";
|
||||
|
||||
@ -28,7 +28,6 @@ import com.google.gerrit.common.TimeUtil;
|
||||
import com.google.gerrit.extensions.api.changes.ReviewInput;
|
||||
import com.google.gerrit.extensions.client.ListChangesOption;
|
||||
import com.google.gerrit.extensions.restapi.ResourceConflictException;
|
||||
import com.google.gerrit.extensions.restapi.RestApiException;
|
||||
import com.google.gerrit.reviewdb.client.Change;
|
||||
import com.google.gerrit.server.update.BatchUpdate;
|
||||
import com.google.gerrit.server.update.BatchUpdateListener;
|
||||
@ -77,7 +76,7 @@ public class NoteDbOnlyIT extends AbstractDaemonTest {
|
||||
|
||||
@Test
|
||||
public void updateChangeFailureRollsBackRefUpdate() throws Exception {
|
||||
assume().that(notesMigration.fuseUpdates()).isTrue();
|
||||
assume().that(notesMigration.disableChangeReviewDb()).isTrue();
|
||||
PushOneCommit.Result r = createChange();
|
||||
Change.Id id = r.getChange().getId();
|
||||
|
||||
@ -149,7 +148,7 @@ public class NoteDbOnlyIT extends AbstractDaemonTest {
|
||||
|
||||
@Test
|
||||
public void retryOnLockFailureWithAtomicUpdates() throws Exception {
|
||||
assume().that(notesMigration.fuseUpdates()).isTrue();
|
||||
assume().that(notesMigration.disableChangeReviewDb()).isTrue();
|
||||
PushOneCommit.Result r = createChange();
|
||||
Change.Id id = r.getChange().getId();
|
||||
String master = "refs/heads/master";
|
||||
@ -195,52 +194,6 @@ public class NoteDbOnlyIT extends AbstractDaemonTest {
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void noRetryOnLockFailureWithoutAtomicUpdates() throws Exception {
|
||||
assume().that(notesMigration.fuseUpdates()).isFalse();
|
||||
|
||||
PushOneCommit.Result r = createChange();
|
||||
Change.Id id = r.getChange().getId();
|
||||
String master = "refs/heads/master";
|
||||
ObjectId initial;
|
||||
try (Repository repo = repoManager.openRepository(project)) {
|
||||
initial = repo.exactRef(master).getObjectId();
|
||||
}
|
||||
|
||||
AtomicInteger updateRepoCalledCount = new AtomicInteger();
|
||||
AtomicInteger updateChangeCalledCount = new AtomicInteger();
|
||||
AtomicInteger afterUpdateReposCalledCount = new AtomicInteger();
|
||||
|
||||
try {
|
||||
retryHelper.execute(
|
||||
batchUpdateFactory -> {
|
||||
try (BatchUpdate bu = newBatchUpdate(batchUpdateFactory)) {
|
||||
bu.addOp(
|
||||
id, new UpdateRefAndAddMessageOp(updateRepoCalledCount, updateChangeCalledCount));
|
||||
bu.execute(new ConcurrentWritingListener(afterUpdateReposCalledCount));
|
||||
}
|
||||
return null;
|
||||
});
|
||||
assert_().fail("expected RestApiException");
|
||||
} catch (RestApiException e) {
|
||||
// Expected.
|
||||
}
|
||||
|
||||
assertThat(updateRepoCalledCount.get()).isEqualTo(1);
|
||||
assertThat(afterUpdateReposCalledCount.get()).isEqualTo(1);
|
||||
assertThat(updateChangeCalledCount.get()).isEqualTo(0);
|
||||
|
||||
// updateChange was never called, so no message was ever added.
|
||||
assertThat(getMessages(id)).doesNotContain(UpdateRefAndAddMessageOp.CHANGE_MESSAGE);
|
||||
|
||||
try (Repository repo = repoManager.openRepository(project)) {
|
||||
// Op lost the race, so the other writer's commit happened first. Op didn't retry, because the
|
||||
// ref updates weren't atomic, so it didn't throw LockFailureException on failure.
|
||||
assertThat(commitMessages(repo, initial, repo.exactRef(master).getObjectId()))
|
||||
.containsExactly(ConcurrentWritingListener.MSG_PREFIX + "1");
|
||||
}
|
||||
}
|
||||
|
||||
private class ConcurrentWritingListener implements BatchUpdateListener {
|
||||
static final String MSG_PREFIX = "Other writer ";
|
||||
|
||||
|
||||
@ -17,7 +17,7 @@ package com.google.gerrit.acceptance.server.notedb;
|
||||
import static com.google.common.truth.Truth.assertThat;
|
||||
import static com.google.common.truth.Truth8.assertThat;
|
||||
import static com.google.common.truth.TruthJUnit.assume;
|
||||
import static com.google.gerrit.server.notedb.NotesMigrationState.NOTE_DB_UNFUSED;
|
||||
import static com.google.gerrit.server.notedb.NotesMigrationState.NOTE_DB;
|
||||
import static com.google.gerrit.server.notedb.NotesMigrationState.READ_WRITE_NO_SEQUENCE;
|
||||
import static com.google.gerrit.server.notedb.NotesMigrationState.READ_WRITE_WITH_SEQUENCE_NOTE_DB_PRIMARY;
|
||||
import static com.google.gerrit.server.notedb.NotesMigrationState.READ_WRITE_WITH_SEQUENCE_REVIEW_DB_PRIMARY;
|
||||
@ -313,7 +313,7 @@ public class OnlineNoteDbMigrationIT extends AbstractDaemonTest {
|
||||
Change.Id id2 = r2.getChange().getId();
|
||||
|
||||
migrate(b -> b.setThreads(threads));
|
||||
assertNotesMigrationState(NOTE_DB_UNFUSED);
|
||||
assertNotesMigrationState(NOTE_DB);
|
||||
|
||||
assertThat(sequences.nextChangeId()).isEqualTo(503);
|
||||
|
||||
@ -372,7 +372,7 @@ public class OnlineNoteDbMigrationIT extends AbstractDaemonTest {
|
||||
assertThat(NoteDbMigrator.getAutoMigrate(gerritConfig)).isTrue();
|
||||
|
||||
migrate(b -> b);
|
||||
assertNotesMigrationState(NOTE_DB_UNFUSED);
|
||||
assertNotesMigrationState(NOTE_DB);
|
||||
assertThat(NoteDbMigrator.getAutoMigrate(gerritConfig)).isFalse();
|
||||
}
|
||||
|
||||
|
||||
@ -63,7 +63,6 @@ import com.google.gerrit.server.git.strategy.SubmitStrategyFactory;
|
||||
import com.google.gerrit.server.git.strategy.SubmitStrategyListener;
|
||||
import com.google.gerrit.server.git.validators.MergeValidationException;
|
||||
import com.google.gerrit.server.git.validators.MergeValidators;
|
||||
import com.google.gerrit.server.notedb.NotesMigration;
|
||||
import com.google.gerrit.server.project.ChangeControl;
|
||||
import com.google.gerrit.server.project.NoSuchProjectException;
|
||||
import com.google.gerrit.server.project.SubmitRuleOptions;
|
||||
@ -234,7 +233,6 @@ public class MergeOp implements AutoCloseable {
|
||||
private final Provider<MergeOpRepoManager> ormProvider;
|
||||
private final NotifyUtil notifyUtil;
|
||||
private final RetryHelper retryHelper;
|
||||
private final NotesMigration notesMigration;
|
||||
|
||||
private Timestamp ts;
|
||||
private RequestId submissionId;
|
||||
@ -262,8 +260,7 @@ public class MergeOp implements AutoCloseable {
|
||||
Provider<MergeOpRepoManager> ormProvider,
|
||||
NotifyUtil notifyUtil,
|
||||
TopicMetrics topicMetrics,
|
||||
RetryHelper retryHelper,
|
||||
NotesMigration notesMigration) {
|
||||
RetryHelper retryHelper) {
|
||||
this.cmUtil = cmUtil;
|
||||
this.batchUpdateFactory = batchUpdateFactory;
|
||||
this.internalUserFactory = internalUserFactory;
|
||||
@ -276,7 +273,6 @@ public class MergeOp implements AutoCloseable {
|
||||
this.notifyUtil = notifyUtil;
|
||||
this.retryHelper = retryHelper;
|
||||
this.topicMetrics = topicMetrics;
|
||||
this.notesMigration = notesMigration;
|
||||
}
|
||||
|
||||
@Override
|
||||
@ -909,11 +905,6 @@ public class MergeOp implements AutoCloseable {
|
||||
return "Error submitting changes";
|
||||
}
|
||||
if (p == 1) {
|
||||
if (!notesMigration.fuseUpdates()) {
|
||||
// No fused updates: any subset of changes might or might not have been submitted, so don't
|
||||
// make any strong claims.
|
||||
return "Error submitting changes";
|
||||
}
|
||||
// Fused updates: it's correct to say that none of the n changes were submitted.
|
||||
return "Error submitting " + c + " changes";
|
||||
}
|
||||
|
||||
@ -72,10 +72,6 @@ public class MutableNotesMigration extends NotesMigration {
|
||||
return set(b -> b.setDisableChangeReviewDb(disableChangeReviewDb));
|
||||
}
|
||||
|
||||
public MutableNotesMigration setFuseUpdates(boolean fuseUpdates) {
|
||||
return set(b -> b.setFuseUpdates(fuseUpdates));
|
||||
}
|
||||
|
||||
public MutableNotesMigration setFailOnLoadForTest(boolean failOnLoadForTest) {
|
||||
return set(b -> b.setFailOnLoadForTest(failOnLoadForTest));
|
||||
}
|
||||
|
||||
@ -56,7 +56,6 @@ public abstract class NotesMigration {
|
||||
public static final String SECTION_NOTE_DB = "noteDb";
|
||||
|
||||
private static final String DISABLE_REVIEW_DB = "disableReviewDb";
|
||||
private static final String FUSE_UPDATES = "fuseUpdates";
|
||||
private static final String PRIMARY_STORAGE = "primaryStorage";
|
||||
private static final String READ = "read";
|
||||
private static final String SEQUENCE = "sequence";
|
||||
@ -87,7 +86,6 @@ public abstract class NotesMigration {
|
||||
SECTION_NOTE_DB, CHANGES.key(), PRIMARY_STORAGE, PrimaryStorage.REVIEW_DB))
|
||||
.setDisableChangeReviewDb(
|
||||
cfg.getBoolean(SECTION_NOTE_DB, CHANGES.key(), DISABLE_REVIEW_DB, false))
|
||||
.setFuseUpdates(cfg.getBoolean(SECTION_NOTE_DB, CHANGES.key(), FUSE_UPDATES, false))
|
||||
.setFailOnLoadForTest(false) // Only set in tests, can't be set via config.
|
||||
.build();
|
||||
}
|
||||
@ -102,8 +100,6 @@ public abstract class NotesMigration {
|
||||
|
||||
abstract boolean disableChangeReviewDb();
|
||||
|
||||
abstract boolean fuseUpdates();
|
||||
|
||||
abstract boolean failOnLoadForTest();
|
||||
|
||||
abstract Builder toBuilder();
|
||||
@ -114,7 +110,6 @@ public abstract class NotesMigration {
|
||||
cfg.setBoolean(SECTION_NOTE_DB, CHANGES.key(), SEQUENCE, readChangeSequence());
|
||||
cfg.setEnum(SECTION_NOTE_DB, CHANGES.key(), PRIMARY_STORAGE, changePrimaryStorage());
|
||||
cfg.setBoolean(SECTION_NOTE_DB, CHANGES.key(), DISABLE_REVIEW_DB, disableChangeReviewDb());
|
||||
cfg.setBoolean(SECTION_NOTE_DB, CHANGES.key(), FUSE_UPDATES, fuseUpdates());
|
||||
}
|
||||
|
||||
@AutoValue.Builder
|
||||
@ -129,8 +124,6 @@ public abstract class NotesMigration {
|
||||
|
||||
abstract Builder setDisableChangeReviewDb(boolean disableChangeReviewDb);
|
||||
|
||||
abstract Builder setFuseUpdates(boolean fuseUpdates);
|
||||
|
||||
abstract Builder setFailOnLoadForTest(boolean failOnLoadForTest);
|
||||
|
||||
abstract Snapshot autoBuild();
|
||||
@ -205,21 +198,6 @@ public abstract class NotesMigration {
|
||||
return snapshot.get().disableChangeReviewDb();
|
||||
}
|
||||
|
||||
/**
|
||||
* Fuse meta ref updates in the same batch as code updates.
|
||||
*
|
||||
* <p>When set, each {@link com.google.gerrit.server.update.BatchUpdate} results in a single
|
||||
* {@link org.eclipse.jgit.lib.BatchRefUpdate} to update both code and meta refs atomically.
|
||||
* Setting this option with a repository backend that does not support atomic multi-ref
|
||||
* transactions ({@link org.eclipse.jgit.lib.RefDatabase#performsAtomicTransactions()}) is a
|
||||
* configuration error, and all updates will fail at runtime.
|
||||
*
|
||||
* <p>Has no effect if {@link #disableChangeReviewDb()} is false.
|
||||
*/
|
||||
public final boolean fuseUpdates() {
|
||||
return snapshot.get().fuseUpdates();
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether to fail when reading any data from NoteDb.
|
||||
*
|
||||
|
||||
@ -31,26 +31,19 @@ import org.eclipse.jgit.lib.Config;
|
||||
* in the order in which they are defined.
|
||||
*/
|
||||
public enum NotesMigrationState {
|
||||
REVIEW_DB(false, false, false, PrimaryStorage.REVIEW_DB, false, false),
|
||||
REVIEW_DB(false, false, false, PrimaryStorage.REVIEW_DB, false),
|
||||
|
||||
WRITE(false, true, false, PrimaryStorage.REVIEW_DB, false, false),
|
||||
WRITE(false, true, false, PrimaryStorage.REVIEW_DB, false),
|
||||
|
||||
READ_WRITE_NO_SEQUENCE(true, true, false, PrimaryStorage.REVIEW_DB, false, false),
|
||||
READ_WRITE_NO_SEQUENCE(true, true, false, PrimaryStorage.REVIEW_DB, false),
|
||||
|
||||
READ_WRITE_WITH_SEQUENCE_REVIEW_DB_PRIMARY(
|
||||
true, true, true, PrimaryStorage.REVIEW_DB, false, false),
|
||||
READ_WRITE_WITH_SEQUENCE_REVIEW_DB_PRIMARY(true, true, true, PrimaryStorage.REVIEW_DB, false),
|
||||
|
||||
READ_WRITE_WITH_SEQUENCE_NOTE_DB_PRIMARY(true, true, true, PrimaryStorage.NOTE_DB, false, false),
|
||||
READ_WRITE_WITH_SEQUENCE_NOTE_DB_PRIMARY(true, true, true, PrimaryStorage.NOTE_DB, false),
|
||||
|
||||
// TODO(dborowitz): This only exists as a separate state to support testing in different
|
||||
// NoteDbModes. Once FileRepository fuses BatchRefUpdates, we won't have separate fused/unfused
|
||||
// states.
|
||||
NOTE_DB_UNFUSED(true, true, true, PrimaryStorage.NOTE_DB, true, false),
|
||||
NOTE_DB(true, true, true, PrimaryStorage.NOTE_DB, true);
|
||||
|
||||
NOTE_DB(true, true, true, PrimaryStorage.NOTE_DB, true, true);
|
||||
|
||||
// TODO(dborowitz): Replace with NOTE_DB when FileRepository fuses BatchRefUpdates.
|
||||
public static final NotesMigrationState FINAL = NOTE_DB_UNFUSED;
|
||||
public static final NotesMigrationState FINAL = NOTE_DB;
|
||||
|
||||
public static Optional<NotesMigrationState> forConfig(Config cfg) {
|
||||
return forSnapshot(Snapshot.create(cfg));
|
||||
@ -72,8 +65,7 @@ public enum NotesMigrationState {
|
||||
boolean rawWriteChangesSetting,
|
||||
boolean readChangeSequence,
|
||||
PrimaryStorage changePrimaryStorage,
|
||||
boolean disableChangeReviewDb,
|
||||
boolean fuseUpdates) {
|
||||
boolean disableChangeReviewDb) {
|
||||
this.snapshot =
|
||||
Snapshot.builder()
|
||||
.setReadChanges(readChanges)
|
||||
@ -81,7 +73,6 @@ public enum NotesMigrationState {
|
||||
.setReadChangeSequence(readChangeSequence)
|
||||
.setChangePrimaryStorage(changePrimaryStorage)
|
||||
.setDisableChangeReviewDb(disableChangeReviewDb)
|
||||
.setFuseUpdates(fuseUpdates)
|
||||
.build();
|
||||
}
|
||||
|
||||
|
||||
@ -19,7 +19,7 @@ import static com.google.common.base.Preconditions.checkNotNull;
|
||||
import static com.google.common.base.Preconditions.checkState;
|
||||
import static com.google.gerrit.reviewdb.server.ReviewDbUtil.unwrapDb;
|
||||
import static com.google.gerrit.server.notedb.NotesMigration.SECTION_NOTE_DB;
|
||||
import static com.google.gerrit.server.notedb.NotesMigrationState.NOTE_DB_UNFUSED;
|
||||
import static com.google.gerrit.server.notedb.NotesMigrationState.NOTE_DB;
|
||||
import static com.google.gerrit.server.notedb.NotesMigrationState.READ_WRITE_NO_SEQUENCE;
|
||||
import static com.google.gerrit.server.notedb.NotesMigrationState.READ_WRITE_WITH_SEQUENCE_NOTE_DB_PRIMARY;
|
||||
import static com.google.gerrit.server.notedb.NotesMigrationState.READ_WRITE_WITH_SEQUENCE_REVIEW_DB_PRIMARY;
|
||||
@ -419,7 +419,7 @@ public class NoteDbMigrator implements AutoCloseable {
|
||||
}
|
||||
|
||||
boolean rebuilt = false;
|
||||
while (state.compareTo(NOTE_DB_UNFUSED) < 0) {
|
||||
while (state.compareTo(NOTE_DB) < 0) {
|
||||
if (state.equals(stopAtState)) {
|
||||
return;
|
||||
}
|
||||
@ -458,18 +458,15 @@ public class NoteDbMigrator implements AutoCloseable {
|
||||
break;
|
||||
case READ_WRITE_WITH_SEQUENCE_NOTE_DB_PRIMARY:
|
||||
// The only way we can get here is if there was a failure on a previous run of
|
||||
// setNoteDbPrimary, since that method moves to NOTE_DB_UNFUSED if it completes
|
||||
// setNoteDbPrimary, since that method moves to NOTE_DB if it completes
|
||||
// successfully. Assume that not all changes were converted and re-run the step.
|
||||
// migrateToNoteDbPrimary is a relatively fast no-op for already-migrated changes, so this
|
||||
// isn't actually repeating work.
|
||||
state = setNoteDbPrimary(state);
|
||||
break;
|
||||
case NOTE_DB_UNFUSED:
|
||||
case NOTE_DB:
|
||||
// Done!
|
||||
break;
|
||||
case NOTE_DB:
|
||||
// TODO(dborowitz): Allow this state once FileRepository supports fused updates.
|
||||
// Until then, fallthrough and throw.
|
||||
default:
|
||||
throw new MigrationException(
|
||||
"Migration out of the following state is not supported:\n" + state.toText());
|
||||
@ -561,7 +558,7 @@ public class NoteDbMigrator implements AutoCloseable {
|
||||
}
|
||||
|
||||
private NotesMigrationState disableReviewDb(NotesMigrationState prev) throws IOException {
|
||||
return saveState(prev, NOTE_DB_UNFUSED, c -> setAutoMigrate(c, false));
|
||||
return saveState(prev, NOTE_DB, c -> setAutoMigrate(c, false));
|
||||
}
|
||||
|
||||
private Optional<NotesMigrationState> loadState() throws IOException {
|
||||
|
||||
@ -95,8 +95,7 @@ public abstract class BatchUpdate implements AutoCloseable {
|
||||
@Override
|
||||
public void configure() {
|
||||
factory(ReviewDbBatchUpdate.AssistedFactory.class);
|
||||
factory(FusedNoteDbBatchUpdate.AssistedFactory.class);
|
||||
factory(UnfusedNoteDbBatchUpdate.AssistedFactory.class);
|
||||
factory(NoteDbBatchUpdate.AssistedFactory.class);
|
||||
}
|
||||
};
|
||||
}
|
||||
@ -105,29 +104,23 @@ public abstract class BatchUpdate implements AutoCloseable {
|
||||
public static class Factory {
|
||||
private final NotesMigration migration;
|
||||
private final ReviewDbBatchUpdate.AssistedFactory reviewDbBatchUpdateFactory;
|
||||
private final FusedNoteDbBatchUpdate.AssistedFactory fusedNoteDbBatchUpdateFactory;
|
||||
private final UnfusedNoteDbBatchUpdate.AssistedFactory unfusedNoteDbBatchUpdateFactory;
|
||||
private final NoteDbBatchUpdate.AssistedFactory noteDbBatchUpdateFactory;
|
||||
|
||||
// TODO(dborowitz): Make this non-injectable to force all callers to use RetryHelper.
|
||||
@Inject
|
||||
Factory(
|
||||
NotesMigration migration,
|
||||
ReviewDbBatchUpdate.AssistedFactory reviewDbBatchUpdateFactory,
|
||||
FusedNoteDbBatchUpdate.AssistedFactory fusedNoteDbBatchUpdateFactory,
|
||||
UnfusedNoteDbBatchUpdate.AssistedFactory unfusedNoteDbBatchUpdateFactory) {
|
||||
NoteDbBatchUpdate.AssistedFactory noteDbBatchUpdateFactory) {
|
||||
this.migration = migration;
|
||||
this.reviewDbBatchUpdateFactory = reviewDbBatchUpdateFactory;
|
||||
this.fusedNoteDbBatchUpdateFactory = fusedNoteDbBatchUpdateFactory;
|
||||
this.unfusedNoteDbBatchUpdateFactory = unfusedNoteDbBatchUpdateFactory;
|
||||
this.noteDbBatchUpdateFactory = noteDbBatchUpdateFactory;
|
||||
}
|
||||
|
||||
public BatchUpdate create(
|
||||
ReviewDb db, Project.NameKey project, CurrentUser user, Timestamp when) {
|
||||
if (migration.disableChangeReviewDb()) {
|
||||
if (migration.fuseUpdates()) {
|
||||
return fusedNoteDbBatchUpdateFactory.create(db, project, user, when);
|
||||
}
|
||||
return unfusedNoteDbBatchUpdateFactory.create(db, project, user, when);
|
||||
return noteDbBatchUpdateFactory.create(db, project, user, when);
|
||||
}
|
||||
return reviewDbBatchUpdateFactory.create(db, project, user, when);
|
||||
}
|
||||
@ -147,15 +140,9 @@ public abstract class BatchUpdate implements AutoCloseable {
|
||||
// copy them into an ImmutableList so there is no chance the callee can pollute the input
|
||||
// collection.
|
||||
if (migration.disableChangeReviewDb()) {
|
||||
if (migration.fuseUpdates()) {
|
||||
ImmutableList<FusedNoteDbBatchUpdate> noteDbUpdates =
|
||||
ImmutableList<NoteDbBatchUpdate> noteDbUpdates =
|
||||
(ImmutableList) ImmutableList.copyOf(updates);
|
||||
FusedNoteDbBatchUpdate.execute(noteDbUpdates, listener, requestId, dryRun);
|
||||
} else {
|
||||
ImmutableList<UnfusedNoteDbBatchUpdate> noteDbUpdates =
|
||||
(ImmutableList) ImmutableList.copyOf(updates);
|
||||
UnfusedNoteDbBatchUpdate.execute(noteDbUpdates, listener, requestId, dryRun);
|
||||
}
|
||||
NoteDbBatchUpdate.execute(noteDbUpdates, listener, requestId, dryRun);
|
||||
} else {
|
||||
ImmutableList<ReviewDbBatchUpdate> reviewDbUpdates =
|
||||
(ImmutableList) ImmutableList.copyOf(updates);
|
||||
|
||||
@ -62,14 +62,14 @@ import org.eclipse.jgit.transport.ReceiveCommand;
|
||||
* <p>Used when {@code noteDb.changes.disableReviewDb=true}, at which point ReviewDb is not
|
||||
* consulted during updates.
|
||||
*/
|
||||
class FusedNoteDbBatchUpdate extends BatchUpdate {
|
||||
class NoteDbBatchUpdate extends BatchUpdate {
|
||||
interface AssistedFactory {
|
||||
FusedNoteDbBatchUpdate create(
|
||||
NoteDbBatchUpdate create(
|
||||
ReviewDb db, Project.NameKey project, CurrentUser user, Timestamp when);
|
||||
}
|
||||
|
||||
static void execute(
|
||||
ImmutableList<FusedNoteDbBatchUpdate> updates,
|
||||
ImmutableList<NoteDbBatchUpdate> updates,
|
||||
BatchUpdateListener listener,
|
||||
@Nullable RequestId requestId,
|
||||
boolean dryrun)
|
||||
@ -88,11 +88,11 @@ class FusedNoteDbBatchUpdate extends BatchUpdate {
|
||||
try {
|
||||
switch (order) {
|
||||
case REPO_BEFORE_DB:
|
||||
for (FusedNoteDbBatchUpdate u : updates) {
|
||||
for (NoteDbBatchUpdate u : updates) {
|
||||
u.executeUpdateRepo();
|
||||
}
|
||||
listener.afterUpdateRepos();
|
||||
for (FusedNoteDbBatchUpdate u : updates) {
|
||||
for (NoteDbBatchUpdate u : updates) {
|
||||
handles.add(u.executeChangeOps(dryrun));
|
||||
}
|
||||
for (ChangesHandle h : handles) {
|
||||
@ -113,10 +113,10 @@ class FusedNoteDbBatchUpdate extends BatchUpdate {
|
||||
// TODO(dborowitz): This may still result in multiple updates to All-Users, but that's
|
||||
// currently not a big deal because multi-change batches generally aren't affecting
|
||||
// drafts anyway.
|
||||
for (FusedNoteDbBatchUpdate u : updates) {
|
||||
for (NoteDbBatchUpdate u : updates) {
|
||||
handles.add(u.executeChangeOps(dryrun));
|
||||
}
|
||||
for (FusedNoteDbBatchUpdate u : updates) {
|
||||
for (NoteDbBatchUpdate u : updates) {
|
||||
u.executeUpdateRepo();
|
||||
}
|
||||
for (ChangesHandle h : handles) {
|
||||
@ -151,7 +151,7 @@ class FusedNoteDbBatchUpdate extends BatchUpdate {
|
||||
u -> u.gitRefUpdated.fire(u.project, u.batchRefUpdate, u.getAccount().orElse(null)));
|
||||
|
||||
if (!dryrun) {
|
||||
for (FusedNoteDbBatchUpdate u : updates) {
|
||||
for (NoteDbBatchUpdate u : updates) {
|
||||
u.executePostOps();
|
||||
}
|
||||
}
|
||||
@ -163,7 +163,7 @@ class FusedNoteDbBatchUpdate extends BatchUpdate {
|
||||
class ContextImpl implements Context {
|
||||
@Override
|
||||
public RepoView getRepoView() throws IOException {
|
||||
return FusedNoteDbBatchUpdate.this.getRepoView();
|
||||
return NoteDbBatchUpdate.this.getRepoView();
|
||||
}
|
||||
|
||||
@Override
|
||||
@ -272,7 +272,7 @@ class FusedNoteDbBatchUpdate extends BatchUpdate {
|
||||
private final ReviewDb db;
|
||||
|
||||
@Inject
|
||||
FusedNoteDbBatchUpdate(
|
||||
NoteDbBatchUpdate(
|
||||
GitRepositoryManager repoManager,
|
||||
@GerritPersonIdent PersonIdent serverIdent,
|
||||
ChangeNotes.Factory changeNotesFactory,
|
||||
@ -354,7 +354,7 @@ class FusedNoteDbBatchUpdate extends BatchUpdate {
|
||||
}
|
||||
|
||||
void execute() throws OrmException, IOException {
|
||||
FusedNoteDbBatchUpdate.this.batchRefUpdate = manager.execute(dryrun);
|
||||
NoteDbBatchUpdate.this.batchRefUpdate = manager.execute(dryrun);
|
||||
}
|
||||
|
||||
@SuppressWarnings("deprecation")
|
||||
@ -51,15 +51,10 @@ public class RetryHelper {
|
||||
@GerritServerConfig Config cfg,
|
||||
NotesMigration migration,
|
||||
ReviewDbBatchUpdate.AssistedFactory reviewDbBatchUpdateFactory,
|
||||
FusedNoteDbBatchUpdate.AssistedFactory fusedNoteDbBatchUpdateFactory,
|
||||
UnfusedNoteDbBatchUpdate.AssistedFactory unfusedNoteDbBatchUpdateFactory) {
|
||||
NoteDbBatchUpdate.AssistedFactory noteDbBatchUpdateFactory) {
|
||||
this.migration = migration;
|
||||
this.updateFactory =
|
||||
new BatchUpdate.Factory(
|
||||
migration,
|
||||
reviewDbBatchUpdateFactory,
|
||||
fusedNoteDbBatchUpdateFactory,
|
||||
unfusedNoteDbBatchUpdateFactory);
|
||||
new BatchUpdate.Factory(migration, reviewDbBatchUpdateFactory, noteDbBatchUpdateFactory);
|
||||
this.stopStrategy =
|
||||
StopStrategies.stopAfterDelay(
|
||||
cfg.getTimeUnit("noteDb", null, "retryTimeout", SECONDS.toMillis(5), MILLISECONDS),
|
||||
@ -80,7 +75,7 @@ public class RetryHelper {
|
||||
throws RestApiException, UpdateException {
|
||||
try {
|
||||
RetryerBuilder<T> builder = RetryerBuilder.newBuilder();
|
||||
if (migration.disableChangeReviewDb() && migration.fuseUpdates()) {
|
||||
if (migration.disableChangeReviewDb()) {
|
||||
builder
|
||||
.withStopStrategy(stopStrategy)
|
||||
.withWaitStrategy(waitStrategy)
|
||||
|
||||
@ -1,459 +0,0 @@
|
||||
// Copyright (C) 2017 The Android Open Source Project
|
||||
//
|
||||
// Licensed under the Apache License, Version 2.0 (the "License");
|
||||
// you may not use this file except in compliance with the License.
|
||||
// You may obtain a copy of the License at
|
||||
//
|
||||
// http://www.apache.org/licenses/LICENSE-2.0
|
||||
//
|
||||
// Unless required by applicable law or agreed to in writing, software
|
||||
// distributed under the License is distributed on an "AS IS" BASIS,
|
||||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
// See the License for the specific language governing permissions and
|
||||
// limitations under the License.
|
||||
|
||||
package com.google.gerrit.server.update;
|
||||
|
||||
import static com.google.common.base.Preconditions.checkNotNull;
|
||||
import static java.util.Comparator.comparing;
|
||||
import static java.util.stream.Collectors.toList;
|
||||
|
||||
import com.google.common.base.Throwables;
|
||||
import com.google.common.collect.ImmutableList;
|
||||
import com.google.common.collect.Maps;
|
||||
import com.google.gerrit.common.Nullable;
|
||||
import com.google.gerrit.extensions.restapi.RestApiException;
|
||||
import com.google.gerrit.reviewdb.client.Change;
|
||||
import com.google.gerrit.reviewdb.client.PatchSet;
|
||||
import com.google.gerrit.reviewdb.client.Project;
|
||||
import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||
import com.google.gerrit.server.CurrentUser;
|
||||
import com.google.gerrit.server.GerritPersonIdent;
|
||||
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
|
||||
import com.google.gerrit.server.git.GitRepositoryManager;
|
||||
import com.google.gerrit.server.index.change.ChangeIndexer;
|
||||
import com.google.gerrit.server.notedb.ChangeNotes;
|
||||
import com.google.gerrit.server.notedb.ChangeUpdate;
|
||||
import com.google.gerrit.server.notedb.NoteDbUpdateManager;
|
||||
import com.google.gerrit.server.project.ChangeControl;
|
||||
import com.google.gerrit.server.util.RequestId;
|
||||
import com.google.gwtorm.server.OrmException;
|
||||
import com.google.inject.Inject;
|
||||
import com.google.inject.assistedinject.Assisted;
|
||||
import java.io.IOException;
|
||||
import java.sql.Timestamp;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collection;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.TimeZone;
|
||||
import java.util.TreeMap;
|
||||
import org.eclipse.jgit.lib.NullProgressMonitor;
|
||||
import org.eclipse.jgit.lib.ObjectInserter;
|
||||
import org.eclipse.jgit.lib.ObjectReader;
|
||||
import org.eclipse.jgit.lib.PersonIdent;
|
||||
import org.eclipse.jgit.lib.Repository;
|
||||
import org.eclipse.jgit.revwalk.RevWalk;
|
||||
import org.eclipse.jgit.transport.ReceiveCommand;
|
||||
|
||||
/**
|
||||
* {@link BatchUpdate} implementation that only supports NoteDb.
|
||||
*
|
||||
* <p>Used when {@code noteDb.changes.disableReviewDb=true}, at which point ReviewDb is not
|
||||
* consulted during updates.
|
||||
*/
|
||||
class UnfusedNoteDbBatchUpdate extends BatchUpdate {
|
||||
interface AssistedFactory {
|
||||
UnfusedNoteDbBatchUpdate create(
|
||||
ReviewDb db, Project.NameKey project, CurrentUser user, Timestamp when);
|
||||
}
|
||||
|
||||
static void execute(
|
||||
ImmutableList<UnfusedNoteDbBatchUpdate> updates,
|
||||
BatchUpdateListener listener,
|
||||
@Nullable RequestId requestId,
|
||||
boolean dryrun)
|
||||
throws UpdateException, RestApiException {
|
||||
if (updates.isEmpty()) {
|
||||
return;
|
||||
}
|
||||
setRequestIds(updates, requestId);
|
||||
|
||||
try {
|
||||
Order order = getOrder(updates, listener);
|
||||
// TODO(dborowitz): Fuse implementations to use a single BatchRefUpdate between phases. Note
|
||||
// that we may still need to respect the order, since op implementations may make assumptions
|
||||
// about the order in which their methods are called.
|
||||
switch (order) {
|
||||
case REPO_BEFORE_DB:
|
||||
for (UnfusedNoteDbBatchUpdate u : updates) {
|
||||
u.executeUpdateRepo();
|
||||
}
|
||||
listener.afterUpdateRepos();
|
||||
for (UnfusedNoteDbBatchUpdate u : updates) {
|
||||
u.executeRefUpdates(dryrun);
|
||||
}
|
||||
listener.afterUpdateRefs();
|
||||
for (UnfusedNoteDbBatchUpdate u : updates) {
|
||||
u.reindexChanges(u.executeChangeOps(dryrun), dryrun);
|
||||
}
|
||||
listener.afterUpdateChanges();
|
||||
break;
|
||||
case DB_BEFORE_REPO:
|
||||
for (UnfusedNoteDbBatchUpdate u : updates) {
|
||||
u.reindexChanges(u.executeChangeOps(dryrun), dryrun);
|
||||
}
|
||||
for (UnfusedNoteDbBatchUpdate u : updates) {
|
||||
u.executeUpdateRepo();
|
||||
}
|
||||
for (UnfusedNoteDbBatchUpdate u : updates) {
|
||||
u.executeRefUpdates(dryrun);
|
||||
}
|
||||
break;
|
||||
default:
|
||||
throw new IllegalStateException("invalid execution order: " + order);
|
||||
}
|
||||
|
||||
ChangeIndexer.allAsList(
|
||||
updates.stream().flatMap(u -> u.indexFutures.stream()).collect(toList()))
|
||||
.get();
|
||||
|
||||
// Fire ref update events only after all mutations are finished, since callers may assume a
|
||||
// patch set ref being created means the change was created, or a branch advancing meaning
|
||||
// some changes were closed.
|
||||
updates
|
||||
.stream()
|
||||
.filter(u -> u.batchRefUpdate != null)
|
||||
.forEach(
|
||||
u -> u.gitRefUpdated.fire(u.project, u.batchRefUpdate, u.getAccount().orElse(null)));
|
||||
|
||||
if (!dryrun) {
|
||||
for (UnfusedNoteDbBatchUpdate u : updates) {
|
||||
u.executePostOps();
|
||||
}
|
||||
}
|
||||
} catch (Exception e) {
|
||||
wrapAndThrowException(e);
|
||||
}
|
||||
}
|
||||
|
||||
class ContextImpl implements Context {
|
||||
@Override
|
||||
public RepoView getRepoView() throws IOException {
|
||||
return UnfusedNoteDbBatchUpdate.this.getRepoView();
|
||||
}
|
||||
|
||||
@Override
|
||||
public RevWalk getRevWalk() throws IOException {
|
||||
return getRepoView().getRevWalk();
|
||||
}
|
||||
|
||||
@Override
|
||||
public Project.NameKey getProject() {
|
||||
return project;
|
||||
}
|
||||
|
||||
@Override
|
||||
public Timestamp getWhen() {
|
||||
return when;
|
||||
}
|
||||
|
||||
@Override
|
||||
public TimeZone getTimeZone() {
|
||||
return tz;
|
||||
}
|
||||
|
||||
@Override
|
||||
public ReviewDb getDb() {
|
||||
return db;
|
||||
}
|
||||
|
||||
@Override
|
||||
public CurrentUser getUser() {
|
||||
return user;
|
||||
}
|
||||
|
||||
@Override
|
||||
public Order getOrder() {
|
||||
return order;
|
||||
}
|
||||
}
|
||||
|
||||
private class RepoContextImpl extends ContextImpl implements RepoContext {
|
||||
@Override
|
||||
public ObjectInserter getInserter() throws IOException {
|
||||
return getRepoView().getInserterWrapper();
|
||||
}
|
||||
|
||||
@Override
|
||||
public void addRefUpdate(ReceiveCommand cmd) throws IOException {
|
||||
getRepoView().getCommands().add(cmd);
|
||||
}
|
||||
}
|
||||
|
||||
private class ChangeContextImpl extends ContextImpl implements ChangeContext {
|
||||
private final ChangeControl ctl;
|
||||
private final Map<PatchSet.Id, ChangeUpdate> updates;
|
||||
|
||||
private boolean deleted;
|
||||
|
||||
protected ChangeContextImpl(ChangeControl ctl) {
|
||||
this.ctl = checkNotNull(ctl);
|
||||
updates = new TreeMap<>(comparing(PatchSet.Id::get));
|
||||
}
|
||||
|
||||
@Override
|
||||
public ChangeUpdate getUpdate(PatchSet.Id psId) {
|
||||
ChangeUpdate u = updates.get(psId);
|
||||
if (u == null) {
|
||||
u = changeUpdateFactory.create(ctl, when);
|
||||
if (newChanges.containsKey(ctl.getId())) {
|
||||
u.setAllowWriteToNewRef(true);
|
||||
}
|
||||
u.setPatchSetId(psId);
|
||||
updates.put(psId, u);
|
||||
}
|
||||
return u;
|
||||
}
|
||||
|
||||
@Override
|
||||
public ChangeControl getControl() {
|
||||
return ctl;
|
||||
}
|
||||
|
||||
@Override
|
||||
public void dontBumpLastUpdatedOn() {
|
||||
// Do nothing; NoteDb effectively updates timestamp if and only if a commit was written to the
|
||||
// change meta ref.
|
||||
}
|
||||
|
||||
@Override
|
||||
public void deleteChange() {
|
||||
deleted = true;
|
||||
}
|
||||
}
|
||||
|
||||
/** Per-change result status from {@link #executeChangeOps}. */
|
||||
private enum ChangeResult {
|
||||
SKIPPED,
|
||||
UPSERTED,
|
||||
DELETED;
|
||||
}
|
||||
|
||||
private final ChangeNotes.Factory changeNotesFactory;
|
||||
private final ChangeControl.GenericFactory changeControlFactory;
|
||||
private final ChangeUpdate.Factory changeUpdateFactory;
|
||||
private final NoteDbUpdateManager.Factory updateManagerFactory;
|
||||
private final ChangeIndexer indexer;
|
||||
private final GitReferenceUpdated gitRefUpdated;
|
||||
private final ReviewDb db;
|
||||
|
||||
@SuppressWarnings("deprecation")
|
||||
private List<com.google.common.util.concurrent.CheckedFuture<?, IOException>> indexFutures;
|
||||
|
||||
@Inject
|
||||
UnfusedNoteDbBatchUpdate(
|
||||
GitRepositoryManager repoManager,
|
||||
@GerritPersonIdent PersonIdent serverIdent,
|
||||
ChangeNotes.Factory changeNotesFactory,
|
||||
ChangeControl.GenericFactory changeControlFactory,
|
||||
ChangeUpdate.Factory changeUpdateFactory,
|
||||
NoteDbUpdateManager.Factory updateManagerFactory,
|
||||
ChangeIndexer indexer,
|
||||
GitReferenceUpdated gitRefUpdated,
|
||||
@Assisted ReviewDb db,
|
||||
@Assisted Project.NameKey project,
|
||||
@Assisted CurrentUser user,
|
||||
@Assisted Timestamp when) {
|
||||
super(repoManager, serverIdent, project, user, when);
|
||||
this.changeNotesFactory = changeNotesFactory;
|
||||
this.changeControlFactory = changeControlFactory;
|
||||
this.changeUpdateFactory = changeUpdateFactory;
|
||||
this.updateManagerFactory = updateManagerFactory;
|
||||
this.indexer = indexer;
|
||||
this.gitRefUpdated = gitRefUpdated;
|
||||
this.db = db;
|
||||
this.indexFutures = new ArrayList<>();
|
||||
}
|
||||
|
||||
@Override
|
||||
public void execute(BatchUpdateListener listener) throws UpdateException, RestApiException {
|
||||
execute(ImmutableList.of(this), listener, requestId, false);
|
||||
}
|
||||
|
||||
@Override
|
||||
protected Context newContext() {
|
||||
return new ContextImpl();
|
||||
}
|
||||
|
||||
private void executeUpdateRepo() throws UpdateException, RestApiException {
|
||||
try {
|
||||
logDebug("Executing updateRepo on {} ops", ops.size());
|
||||
RepoContextImpl ctx = new RepoContextImpl();
|
||||
for (BatchUpdateOp op : ops.values()) {
|
||||
op.updateRepo(ctx);
|
||||
}
|
||||
|
||||
logDebug("Executing updateRepo on {} RepoOnlyOps", repoOnlyOps.size());
|
||||
for (RepoOnlyOp op : repoOnlyOps) {
|
||||
op.updateRepo(ctx);
|
||||
}
|
||||
|
||||
if (onSubmitValidators != null && !getRefUpdates().isEmpty()) {
|
||||
// Validation of refs has to take place here and not at the beginning of executeRefUpdates.
|
||||
// Otherwise, failing validation in a second BatchUpdate object will happen *after* the
|
||||
// first update's executeRefUpdates has finished, hence after first repo's refs have been
|
||||
// updated, which is too late.
|
||||
onSubmitValidators.validate(
|
||||
project, ctx.getRevWalk().getObjectReader(), repoView.getCommands());
|
||||
}
|
||||
|
||||
// TODO(dborowitz): Don't flush when fusing phases.
|
||||
if (repoView != null) {
|
||||
logDebug("Flushing inserter");
|
||||
repoView.getInserter().flush();
|
||||
} else {
|
||||
logDebug("No objects to flush");
|
||||
}
|
||||
} catch (Exception e) {
|
||||
Throwables.throwIfInstanceOf(e, RestApiException.class);
|
||||
throw new UpdateException(e);
|
||||
}
|
||||
}
|
||||
|
||||
// TODO(dborowitz): Don't execute non-change ref updates separately when fusing phases.
|
||||
private void executeRefUpdates(boolean dryrun) throws IOException, RestApiException {
|
||||
if (getRefUpdates().isEmpty()) {
|
||||
logDebug("No ref updates to execute");
|
||||
return;
|
||||
}
|
||||
// May not be opened if the caller added ref updates but no new objects.
|
||||
initRepository();
|
||||
batchRefUpdate = repoView.getRepository().getRefDatabase().newBatchUpdate();
|
||||
batchRefUpdate.setPushCertificate(pushCert);
|
||||
batchRefUpdate.setRefLogMessage(refLogMessage, true);
|
||||
batchRefUpdate.setAllowNonFastForwards(true);
|
||||
repoView.getCommands().addTo(batchRefUpdate);
|
||||
logDebug("Executing batch of {} ref updates", batchRefUpdate.getCommands().size());
|
||||
if (dryrun) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Force BatchRefUpdate to read newly referenced objects using a new RevWalk, rather than one
|
||||
// that might have access to unflushed objects.
|
||||
try (RevWalk updateRw = new RevWalk(repoView.getRepository())) {
|
||||
batchRefUpdate.execute(updateRw, NullProgressMonitor.INSTANCE);
|
||||
}
|
||||
boolean ok = true;
|
||||
for (ReceiveCommand cmd : batchRefUpdate.getCommands()) {
|
||||
if (cmd.getResult() != ReceiveCommand.Result.OK) {
|
||||
ok = false;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (!ok) {
|
||||
throw new RestApiException("BatchRefUpdate failed: " + batchRefUpdate);
|
||||
}
|
||||
}
|
||||
|
||||
private Map<Change.Id, ChangeResult> executeChangeOps(boolean dryrun) throws Exception {
|
||||
logDebug("Executing change ops");
|
||||
Map<Change.Id, ChangeResult> result =
|
||||
Maps.newLinkedHashMapWithExpectedSize(ops.keySet().size());
|
||||
initRepository();
|
||||
Repository repo = repoView.getRepository();
|
||||
// TODO(dborowitz): Teach NoteDbUpdateManager to allow reusing the same inserter and batch ref
|
||||
// update as in executeUpdateRepo.
|
||||
try (ObjectInserter ins = repo.newObjectInserter();
|
||||
ObjectReader reader = ins.newReader();
|
||||
RevWalk rw = new RevWalk(reader);
|
||||
NoteDbUpdateManager updateManager =
|
||||
updateManagerFactory
|
||||
.create(project)
|
||||
.setChangeRepo(repo, rw, ins, new ChainedReceiveCommands(repo))) {
|
||||
if (user.isIdentifiedUser()) {
|
||||
updateManager.setRefLogIdent(user.asIdentifiedUser().newRefLogIdent(when, tz));
|
||||
}
|
||||
for (Map.Entry<Change.Id, Collection<BatchUpdateOp>> e : ops.asMap().entrySet()) {
|
||||
Change.Id id = e.getKey();
|
||||
ChangeContextImpl ctx = newChangeContext(id);
|
||||
boolean dirty = false;
|
||||
logDebug("Applying {} ops for change {}", e.getValue().size(), id);
|
||||
for (BatchUpdateOp op : e.getValue()) {
|
||||
dirty |= op.updateChange(ctx);
|
||||
}
|
||||
if (!dirty) {
|
||||
logDebug("No ops reported dirty, short-circuiting");
|
||||
result.put(id, ChangeResult.SKIPPED);
|
||||
continue;
|
||||
}
|
||||
for (ChangeUpdate u : ctx.updates.values()) {
|
||||
updateManager.add(u);
|
||||
}
|
||||
if (ctx.deleted) {
|
||||
logDebug("Change {} was deleted", id);
|
||||
updateManager.deleteChange(id);
|
||||
result.put(id, ChangeResult.DELETED);
|
||||
} else {
|
||||
result.put(id, ChangeResult.UPSERTED);
|
||||
}
|
||||
}
|
||||
|
||||
if (!dryrun) {
|
||||
logDebug("Executing NoteDb updates");
|
||||
updateManager.execute();
|
||||
}
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
private ChangeContextImpl newChangeContext(Change.Id id) throws OrmException {
|
||||
logDebug("Opening change {} for update", id);
|
||||
Change c = newChanges.get(id);
|
||||
boolean isNew = c != null;
|
||||
if (!isNew) {
|
||||
// Pass a synthetic change into ChangeNotes.Factory, which will take care of checking for
|
||||
// existence and populating columns from the parsed notes state.
|
||||
// TODO(dborowitz): This dance made more sense when using Reviewdb; consider a nicer way.
|
||||
c = ChangeNotes.Factory.newNoteDbOnlyChange(project, id);
|
||||
} else {
|
||||
logDebug("Change {} is new", id);
|
||||
}
|
||||
ChangeNotes notes = changeNotesFactory.createForBatchUpdate(c, !isNew);
|
||||
ChangeControl ctl = changeControlFactory.controlFor(notes, user);
|
||||
return new ChangeContextImpl(ctl);
|
||||
}
|
||||
|
||||
private void reindexChanges(Map<Change.Id, ChangeResult> updateResults, boolean dryrun) {
|
||||
if (dryrun) {
|
||||
return;
|
||||
}
|
||||
logDebug("Reindexing {} changes", updateResults.size());
|
||||
for (Map.Entry<Change.Id, ChangeResult> e : updateResults.entrySet()) {
|
||||
Change.Id id = e.getKey();
|
||||
switch (e.getValue()) {
|
||||
case UPSERTED:
|
||||
indexFutures.add(indexer.indexAsync(project, id));
|
||||
break;
|
||||
case DELETED:
|
||||
indexFutures.add(indexer.deleteAsync(id));
|
||||
break;
|
||||
case SKIPPED:
|
||||
break;
|
||||
default:
|
||||
throw new IllegalStateException("unexpected result: " + e.getValue());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private void executePostOps() throws Exception {
|
||||
ContextImpl ctx = new ContextImpl();
|
||||
for (BatchUpdateOp op : ops.values()) {
|
||||
op.postUpdate(ctx);
|
||||
}
|
||||
|
||||
for (RepoOnlyOp op : repoOnlyOps) {
|
||||
op.postUpdate(ctx);
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -19,7 +19,6 @@ import com.google.common.collect.Sets;
|
||||
import com.google.gerrit.reviewdb.client.Project;
|
||||
import com.google.gerrit.server.git.GitRepositoryManager;
|
||||
import com.google.gerrit.server.git.RepositoryCaseMismatchException;
|
||||
import com.google.gerrit.server.notedb.NotesMigration;
|
||||
import com.google.inject.Inject;
|
||||
import java.util.HashMap;
|
||||
import java.util.Map;
|
||||
@ -32,7 +31,7 @@ import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
|
||||
/** Repository manager that uses in-memory repositories. */
|
||||
public class InMemoryRepositoryManager implements GitRepositoryManager {
|
||||
public static InMemoryRepository newRepository(Project.NameKey name) {
|
||||
return new Repo(NoteDbMode.newNotesMigrationFromEnv(), name);
|
||||
return new Repo(name);
|
||||
}
|
||||
|
||||
public static class Description extends DfsRepositoryDescription {
|
||||
@ -51,15 +50,9 @@ public class InMemoryRepositoryManager implements GitRepositoryManager {
|
||||
public static class Repo extends InMemoryRepository {
|
||||
private String description;
|
||||
|
||||
private Repo(NotesMigration notesMigration, Project.NameKey name) {
|
||||
private Repo(Project.NameKey name) {
|
||||
super(new Description(name));
|
||||
// Normally, mimic the behavior of JGit FileRepository, the standard Gerrit repository
|
||||
// backend, and don't support atomic ref updates. The exception is when we're testing with
|
||||
// fused ref updates, which requires atomic ref updates to function.
|
||||
//
|
||||
// TODO(dborowitz): Change to match the behavior of JGit FileRepository after fixing
|
||||
// https://bugs.eclipse.org/bugs/show_bug.cgi?id=515678
|
||||
setPerformsAtomicTransactions(notesMigration.fuseUpdates());
|
||||
setPerformsAtomicTransactions(true);
|
||||
}
|
||||
|
||||
@Override
|
||||
@ -78,16 +71,10 @@ public class InMemoryRepositoryManager implements GitRepositoryManager {
|
||||
}
|
||||
}
|
||||
|
||||
private final NotesMigration notesMigration;
|
||||
private final Map<String, Repo> repos;
|
||||
|
||||
public InMemoryRepositoryManager() {
|
||||
this(NoteDbMode.newNotesMigrationFromEnv());
|
||||
}
|
||||
|
||||
@Inject
|
||||
InMemoryRepositoryManager(NotesMigration notesMigration) {
|
||||
this.notesMigration = notesMigration;
|
||||
public InMemoryRepositoryManager() {
|
||||
this.repos = new HashMap<>();
|
||||
}
|
||||
|
||||
@ -106,7 +93,7 @@ public class InMemoryRepositoryManager implements GitRepositoryManager {
|
||||
throw new RepositoryCaseMismatchException(name);
|
||||
}
|
||||
} catch (RepositoryNotFoundException e) {
|
||||
repo = new Repo(notesMigration, name);
|
||||
repo = new Repo(name);
|
||||
repos.put(normalize(name), repo);
|
||||
}
|
||||
return repo;
|
||||
|
||||
@ -35,11 +35,15 @@ public enum NoteDbMode {
|
||||
PRIMARY(NotesMigrationState.READ_WRITE_WITH_SEQUENCE_NOTE_DB_PRIMARY),
|
||||
|
||||
/** All change tables are entirely disabled. */
|
||||
DISABLE_CHANGE_REVIEW_DB(NotesMigrationState.NOTE_DB_UNFUSED),
|
||||
DISABLE_CHANGE_REVIEW_DB(NotesMigrationState.NOTE_DB),
|
||||
|
||||
/** All change tables are entirely disabled, and code/meta ref updates are fused. */
|
||||
FUSED(NotesMigrationState.NOTE_DB),
|
||||
|
||||
// TODO(dborowitz): Change CI to use this, then remove FUSED and DISABLE_CHANGE_REVIEW_DB.
|
||||
/** All change tables are entirely disabled, and code/meta ref updates are fused. */
|
||||
ON(NotesMigrationState.NOTE_DB),
|
||||
|
||||
/**
|
||||
* Run tests with NoteDb disabled, then convert ReviewDb to NoteDb and check that the results
|
||||
* match.
|
||||
|
||||
Loading…
Reference in New Issue
Block a user