Merge "Merge branch 'stable-2.14'"
diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GerritServer.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GerritServer.java
index 52dc4f2..2b13df0 100644
--- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GerritServer.java
+++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GerritServer.java
@@ -61,6 +61,14 @@
import org.eclipse.jgit.util.FS;
public class GerritServer implements AutoCloseable {
+ public static class StartupException extends Exception {
+ private static final long serialVersionUID = 1L;
+
+ StartupException(String msg, Throwable cause) {
+ super(msg, cause);
+ }
+ }
+
@AutoValue
public abstract static class Description {
public static Description forTestClass(
@@ -301,7 +309,12 @@
}
return null;
});
- serverStarted.await();
+ try {
+ serverStarted.await();
+ } catch (BrokenBarrierException e) {
+ daemon.stop();
+ throw new StartupException("Failed to start Gerrit daemon; see log", e);
+ }
System.out.println("Gerrit Server Started");
return new GerritServer(desc, createTestInjector(daemon), daemon, daemonService);
diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/StandaloneSiteTest.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/StandaloneSiteTest.java
new file mode 100644
index 0000000..be34ba6
--- /dev/null
+++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/StandaloneSiteTest.java
@@ -0,0 +1,142 @@
+// 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.acceptance;
+
+import static com.google.common.truth.Truth.assertThat;
+import static java.util.stream.Collectors.joining;
+import static org.junit.Assert.fail;
+
+import com.google.common.collect.Streams;
+import com.google.gerrit.launcher.GerritLauncher;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.util.ManualRequestContext;
+import com.google.gerrit.server.util.OneOffRequestContext;
+import com.google.gerrit.server.util.RequestContext;
+import com.google.gerrit.testutil.ConfigSuite;
+import com.google.inject.Injector;
+import com.google.inject.Provider;
+import java.util.Arrays;
+import org.eclipse.jgit.lib.Config;
+import org.junit.Rule;
+import org.junit.rules.RuleChain;
+import org.junit.rules.TemporaryFolder;
+import org.junit.rules.TestRule;
+import org.junit.runner.Description;
+import org.junit.runner.RunWith;
+import org.junit.runners.model.Statement;
+
+@RunWith(ConfigSuite.class)
+@UseLocalDisk
+public abstract class StandaloneSiteTest {
+ protected class ServerContext implements RequestContext, AutoCloseable {
+ private final GerritServer server;
+ private final ManualRequestContext ctx;
+
+ private ServerContext(GerritServer server) throws Exception {
+ this.server = server;
+ Injector i = server.getTestInjector();
+ if (adminId == null) {
+ adminId = i.getInstance(AccountCreator.class).admin().getId();
+ }
+ ctx = i.getInstance(OneOffRequestContext.class).openAs(adminId);
+ }
+
+ @Override
+ public CurrentUser getUser() {
+ return ctx.getUser();
+ }
+
+ @Override
+ public Provider<ReviewDb> getReviewDbProvider() {
+ return ctx.getReviewDbProvider();
+ }
+
+ public Injector getInjector() {
+ return server.getTestInjector();
+ }
+
+ @Override
+ public void close() throws Exception {
+ try {
+ ctx.close();
+ } finally {
+ server.close();
+ }
+ }
+ }
+
+ @ConfigSuite.Parameter public Config baseConfig;
+ @ConfigSuite.Name private String configName;
+
+ private final TemporaryFolder tempSiteDir = new TemporaryFolder();
+
+ private final TestRule testRunner =
+ new TestRule() {
+ @Override
+ public Statement apply(Statement base, Description description) {
+ return new Statement() {
+ @Override
+ public void evaluate() throws Throwable {
+ beforeTest(description);
+ base.evaluate();
+ }
+ };
+ }
+ };
+
+ @Rule public RuleChain ruleChain = RuleChain.outerRule(tempSiteDir).around(testRunner);
+
+ protected SitePaths sitePaths;
+
+ private GerritServer.Description serverDesc;
+ private Account.Id adminId;
+
+ private void beforeTest(Description description) throws Exception {
+ serverDesc = GerritServer.Description.forTestMethod(description, configName);
+ sitePaths = new SitePaths(tempSiteDir.getRoot().toPath());
+ GerritServer.init(serverDesc, baseConfig, sitePaths.site_path);
+ }
+
+ protected ServerContext startServer() throws Exception {
+ return new ServerContext(startImpl());
+ }
+
+ protected void assertServerStartupFails() throws Exception {
+ try (GerritServer server = startImpl()) {
+ fail("expected server startup to fail");
+ } catch (GerritServer.StartupException e) {
+ // Expected.
+ }
+ }
+
+ private GerritServer startImpl() throws Exception {
+ return GerritServer.start(serverDesc, baseConfig, sitePaths.site_path);
+ }
+
+ protected static void runGerrit(String... args) throws Exception {
+ assertThat(GerritLauncher.mainImpl(args))
+ .named("gerrit.war " + Arrays.stream(args).collect(joining(" ")))
+ .isEqualTo(0);
+ }
+
+ @SafeVarargs
+ protected static void runGerrit(Iterable<String>... multiArgs) throws Exception {
+ runGerrit(
+ Arrays.stream(multiArgs).flatMap(args -> Streams.stream(args)).toArray(String[]::new));
+ }
+}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/pgm/OfflineNoteDbMigrationIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/pgm/OfflineNoteDbMigrationIT.java
index 1676f3e..9b36b03 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/pgm/OfflineNoteDbMigrationIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/pgm/OfflineNoteDbMigrationIT.java
@@ -17,48 +17,36 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth8.assertThat;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
-import com.google.gerrit.acceptance.AccountCreator;
-import com.google.gerrit.acceptance.GerritServer;
import com.google.gerrit.acceptance.NoHttpd;
-import com.google.gerrit.acceptance.TestAccount;
-import com.google.gerrit.acceptance.UseLocalDisk;
+import com.google.gerrit.acceptance.StandaloneSiteTest;
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.common.ChangeInput;
-import com.google.gerrit.launcher.GerritLauncher;
-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.config.SitePaths;
import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.index.GerritIndexStatus;
+import com.google.gerrit.server.index.change.ChangeSchemaDefinitions;
import com.google.gerrit.server.notedb.ConfigNotesMigration;
import com.google.gerrit.server.notedb.NoteDbChangeState;
import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.notedb.NoteDbChangeState.RefState;
import com.google.gerrit.server.notedb.NotesMigrationState;
-import com.google.gerrit.server.util.ManualRequestContext;
-import com.google.gerrit.server.util.OneOffRequestContext;
-import com.google.gerrit.testutil.ConfigSuite;
-import com.google.gerrit.testutil.TempFileUtil;
-import com.google.inject.Injector;
-import java.nio.file.Path;
-import java.util.stream.Stream;
-import org.eclipse.jgit.lib.Config;
+import com.google.gerrit.server.schema.ReviewDbFactory;
+import com.google.gwtorm.server.SchemaFactory;
+import com.google.inject.Key;
+import com.google.inject.TypeLiteral;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.lib.StoredConfig;
import org.eclipse.jgit.storage.file.FileBasedConfig;
import org.eclipse.jgit.util.FS;
-import org.junit.After;
import org.junit.Before;
-import org.junit.Rule;
import org.junit.Test;
-import org.junit.rules.TestWatcher;
-import org.junit.runner.Description;
/**
* Tests for offline {@code migrate-to-note-db} program.
@@ -67,119 +55,115 @@
* adding tests to {@link com.google.gerrit.acceptance.server.notedb.OnlineNoteDbMigrationIT} if
* possible.
*/
-@UseLocalDisk
@NoHttpd
-public class OfflineNoteDbMigrationIT {
- @Rule
- public TestWatcher testWatcher =
- new TestWatcher() {
- @Override
- protected void starting(Description description) {
- serverDesc = GerritServer.Description.forTestMethod(description, ConfigSuite.DEFAULT);
- }
- };
-
- private GerritServer.Description serverDesc;
-
- private Path site;
+public class OfflineNoteDbMigrationIT extends StandaloneSiteTest {
private StoredConfig gerritConfig;
+ private Project.NameKey project;
+ private Change.Id changeId;
+
@Before
public void setUp() throws Exception {
- site = TempFileUtil.createTempDirectory().toPath();
- GerritServer.init(serverDesc, new Config(), site);
- gerritConfig = new FileBasedConfig(new SitePaths(site).gerrit_config.toFile(), FS.detect());
- }
-
- @After
- public void tearDown() throws Exception {
- TempFileUtil.cleanup();
- }
-
- @Test
- public void rebuildEmptySiteStartingWithNoteDbDisabed() throws Exception {
- assertNotesMigrationState(NotesMigrationState.REVIEW_DB);
- migrate();
- assertNotesMigrationState(NotesMigrationState.READ_WRITE_NO_SEQUENCE);
- }
-
- @Test
- public void rebuildEmptySiteStartingWithNoteDbEnabled() throws Exception {
- setNotesMigrationState(NotesMigrationState.READ_WRITE_NO_SEQUENCE);
- migrate();
- assertNotesMigrationState(NotesMigrationState.READ_WRITE_NO_SEQUENCE);
+ gerritConfig = new FileBasedConfig(sitePaths.gerrit_config.toFile(), FS.detect());
}
@Test
public void rebuildOneChangeTrialMode() throws Exception {
assertNotesMigrationState(NotesMigrationState.REVIEW_DB);
- Project.NameKey project = new Project.NameKey("project");
+ setUpOneChange();
- Account.Id accountId;
- Change.Id id;
- try (GerritServer server = startServer();
- ManualRequestContext ctx = openContext(server)) {
- accountId = ctx.getUser().getAccountId();
- GerritApi gApi = server.getTestInjector().getInstance(GerritApi.class);
- gApi.projects().create("project");
-
- ChangeInput in = new ChangeInput(project.get(), "master", "Test change");
- in.newBranch = true;
- id = new Change.Id(gApi.changes().create(in).info()._number);
- }
-
- migrate("--trial");
+ migrate();
assertNotesMigrationState(NotesMigrationState.READ_WRITE_NO_SEQUENCE);
- try (GerritServer server = startServer();
- ManualRequestContext ctx = openContext(server, accountId)) {
- GitRepositoryManager repoManager =
- server.getTestInjector().getInstance(GitRepositoryManager.class);
+ try (ServerContext ctx = startServer()) {
+ GitRepositoryManager repoManager = ctx.getInjector().getInstance(GitRepositoryManager.class);
ObjectId metaId;
try (Repository repo = repoManager.openRepository(project)) {
- Ref ref = repo.exactRef(RefNames.changeMetaRef(id));
+ Ref ref = repo.exactRef(RefNames.changeMetaRef(changeId));
assertThat(ref).isNotNull();
metaId = ref.getObjectId();
}
- ReviewDb db = ReviewDbUtil.unwrapDb(ctx.getReviewDbProvider().get());
- Change c = db.changes().get(id);
- assertThat(c).isNotNull();
- NoteDbChangeState state = NoteDbChangeState.parse(c);
- assertThat(state).isNotNull();
- assertThat(state.getPrimaryStorage()).isEqualTo(PrimaryStorage.REVIEW_DB);
- assertThat(state.getRefState()).hasValue(RefState.create(metaId, ImmutableMap.of()));
+ try (ReviewDb db = openUnderlyingReviewDb(ctx)) {
+ Change c = db.changes().get(changeId);
+ assertThat(c).isNotNull();
+ NoteDbChangeState state = NoteDbChangeState.parse(c);
+ assertThat(state).isNotNull();
+ assertThat(state.getPrimaryStorage()).isEqualTo(PrimaryStorage.REVIEW_DB);
+ assertThat(state.getRefState()).hasValue(RefState.create(metaId, ImmutableMap.of()));
+ }
}
}
- private GerritServer startServer() throws Exception {
- return GerritServer.start(serverDesc, new Config(), site);
+ @Test
+ public void migrateOneChange() throws Exception {
+ assertNotesMigrationState(NotesMigrationState.REVIEW_DB);
+ setUpOneChange();
+
+ migrate("--trial", "false");
+ assertNotesMigrationState(NotesMigrationState.NOTE_DB_UNFUSED);
+
+ try (ServerContext ctx = startServer()) {
+ GitRepositoryManager repoManager = ctx.getInjector().getInstance(GitRepositoryManager.class);
+ try (Repository repo = repoManager.openRepository(project)) {
+ assertThat(repo.exactRef(RefNames.changeMetaRef(changeId))).isNotNull();
+ }
+
+ try (ReviewDb db = openUnderlyingReviewDb(ctx)) {
+ Change c = db.changes().get(changeId);
+ assertThat(c).isNotNull();
+ NoteDbChangeState state = NoteDbChangeState.parse(c);
+ assertThat(state).isNotNull();
+ assertThat(state.getPrimaryStorage()).isEqualTo(PrimaryStorage.NOTE_DB);
+ assertThat(state.getRefState()).isEmpty();
+
+ ChangeInput in = new ChangeInput(project.get(), "master", "NoteDb-only change");
+ in.newBranch = true;
+ GerritApi gApi = ctx.getInjector().getInstance(GerritApi.class);
+ Change.Id id2 = new Change.Id(gApi.changes().create(in).info()._number);
+ assertThat(db.changes().get(id2)).isNull();
+ }
+ }
}
- private ManualRequestContext openContext(GerritServer server) throws Exception {
- Injector i = server.getTestInjector();
- TestAccount a = i.getInstance(AccountCreator.class).admin();
- return openContext(server, a.getId());
+ @Test
+ public void migrationDoesNotRequireIndex() throws Exception {
+ assertNotesMigrationState(NotesMigrationState.REVIEW_DB);
+ setUpOneChange();
+
+ int version = ChangeSchemaDefinitions.INSTANCE.getLatest().getVersion();
+ GerritIndexStatus status = new GerritIndexStatus(sitePaths);
+ assertThat(status.getReady(ChangeSchemaDefinitions.NAME, version)).isTrue();
+ status.setReady(ChangeSchemaDefinitions.NAME, version, false);
+ status.save();
+
+ migrate("--trial", "false");
+ assertNotesMigrationState(NotesMigrationState.NOTE_DB_UNFUSED);
+
+ status = new GerritIndexStatus(sitePaths);
+ assertThat(status.getReady(ChangeSchemaDefinitions.NAME, version)).isFalse();
+
+ // TODO(dborowitz): Remove when offline migration includes reindex.
+ assertServerStartupFails();
}
- private ManualRequestContext openContext(GerritServer server, Account.Id accountId)
- throws Exception {
- return server.getTestInjector().getInstance(OneOffRequestContext.class).openAs(accountId);
+ private void setUpOneChange() throws Exception {
+ project = new Project.NameKey("project");
+ try (ServerContext ctx = startServer()) {
+ GerritApi gApi = ctx.getInjector().getInstance(GerritApi.class);
+ gApi.projects().create("project");
+
+ ChangeInput in = new ChangeInput(project.get(), "master", "Test change");
+ in.newBranch = true;
+ changeId = new Change.Id(gApi.changes().create(in).info()._number);
+ }
}
private void migrate(String... additionalArgs) throws Exception {
- String[] args =
- Stream.concat(
- Stream.of("migrate-to-note-db", "-d", site.toString(), "--show-stack-trace"),
- Stream.of(additionalArgs))
- .toArray(String[]::new);
- assertThat(GerritLauncher.mainImpl(args)).isEqualTo(0);
- }
-
- private void setNotesMigrationState(NotesMigrationState state) throws Exception {
- gerritConfig.load();
- ConfigNotesMigration.setConfigValues(gerritConfig, state.migration());
- gerritConfig.save();
+ runGerrit(
+ ImmutableList.of(
+ "migrate-to-note-db", "-d", sitePaths.site_path.toString(), "--show-stack-trace"),
+ ImmutableList.copyOf(additionalArgs));
}
private void assertNotesMigrationState(NotesMigrationState expected) throws Exception {
@@ -187,4 +171,10 @@
assertThat(NotesMigrationState.forNotesMigration(new ConfigNotesMigration(gerritConfig)))
.hasValue(expected);
}
+
+ private ReviewDb openUnderlyingReviewDb(ServerContext ctx) throws Exception {
+ return ctx.getInjector()
+ .getInstance(Key.get(new TypeLiteral<SchemaFactory<ReviewDb>>() {}, ReviewDbFactory.class))
+ .open();
+ }
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/pgm/ReindexIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/pgm/ReindexIT.java
index 79bbba2..10c51df 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/pgm/ReindexIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/pgm/ReindexIT.java
@@ -14,48 +14,14 @@
package com.google.gerrit.acceptance.pgm;
-import static com.google.common.truth.Truth.assertThat;
-
-import com.google.gerrit.launcher.GerritLauncher;
-import com.google.gerrit.testutil.TempFileUtil;
-import java.io.File;
-import org.junit.After;
-import org.junit.Before;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.StandaloneSiteTest;
import org.junit.Test;
-public class ReindexIT {
- private File sitePath;
-
- @Before
- public void createTempDirectory() throws Exception {
- sitePath = TempFileUtil.createTempDirectory();
- }
-
- @After
- public void destroySite() throws Exception {
- if (sitePath != null) {
- TempFileUtil.cleanup();
- }
- }
-
+@NoHttpd
+public class ReindexIT extends StandaloneSiteTest {
@Test
public void reindexEmptySite() throws Exception {
- initSite();
- runGerrit("reindex", "-d", sitePath.toString(), "--show-stack-trace");
- }
-
- private void initSite() throws Exception {
- runGerrit(
- "init",
- "-d",
- sitePath.getPath(),
- "--batch",
- "--no-auto-start",
- "--skip-plugins",
- "--show-stack-trace");
- }
-
- private static void runGerrit(String... args) throws Exception {
- assertThat(GerritLauncher.mainImpl(args)).isEqualTo(0);
+ runGerrit("reindex", "-d", sitePaths.site_path.toString(), "--show-stack-trace");
}
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java
index 8902c4e..ae875f4 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java
@@ -122,6 +122,10 @@
// want precise control over when auto-rebuilding happens.
cfg.setBoolean("index", null, "autoReindexIfStale", false);
+ // setNotesMigration tries to keep IDs in sync between ReviewDb and NoteDb, which is behavior
+ // unique to this test. This gets prohibitively slow if we use the default sequence gap.
+ cfg.setInt("noteDb", "changes", "initialSequenceGap", 0);
+
return cfg;
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
index 4008634..bb72a58 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
@@ -17,10 +17,18 @@
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.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;
+import static com.google.gerrit.server.notedb.NotesMigrationState.REVIEW_DB;
+import static com.google.gerrit.server.notedb.NotesMigrationState.WRITE;
+import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.Sandboxed;
@@ -28,6 +36,7 @@
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.server.Sequences;
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.notedb.ConfigNotesMigration;
import com.google.gerrit.server.notedb.NoteDbChangeState;
@@ -37,13 +46,18 @@
import com.google.gerrit.server.notedb.rebuild.MigrationException;
import com.google.gerrit.server.notedb.rebuild.NoteDbMigrator;
import com.google.gerrit.server.schema.ReviewDbFactory;
+import com.google.gerrit.testutil.ConfigSuite;
import com.google.gerrit.testutil.NoteDbMode;
import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
import com.google.inject.Provider;
import java.util.List;
import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectLoader;
+import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.storage.file.FileBasedConfig;
@@ -56,12 +70,21 @@
public class OnlineNoteDbMigrationIT extends AbstractDaemonTest {
private static final String INVALID_STATE = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef";
+ @ConfigSuite.Default
+ public static Config defaultConfig() {
+ Config cfg = new Config();
+ cfg.setInt("noteDb", "changes", "sequenceBatchSize", 10);
+ cfg.setInt("noteDb", "changes", "initialSequenceGap", 500);
+ return cfg;
+ }
+
// Tests in this class are generally interested in the actual ReviewDb contents, but the shifting
// migration state may result in various kinds of wrappers showing up unexpectedly.
@Inject @ReviewDbFactory private SchemaFactory<ReviewDb> schemaFactory;
@Inject private SitePaths sitePaths;
@Inject private Provider<NoteDbMigrator.Builder> migratorBuilderProvider;
+ @Inject private Sequences sequences;
private FileBasedConfig gerritConfig;
@@ -69,7 +92,7 @@
public void setUp() throws Exception {
assume().that(NoteDbMode.get()).isEqualTo(NoteDbMode.OFF);
gerritConfig = new FileBasedConfig(sitePaths.gerrit_config.toFile(), FS.detect());
- assertNotesMigrationState(NotesMigrationState.REVIEW_DB);
+ assertNotesMigrationState(REVIEW_DB);
}
@Test
@@ -89,13 +112,13 @@
b -> b.setProjects(ps),
NoteDbMigrator::migrate);
- setNotesMigrationState(NotesMigrationState.READ_WRITE_WITH_SEQUENCE_REVIEW_DB_PRIMARY);
+ setNotesMigrationState(READ_WRITE_WITH_SEQUENCE_REVIEW_DB_PRIMARY);
assertMigrationException(
"Migration has already progressed past the endpoint of the \"trial mode\" state",
b -> b.setTrialMode(true),
NoteDbMigrator::migrate);
- setNotesMigrationState(NotesMigrationState.READ_WRITE_WITH_SEQUENCE_NOTE_DB_PRIMARY);
+ setNotesMigrationState(READ_WRITE_WITH_SEQUENCE_NOTE_DB_PRIMARY);
assertMigrationException(
"Cannot force rebuild changes; NoteDb is already the primary storage for some changes",
b -> b.setForceRebuild(true),
@@ -103,6 +126,13 @@
}
@Test
+ @GerritConfig(name = "noteDb.changes.initialSequenceGap", value = "-7")
+ public void initialSequenceGapMustBeNonNegative() throws Exception {
+ setNotesMigrationState(READ_WRITE_NO_SEQUENCE);
+ assertMigrationException("Sequence gap must be non-negative: -7", b -> b, m -> {});
+ }
+
+ @Test
public void rebuildOneChangeTrialModeAndForceRebuild() throws Exception {
PushOneCommit.Result r = createChange();
Change.Id id = r.getChange().getId();
@@ -110,7 +140,7 @@
try (NoteDbMigrator migrator = migratorBuilderProvider.get().setTrialMode(true).build()) {
migrator.migrate();
}
- assertNotesMigrationState(NotesMigrationState.READ_WRITE_NO_SEQUENCE);
+ assertNotesMigrationState(READ_WRITE_NO_SEQUENCE);
ObjectId oldMetaId;
try (Repository repo = repoManager.openRepository(project);
@@ -134,7 +164,7 @@
}
migrate(b -> b.setTrialMode(true));
- assertNotesMigrationState(NotesMigrationState.READ_WRITE_NO_SEQUENCE);
+ assertNotesMigrationState(READ_WRITE_NO_SEQUENCE);
try (Repository repo = repoManager.openRepository(project);
ReviewDb db = schemaFactory.open()) {
@@ -145,7 +175,7 @@
}
migrate(b -> b.setTrialMode(true).setForceRebuild(true));
- assertNotesMigrationState(NotesMigrationState.READ_WRITE_NO_SEQUENCE);
+ assertNotesMigrationState(READ_WRITE_NO_SEQUENCE);
try (Repository repo = repoManager.openRepository(project);
ReviewDb db = schemaFactory.open()) {
@@ -163,7 +193,7 @@
@Test
public void rebuildSubsetOfChanges() throws Exception {
- setNotesMigrationState(NotesMigrationState.WRITE);
+ setNotesMigrationState(WRITE);
PushOneCommit.Result r1 = createChange();
PushOneCommit.Result r2 = createChange();
@@ -191,7 +221,7 @@
@Test
public void rebuildSubsetOfProjects() throws Exception {
- setNotesMigrationState(NotesMigrationState.WRITE);
+ setNotesMigrationState(WRITE);
Project.NameKey p2 = createProject("project2");
TestRepository<?> tr2 = cloneProject(p2, admin);
@@ -221,6 +251,92 @@
}
}
+ @Test
+ public void enableSequencesNoGap() throws Exception {
+ testEnableSequences(0, 2, "12");
+ }
+
+ @Test
+ public void enableSequencesWithGap() throws Exception {
+ testEnableSequences(-1, 502, "512");
+ }
+
+ private void testEnableSequences(int builderOption, int expectedFirstId, String expectedRefValue)
+ throws Exception {
+ PushOneCommit.Result r = createChange();
+ Change.Id id = r.getChange().getId();
+ assertThat(id.get()).isEqualTo(1);
+
+ migrate(
+ b ->
+ b.setSequenceGap(builderOption)
+ .setStopAtStateForTesting(READ_WRITE_WITH_SEQUENCE_REVIEW_DB_PRIMARY));
+
+ assertThat(sequences.nextChangeId()).isEqualTo(expectedFirstId);
+ assertThat(sequences.nextChangeId()).isEqualTo(expectedFirstId + 1);
+
+ try (Repository repo = repoManager.openRepository(allProjects);
+ ObjectReader reader = repo.newObjectReader()) {
+ Ref ref = repo.exactRef("refs/sequences/changes");
+ assertThat(ref).isNotNull();
+ ObjectLoader loader = reader.open(ref.getObjectId());
+ assertThat(loader.getType()).isEqualTo(Constants.OBJ_BLOB);
+ // Acquired a block of 10 to serve the first nextChangeId call after migration.
+ assertThat(new String(loader.getCachedBytes(), UTF_8)).isEqualTo(expectedRefValue);
+ }
+
+ try (ReviewDb db = schemaFactory.open()) {
+ // Underlying, unused ReviewDb is still on its own sequence.
+ @SuppressWarnings("deprecation")
+ int nextFromReviewDb = db.nextChangeId();
+ assertThat(nextFromReviewDb).isEqualTo(3);
+ }
+ }
+
+ @Test
+ public void fullMigration() throws Exception {
+ PushOneCommit.Result r = createChange();
+ Change.Id id = r.getChange().getId();
+
+ migrate(b -> b);
+ assertNotesMigrationState(NOTE_DB_UNFUSED);
+
+ assertThat(sequences.nextChangeId()).isEqualTo(502);
+
+ ObjectId oldMetaId;
+ int rowVersion;
+ try (ReviewDb db = schemaFactory.open();
+ Repository repo = repoManager.openRepository(project)) {
+ Ref ref = repo.exactRef(RefNames.changeMetaRef(id));
+ assertThat(ref).isNotNull();
+ oldMetaId = ref.getObjectId();
+
+ Change c = db.changes().get(id);
+ assertThat(c.getTopic()).isNull();
+ rowVersion = c.getRowVersion();
+ NoteDbChangeState s = NoteDbChangeState.parse(c);
+ assertThat(s.getPrimaryStorage()).isEqualTo(PrimaryStorage.NOTE_DB);
+ assertThat(s.getRefState()).isEmpty();
+ }
+
+ // Do not open a new context, to simulate races with other threads that opened a context earlier
+ // in the migration process; this needs to work.
+ gApi.changes().id(id.get()).topic(name("a-topic"));
+
+ // Of course, it should also work with a new context.
+ resetCurrentApiUser();
+ gApi.changes().id(id.get()).topic(name("another-topic"));
+
+ try (ReviewDb db = schemaFactory.open();
+ Repository repo = repoManager.openRepository(project)) {
+ assertThat(repo.exactRef(RefNames.changeMetaRef(id)).getObjectId()).isNotEqualTo(oldMetaId);
+
+ Change c = db.changes().get(id);
+ assertThat(c.getTopic()).isNull();
+ assertThat(c.getRowVersion()).isEqualTo(rowVersion);
+ }
+ }
+
private void assertNotesMigrationState(NotesMigrationState expected) throws Exception {
assertThat(NotesMigrationState.forNotesMigration(notesMigration)).hasValue(expected);
gerritConfig.load();
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/MigrateToNoteDb.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/MigrateToNoteDb.java
index 9a1cc49..3cbf71a 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/MigrateToNoteDb.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/MigrateToNoteDb.java
@@ -37,6 +37,7 @@
import java.util.ArrayList;
import java.util.List;
import org.kohsuke.args4j.Option;
+import org.kohsuke.args4j.spi.ExplicitBooleanOptionHandler;
public class MigrateToNoteDb extends SiteProgram {
@Option(name = "--threads", usage = "Number of threads to use for rebuilding NoteDb")
@@ -70,10 +71,20 @@
name = "--trial",
usage =
"trial mode: migrate changes and turn on reading from NoteDb, but leave ReviewDb as"
- + " the source of truth"
+ + " the source of truth",
+ handler = ExplicitBooleanOptionHandler.class
)
private boolean trial = true; // TODO(dborowitz): Default to false in 3.0.
+ @Option(
+ name = "--sequence-gap",
+ usage =
+ "gap in change sequence numbers between last ReviewDb number and first NoteDb number;"
+ + " negative indicates using the value of noteDb.changes.initialSequenceGap (default"
+ + " 1000)"
+ )
+ private int sequenceGap;
+
private Injector dbInjector;
private Injector sysInjector;
private LifecycleManager dbManager;
@@ -108,6 +119,7 @@
.setChanges(changes.stream().map(Change.Id::new).collect(toList()))
.setTrialMode(trial)
.setForceRebuild(force)
+ .setSequenceGap(sequenceGap)
.build()) {
if (!projects.isEmpty() || !changes.isEmpty()) {
migrator.rebuild();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/Sequences.java b/gerrit-server/src/main/java/com/google/gerrit/server/Sequences.java
index e6d56ee..f1f0e6f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/Sequences.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/Sequences.java
@@ -37,6 +37,10 @@
public class Sequences {
public static final String CHANGES = "changes";
+ public static int getChangeSequenceGap(Config cfg) {
+ return cfg.getInt("noteDb", "changes", "initialSequenceGap", 1000);
+ }
+
private final Provider<ReviewDb> db;
private final NotesMigration migration;
private final RepoSequence changeSeq;
@@ -51,7 +55,7 @@
this.db = db;
this.migration = migration;
- int gap = cfg.getInt("noteDb", "changes", "initialSequenceGap", 0);
+ int gap = getChangeSequenceGap(cfg);
changeSeq =
new RepoSequence(
repoManager,
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
index a40f9cf..a866314 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
@@ -16,19 +16,21 @@
import static com.google.common.base.Preconditions.checkArgument;
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.NotesMigrationState.NOTE_DB_UNFUSED;
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;
import static com.google.gerrit.server.notedb.NotesMigrationState.WRITE;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Comparator.comparing;
+import static java.util.stream.Collectors.toList;
-import com.google.common.base.Predicates;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Stopwatch;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
-import com.google.common.collect.Iterables;
import com.google.common.collect.MultimapBuilder;
import com.google.common.collect.Ordering;
import com.google.common.collect.SetMultimap;
@@ -42,12 +44,18 @@
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.Sequences;
+import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.LockFailureException;
import com.google.gerrit.server.git.WorkQueue;
import com.google.gerrit.server.notedb.ConfigNotesMigration;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.notedb.NotesMigrationState;
+import com.google.gerrit.server.notedb.PrimaryStorageMigrator;
+import com.google.gerrit.server.notedb.RepoSequence;
import com.google.gerrit.server.notedb.rebuild.ChangeRebuilder.NoPatchSetsException;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.SchemaFactory;
@@ -65,6 +73,7 @@
import java.util.concurrent.TimeUnit;
import java.util.function.Predicate;
import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ProgressMonitor;
import org.eclipse.jgit.lib.TextProgressMonitor;
import org.eclipse.jgit.storage.file.FileBasedConfig;
@@ -78,31 +87,45 @@
private static final Logger log = LoggerFactory.getLogger(NoteDbMigrator.class);
public static class Builder {
+ private final Config cfg;
private final SitePaths sitePaths;
private final SchemaFactory<ReviewDb> schemaFactory;
+ private final GitRepositoryManager repoManager;
+ private final AllProjectsName allProjects;
private final ChangeRebuilder rebuilder;
private final WorkQueue workQueue;
private final NotesMigration globalNotesMigration;
+ private final PrimaryStorageMigrator primaryStorageMigrator;
private int threads;
private ImmutableList<Project.NameKey> projects = ImmutableList.of();
private ImmutableList<Change.Id> changes = ImmutableList.of();
private OutputStream progressOut = NullOutputStream.INSTANCE;
+ private NotesMigrationState stopAtState;
private boolean trial;
private boolean forceRebuild;
+ private int sequenceGap = -1;
@Inject
Builder(
+ @GerritServerConfig Config cfg,
SitePaths sitePaths,
SchemaFactory<ReviewDb> schemaFactory,
+ GitRepositoryManager repoManager,
+ AllProjectsName allProjects,
ChangeRebuilder rebuilder,
WorkQueue workQueue,
- NotesMigration globalNotesMigration) {
+ NotesMigration globalNotesMigration,
+ PrimaryStorageMigrator primaryStorageMigrator) {
+ this.cfg = cfg;
this.sitePaths = sitePaths;
this.schemaFactory = schemaFactory;
+ this.repoManager = repoManager;
+ this.allProjects = allProjects;
this.rebuilder = rebuilder;
this.workQueue = workQueue;
this.globalNotesMigration = globalNotesMigration;
+ this.primaryStorageMigrator = primaryStorageMigrator;
}
/**
@@ -166,6 +189,18 @@
}
/**
+ * Stop at a specific migration state, for testing only.
+ *
+ * @param stopAtState state to stop at.
+ * @return this.
+ */
+ @VisibleForTesting
+ public Builder setStopAtStateForTesting(NotesMigrationState stopAtState) {
+ this.stopAtState = stopAtState;
+ return this;
+ }
+
+ /**
* Rebuild in "trial mode": configure Gerrit to write to and read from NoteDb, but leave
* ReviewDb as the source of truth for all changes.
*
@@ -196,61 +231,108 @@
return this;
}
+ /**
+ * Gap between ReviewDb change sequence numbers and NoteDb.
+ *
+ * <p>If NoteDb sequences are enabled in a running server, there is a race between the migration
+ * step that calls {@code nextChangeId()} to seed the ref, and other threads that call {@code
+ * nextChangeId()} to create new changes. In order to prevent these operations stepping on one
+ * another, we use this value to skip some predefined sequence numbers. This is strongly
+ * recommended in a running server.
+ *
+ * <p>If the migration takes place offline, there is no race with other threads, and this option
+ * may be set to 0. However, admins may still choose to use a gap, for example to make it easier
+ * to distinguish changes that were created before and after the NoteDb migration.
+ *
+ * <p>By default, uses the value from {@code noteDb.changes.initialSequenceGap} in {@code
+ * gerrit.config}, which defaults to 1000.
+ *
+ * @param sequenceGap sequence gap size; if negative, use the default.
+ * @return this.
+ */
+ public Builder setSequenceGap(int sequenceGap) {
+ this.sequenceGap = sequenceGap;
+ return this;
+ }
+
public NoteDbMigrator build() throws MigrationException {
return new NoteDbMigrator(
sitePaths,
schemaFactory,
+ repoManager,
+ allProjects,
rebuilder,
globalNotesMigration,
+ primaryStorageMigrator,
threads > 1
? MoreExecutors.listeningDecorator(workQueue.createQueue(threads, "RebuildChange"))
: MoreExecutors.newDirectExecutorService(),
projects,
changes,
progressOut,
+ stopAtState,
trial,
- forceRebuild);
+ forceRebuild,
+ sequenceGap >= 0 ? sequenceGap : Sequences.getChangeSequenceGap(cfg));
}
}
private final FileBasedConfig gerritConfig;
private final SchemaFactory<ReviewDb> schemaFactory;
+ private final GitRepositoryManager repoManager;
+ private final AllProjectsName allProjects;
private final ChangeRebuilder rebuilder;
private final NotesMigration globalNotesMigration;
+ private final PrimaryStorageMigrator primaryStorageMigrator;
private final ListeningExecutorService executor;
private final ImmutableList<Project.NameKey> projects;
private final ImmutableList<Change.Id> changes;
private final OutputStream progressOut;
+ private final NotesMigrationState stopAtState;
private final boolean trial;
private final boolean forceRebuild;
+ private final int sequenceGap;
private NoteDbMigrator(
SitePaths sitePaths,
SchemaFactory<ReviewDb> schemaFactory,
+ GitRepositoryManager repoManager,
+ AllProjectsName allProjects,
ChangeRebuilder rebuilder,
NotesMigration globalNotesMigration,
+ PrimaryStorageMigrator primaryStorageMigrator,
ListeningExecutorService executor,
ImmutableList<Project.NameKey> projects,
ImmutableList<Change.Id> changes,
OutputStream progressOut,
+ NotesMigrationState stopAtState,
boolean trial,
- boolean forceRebuild)
+ boolean forceRebuild,
+ int sequenceGap)
throws MigrationException {
if (!changes.isEmpty() && !projects.isEmpty()) {
throw new MigrationException("Cannot set both changes and projects");
}
+ if (sequenceGap < 0) {
+ throw new MigrationException("Sequence gap must be non-negative: " + sequenceGap);
+ }
this.schemaFactory = schemaFactory;
this.rebuilder = rebuilder;
+ this.repoManager = repoManager;
+ this.allProjects = allProjects;
this.globalNotesMigration = globalNotesMigration;
+ this.primaryStorageMigrator = primaryStorageMigrator;
this.gerritConfig = new FileBasedConfig(sitePaths.gerrit_config.toFile(), FS.detect());
this.executor = executor;
this.projects = projects;
this.changes = changes;
this.progressOut = progressOut;
+ this.stopAtState = stopAtState;
this.trial = trial;
this.forceRebuild = forceRebuild;
+ this.sequenceGap = sequenceGap;
}
@Override
@@ -281,6 +363,9 @@
boolean rebuilt = false;
while (state.compareTo(NOTE_DB_UNFUSED) < 0) {
+ if (state.equals(stopAtState)) {
+ return;
+ }
boolean stillNeedsRebuild = forceRebuild && !rebuilt;
if (trial && state.compareTo(READ_WRITE_NO_SEQUENCE) >= 0) {
if (stillNeedsRebuild && state == READ_WRITE_NO_SEQUENCE) {
@@ -303,7 +388,7 @@
state = rebuildAndEnableReads(state);
rebuilt = true;
} else {
- state = enableSequences();
+ state = enableSequences(state);
}
break;
case READ_WRITE_WITH_SEQUENCE_REVIEW_DB_PRIMARY:
@@ -311,11 +396,16 @@
state = rebuildAndEnableReads(state);
rebuilt = true;
} else {
- state = setNoteDbPrimary();
+ state = setNoteDbPrimary(state);
}
break;
case READ_WRITE_WITH_SEQUENCE_NOTE_DB_PRIMARY:
- state = disableReviewDb();
+ // 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
+ // 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:
// Done!
@@ -340,16 +430,80 @@
return saveState(prev, READ_WRITE_NO_SEQUENCE);
}
- private NotesMigrationState enableSequences() {
- throw new UnsupportedOperationException("not yet implemented");
+ private NotesMigrationState enableSequences(NotesMigrationState prev)
+ throws OrmException, IOException {
+ try (ReviewDb db = schemaFactory.open()) {
+ @SuppressWarnings("deprecation")
+ RepoSequence seq =
+ new RepoSequence(
+ repoManager,
+ allProjects,
+ Sequences.CHANGES,
+ // If sequenceGap is 0, this writes into the sequence ref the same ID that is returned
+ // by the call to seq.next() below. If we actually used this as a change ID, that
+ // would be a problem, but we just discard it, so this is safe.
+ () -> db.nextChangeId() + sequenceGap - 1,
+ 1);
+ seq.next();
+ }
+ return saveState(prev, READ_WRITE_WITH_SEQUENCE_REVIEW_DB_PRIMARY);
}
- private NotesMigrationState setNoteDbPrimary() {
- throw new UnsupportedOperationException("not yet implemented");
+ private NotesMigrationState setNoteDbPrimary(NotesMigrationState prev)
+ throws MigrationException, OrmException, IOException {
+ checkState(
+ projects.isEmpty() && changes.isEmpty(),
+ "Should not have attempted setNoteDbPrimary with a subset of changes");
+ checkState(
+ prev == READ_WRITE_WITH_SEQUENCE_REVIEW_DB_PRIMARY
+ || prev == READ_WRITE_WITH_SEQUENCE_NOTE_DB_PRIMARY,
+ "Unexpected start state for setNoteDbPrimary: %s",
+ prev);
+
+ // Before changing the primary storage of old changes, ensure new changes are created with
+ // NoteDb primary.
+ prev = saveState(prev, READ_WRITE_WITH_SEQUENCE_NOTE_DB_PRIMARY);
+
+ Stopwatch sw = Stopwatch.createStarted();
+ log.info("Setting primary storage to NoteDb");
+ List<Change.Id> allChanges;
+ try (ReviewDb db = unwrapDb(schemaFactory.open())) {
+ allChanges = Streams.stream(db.changes().all()).map(Change::getId).collect(toList());
+ }
+
+ List<ListenableFuture<Boolean>> futures =
+ allChanges
+ .stream()
+ .map(
+ id ->
+ executor.submit(
+ () -> {
+ // TODO(dborowitz): Avoid reopening db if using a single thread.
+ try (ReviewDb db = unwrapDb(schemaFactory.open())) {
+ primaryStorageMigrator.migrateToNoteDbPrimary(id);
+ return true;
+ } catch (Exception e) {
+ log.error("Error migrating primary storage for " + id, e);
+ return false;
+ }
+ }))
+ .collect(toList());
+
+ boolean ok = futuresToBoolean(futures, "Error migrating primary storage");
+ double t = sw.elapsed(TimeUnit.MILLISECONDS) / 1000d;
+ log.info(
+ String.format(
+ "Migrated primary storage of %d changes in %.01fs (%.01f/s)\n",
+ allChanges.size(), t, allChanges.size() / t));
+ if (!ok) {
+ throw new MigrationException("Migrating primary storage for some changes failed, see log");
+ }
+
+ return disableReviewDb(prev);
}
- private NotesMigrationState disableReviewDb() {
- throw new UnsupportedOperationException("not yet implemented");
+ private NotesMigrationState disableReviewDb(NotesMigrationState prev) throws IOException {
+ return saveState(prev, NOTE_DB_UNFUSED);
}
private Optional<NotesMigrationState> loadState() throws IOException {
@@ -394,7 +548,6 @@
if (!globalNotesMigration.commitChangeWrites()) {
throw new MigrationException("Cannot rebuild without noteDb.changes.write=true");
}
- boolean ok;
Stopwatch sw = Stopwatch.createStarted();
log.info("Rebuilding changes in NoteDb");
@@ -416,13 +569,7 @@
futures.add(future);
}
- try {
- ok = Iterables.all(Futures.allAsList(futures).get(), Predicates.equalTo(true));
- } catch (InterruptedException | ExecutionException e) {
- log.error("Error rebuilding projects", e);
- ok = false;
- }
-
+ boolean ok = futuresToBoolean(futures, "Error rebuilding projects");
double t = sw.elapsed(TimeUnit.MILLISECONDS) / 1000d;
log.info(
String.format(
@@ -502,4 +649,13 @@
}
return ok;
}
+
+ private static boolean futuresToBoolean(List<ListenableFuture<Boolean>> futures, String errMsg) {
+ try {
+ return Futures.allAsList(futures).get().stream().allMatch(b -> b);
+ } catch (InterruptedException | ExecutionException e) {
+ log.error(errMsg, e);
+ return false;
+ }
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/update/FusedNoteDbBatchUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/update/FusedNoteDbBatchUpdate.java
index b979636..7db5e43 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/update/FusedNoteDbBatchUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/update/FusedNoteDbBatchUpdate.java
@@ -286,7 +286,6 @@
@Assisted CurrentUser user,
@Assisted Timestamp when) {
super(repoManager, serverIdent, project, user, when);
- checkArgument(!db.changesTablesEnabled(), "expected Change tables to be disabled on %s", db);
this.changeNotesFactory = changeNotesFactory;
this.changeControlFactory = changeControlFactory;
this.changeUpdateFactory = changeUpdateFactory;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/update/UnfusedNoteDbBatchUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/update/UnfusedNoteDbBatchUpdate.java
index b5c7256..ce96c0e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/update/UnfusedNoteDbBatchUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/update/UnfusedNoteDbBatchUpdate.java
@@ -14,7 +14,6 @@
package com.google.gerrit.server.update;
-import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static java.util.Comparator.comparing;
import static java.util.stream.Collectors.toList;
@@ -267,7 +266,6 @@
@Assisted CurrentUser user,
@Assisted Timestamp when) {
super(repoManager, serverIdent, project, user, when);
- checkArgument(!db.changesTablesEnabled(), "expected Change tables to be disabled on %s", db);
this.changeNotesFactory = changeNotesFactory;
this.changeControlFactory = changeControlFactory;
this.changeUpdateFactory = changeUpdateFactory;
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/RepoSequenceTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/RepoSequenceTest.java
index 4b302b7..66ccdad 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/RepoSequenceTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/RepoSequenceTest.java
@@ -264,13 +264,7 @@
Runnable afterReadRef,
Retryer<RefUpdate.Result> retryer) {
return new RepoSequence(
- repoManager,
- project,
- name,
- () -> start,
- batchSize,
- afterReadRef,
- retryer);
+ repoManager, project, name, () -> start, batchSize, afterReadRef, retryer);
}
private ObjectId writeBlob(String sequenceName, String value) {
