Merge changes Idb8e5b66,Ic5087e36,I0a7def4d,Iea89f3bf,Ic324a17e

* changes:
  Extract a StandaloneSiteTest for testing offline programs
  Add more offline NoteDb migration tests
  Finish NoteDbMigrator
  NoteDbMigrator: Migrate to read change IDs from All-Projects
  Format RepoSequenceTest
This commit is contained in:
David Ostrovsky
2017-06-24 04:36:03 +00:00
committed by Gerrit Code Review
12 changed files with 580 additions and 186 deletions

View File

@@ -61,6 +61,14 @@ import org.eclipse.jgit.lib.RepositoryCache;
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 @@ public class GerritServer implements AutoCloseable {
}
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);

View File

@@ -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));
}
}

View File

@@ -17,48 +17,36 @@ package com.google.gerrit.acceptance.pgm;
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 @@ import org.junit.runner.Description;
* 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 @@ public class OfflineNoteDbMigrationIT {
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();
}
}

View File

@@ -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");
}
}

View File

@@ -122,6 +122,10 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
// want precise control over when auto-rebuilding happens.
cfg.setBoolean("index", null, "testAutoReindexIfStale", 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;
}

View File

@@ -17,10 +17,18 @@ 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.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.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.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.NotesMigrationState;
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 @@ import org.junit.Test;
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 class OnlineNoteDbMigrationIT extends AbstractDaemonTest {
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,19 +112,26 @@ public class OnlineNoteDbMigrationIT extends AbstractDaemonTest {
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),
NoteDbMigrator::migrate);
}
@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();
@@ -110,7 +140,7 @@ public class OnlineNoteDbMigrationIT extends AbstractDaemonTest {
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 @@ public class OnlineNoteDbMigrationIT extends AbstractDaemonTest {
}
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 @@ public class OnlineNoteDbMigrationIT extends AbstractDaemonTest {
}
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 @@ public class OnlineNoteDbMigrationIT extends AbstractDaemonTest {
@Test
public void rebuildSubsetOfChanges() throws Exception {
setNotesMigrationState(NotesMigrationState.WRITE);
setNotesMigrationState(WRITE);
PushOneCommit.Result r1 = createChange();
PushOneCommit.Result r2 = createChange();
@@ -191,7 +221,7 @@ public class OnlineNoteDbMigrationIT extends AbstractDaemonTest {
@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 @@ public class OnlineNoteDbMigrationIT extends AbstractDaemonTest {
}
}
@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();

View File

@@ -37,6 +37,7 @@ import com.google.inject.Provider;
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 @@ public class MigrateToNoteDb extends SiteProgram {
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 @@ public class MigrateToNoteDb extends SiteProgram {
.setChanges(changes.stream().map(Change.Id::new).collect(toList()))
.setTrialMode(trial)
.setForceRebuild(force)
.setSequenceGap(sequenceGap)
.build()) {
if (!projects.isEmpty() || !changes.isEmpty()) {
migrator.rebuild();

View File

@@ -37,6 +37,10 @@ import org.eclipse.jgit.lib.Config;
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 @@ public class Sequences {
this.db = db;
this.migration = migration;
int gap = cfg.getInt("noteDb", "changes", "initialSequenceGap", 0);
int gap = getChangeSequenceGap(cfg);
changeSeq =
new RepoSequence(
repoManager,

View File

@@ -16,19 +16,21 @@ package com.google.gerrit.server.notedb.rebuild;
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.common.Nullable;
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.ExecutionException;
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 @@ public class NoteDbMigrator implements AutoCloseable {
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;
}
/**
@@ -165,6 +188,18 @@ public class NoteDbMigrator implements AutoCloseable {
return this;
}
/**
* 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 @@ public class NoteDbMigrator implements AutoCloseable {
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 @@ public class NoteDbMigrator implements AutoCloseable {
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 @@ public class NoteDbMigrator implements AutoCloseable {
state = rebuildAndEnableReads(state);
rebuilt = true;
} else {
state = enableSequences();
state = enableSequences(state);
}
break;
case READ_WRITE_WITH_SEQUENCE_REVIEW_DB_PRIMARY:
@@ -311,11 +396,16 @@ public class NoteDbMigrator implements AutoCloseable {
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 @@ public class NoteDbMigrator implements AutoCloseable {
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 @@ public class NoteDbMigrator implements AutoCloseable {
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 @@ public class NoteDbMigrator implements AutoCloseable {
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 @@ public class NoteDbMigrator implements AutoCloseable {
}
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;
}
}
}

View File

@@ -286,7 +286,6 @@ class FusedNoteDbBatchUpdate extends BatchUpdate {
@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;

View File

@@ -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 @@ class UnfusedNoteDbBatchUpdate extends BatchUpdate {
@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;

View File

@@ -264,13 +264,7 @@ public class RepoSequenceTest {
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) {