diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/pgm/MigrateToNoteDbIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/pgm/MigrateToNoteDbIT.java deleted file mode 100644 index 18217ebe03..0000000000 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/pgm/MigrateToNoteDbIT.java +++ /dev/null @@ -1,89 +0,0 @@ -// Copyright (C) 2014 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.pgm; - -import static com.google.common.truth.Truth.assertThat; -import static com.google.common.truth.Truth8.assertThat; - -import com.google.gerrit.launcher.GerritLauncher; -import com.google.gerrit.server.config.SitePaths; -import com.google.gerrit.server.notedb.ConfigNotesMigration; -import com.google.gerrit.server.notedb.NotesMigrationState; -import com.google.gerrit.testutil.TempFileUtil; -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.Test; - -public class MigrateToNoteDbIT { - private String sitePath; - private StoredConfig gerritConfig; - - @Before - public void setUp() throws Exception { - SitePaths sitePaths = new SitePaths(TempFileUtil.createTempDirectory().toPath()); - sitePath = sitePaths.site_path.toString(); - gerritConfig = new FileBasedConfig(sitePaths.gerrit_config.toFile(), FS.detect()); - initSite(); - } - - @After - public void tearDown() throws Exception { - TempFileUtil.cleanup(); - } - - @Test - public void rebuildEmptySiteStartingWithNoteDbDisabed() throws Exception { - assertNotesMigrationState(NotesMigrationState.REVIEW_DB); - runGerrit("MigrateToNoteDb", "-d", sitePath, "--show-stack-trace"); - assertNotesMigrationState(NotesMigrationState.READ_WRITE_NO_SEQUENCE); - } - - @Test - public void rebuildEmptySiteStartingWithNoteDbEnabled() throws Exception { - setNotesMigrationState(NotesMigrationState.READ_WRITE_NO_SEQUENCE); - runGerrit("MigrateToNoteDb", "-d", sitePath, "--show-stack-trace"); - assertNotesMigrationState(NotesMigrationState.READ_WRITE_NO_SEQUENCE); - } - - private void initSite() throws Exception { - runGerrit( - "init", - "-d", - sitePath, - "--batch", - "--no-auto-start", - "--skip-plugins", - "--show-stack-trace"); - } - - private static void runGerrit(String... args) throws Exception { - assertThat(GerritLauncher.mainImpl(args)).isEqualTo(0); - } - - private void setNotesMigrationState(NotesMigrationState state) throws Exception { - gerritConfig.load(); - ConfigNotesMigration.setConfigValues(gerritConfig, state.migration()); - gerritConfig.save(); - } - - private void assertNotesMigrationState(NotesMigrationState expected) throws Exception { - gerritConfig.load(); - assertThat(NotesMigrationState.forNotesMigration(new ConfigNotesMigration(gerritConfig))) - .hasValue(expected); - } -} 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 new file mode 100644 index 0000000000..1676f3ed4c --- /dev/null +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/pgm/OfflineNoteDbMigrationIT.java @@ -0,0 +1,190 @@ +// Copyright (C) 2014 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.pgm; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth8.assertThat; + +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.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.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 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. + * + *

Note: These tests are very slow due to the repeated daemon startup. Prefer + * 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; + private StoredConfig gerritConfig; + + @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); + } + + @Test + public void rebuildOneChangeTrialMode() throws Exception { + assertNotesMigrationState(NotesMigrationState.REVIEW_DB); + Project.NameKey project = new Project.NameKey("project"); + + 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"); + assertNotesMigrationState(NotesMigrationState.READ_WRITE_NO_SEQUENCE); + + try (GerritServer server = startServer(); + ManualRequestContext ctx = openContext(server, accountId)) { + GitRepositoryManager repoManager = + server.getTestInjector().getInstance(GitRepositoryManager.class); + ObjectId metaId; + try (Repository repo = repoManager.openRepository(project)) { + Ref ref = repo.exactRef(RefNames.changeMetaRef(id)); + 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())); + } + } + + private GerritServer startServer() throws Exception { + return GerritServer.start(serverDesc, new Config(), site); + } + + private ManualRequestContext openContext(GerritServer server) throws Exception { + Injector i = server.getTestInjector(); + TestAccount a = i.getInstance(AccountCreator.class).admin(); + return openContext(server, a.getId()); + } + + private ManualRequestContext openContext(GerritServer server, Account.Id accountId) + throws Exception { + return server.getTestInjector().getInstance(OneOffRequestContext.class).openAs(accountId); + } + + 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(); + } + + private void assertNotesMigrationState(NotesMigrationState expected) throws Exception { + gerritConfig.load(); + assertThat(NotesMigrationState.forNotesMigration(new ConfigNotesMigration(gerritConfig))) + .hasValue(expected); + } +} 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 new file mode 100644 index 0000000000..f7331b88d6 --- /dev/null +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java @@ -0,0 +1,82 @@ +// 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.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 com.google.common.collect.ImmutableMap; +import com.google.gerrit.acceptance.AbstractDaemonTest; +import com.google.gerrit.acceptance.NoHttpd; +import com.google.gerrit.acceptance.PushOneCommit; +import com.google.gerrit.acceptance.Sandboxed; +import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.RefNames; +import com.google.gerrit.reviewdb.server.ReviewDbUtil; +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.notedb.rebuild.NoteDbMigrator; +import com.google.gerrit.testutil.NoteDbMode; +import com.google.inject.Inject; +import com.google.inject.Provider; +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.Ref; +import org.eclipse.jgit.lib.Repository; +import org.junit.Before; +import org.junit.Test; + +@Sandboxed +@NoHttpd +public class OnlineNoteDbMigrationIT extends AbstractDaemonTest { + @Inject private Provider migratorBuilderProvider; + + @Before + public void setUp() { + assume().that(NoteDbMode.get()).isEqualTo(NoteDbMode.OFF); + assertNotesMigrationState(NotesMigrationState.REVIEW_DB); + } + + @Test + public void rebuildOneChangeTrialMode() throws Exception { + PushOneCommit.Result r = createChange(); + Change.Id id = r.getChange().getId(); + + try (NoteDbMigrator migrator = migratorBuilderProvider.get().setTrialMode(true).build()) { + migrator.migrate(); + } + assertNotesMigrationState(NotesMigrationState.READ_WRITE_NO_SEQUENCE); + + ObjectId metaId; + try (Repository repo = repoManager.openRepository(project)) { + Ref ref = repo.exactRef(RefNames.changeMetaRef(id)); + assertThat(ref).isNotNull(); + metaId = ref.getObjectId(); + } + + Change c = ReviewDbUtil.unwrapDb(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())); + } + + private void assertNotesMigrationState(NotesMigrationState expected) { + assertThat(NotesMigrationState.forNotesMigration(notesMigration)).hasValue(expected); + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java index e88554a6dd..9c7f3bdbca 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java @@ -247,17 +247,17 @@ public class ChangeRebuilderImpl extends ChangeRebuilder { throw new AbortUpdateException(); } else if (!Objects.equals(oldNoteDbState, currNoteDbState)) { // Another thread updated the state to something else. - throw new ConflictingUpdateException(change, oldNoteDbState); + throw new ConflictingUpdateRuntimeException(change, oldNoteDbState); } change.setNoteDbState(newNoteDbState); return change; } }); - } catch (ConflictingUpdateException e) { + } catch (ConflictingUpdateRuntimeException e) { // Rethrow as an OrmException so the caller knows to use staged results. Strictly speaking // they are not completely up to date, but result we send to the caller is the same as if this // rebuild had executed before the other thread. - throw new OrmException(e.getMessage()); + throw new ConflictingUpdateException(e); } catch (AbortUpdateException e) { if (NoteDbChangeState.parse(changeId, newNoteDbState) .isUpToDate( diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ConflictingUpdateException.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ConflictingUpdateException.java index c6ffffc81f..d8e7480fdf 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ConflictingUpdateException.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ConflictingUpdateException.java @@ -1,4 +1,4 @@ -// Copyright (C) 2016 The Android Open Source Project +// 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. @@ -14,16 +14,19 @@ package com.google.gerrit.server.notedb.rebuild; -import com.google.gerrit.reviewdb.client.Change; -import com.google.gwtorm.server.OrmRuntimeException; +import com.google.gwtorm.server.OrmException; -class ConflictingUpdateException extends OrmRuntimeException { +/** + * {@link com.google.gwtorm.server.OrmException} thrown by {@link ChangeRebuilder} when rebuilding a + * change failed because another operation modified its {@link + * com.google.gerrit.server.notedb.NoteDbChangeState}. + */ +public class ConflictingUpdateException extends OrmException { private static final long serialVersionUID = 1L; - ConflictingUpdateException(Change change, String expectedNoteDbState) { - super( - String.format( - "Expected change %s to have noteDbState %s but was %s", - change.getId(), expectedNoteDbState, change.getNoteDbState())); + // Always created from a ConflictingUpdateRuntimeException because it originates from an + // AtomicUpdate, which cannot throw checked exceptions. + ConflictingUpdateException(ConflictingUpdateRuntimeException cause) { + super(cause.getMessage(), cause); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ConflictingUpdateRuntimeException.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ConflictingUpdateRuntimeException.java new file mode 100644 index 0000000000..abfafa2041 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ConflictingUpdateRuntimeException.java @@ -0,0 +1,29 @@ +// Copyright (C) 2016 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.notedb.rebuild; + +import com.google.gerrit.reviewdb.client.Change; +import com.google.gwtorm.server.OrmRuntimeException; + +class ConflictingUpdateRuntimeException extends OrmRuntimeException { + private static final long serialVersionUID = 1L; + + ConflictingUpdateRuntimeException(Change change, String expectedNoteDbState) { + super( + String.format( + "Expected change %s to have noteDbState %s but was %s", + change.getId(), expectedNoteDbState, change.getNoteDbState())); + } +} 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 97fecb72aa..8d43e0b5ad 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 @@ -44,10 +44,9 @@ 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.config.SitePaths; +import com.google.gerrit.server.git.LockFailureException; import com.google.gerrit.server.git.WorkQueue; -import com.google.gerrit.server.notedb.ChangeBundleReader; import com.google.gerrit.server.notedb.ConfigNotesMigration; -import com.google.gerrit.server.notedb.NoteDbUpdateManager; import com.google.gerrit.server.notedb.NotesMigration; import com.google.gerrit.server.notedb.NotesMigrationState; import com.google.gerrit.server.notedb.rebuild.ChangeRebuilder.NoPatchSetsException; @@ -82,9 +81,7 @@ public class NoteDbMigrator implements AutoCloseable { public static class Builder { private final SitePaths sitePaths; private final SchemaFactory schemaFactory; - private final NoteDbUpdateManager.Factory updateManagerFactory; private final ChangeRebuilder rebuilder; - private final ChangeBundleReader bundleReader; private final WorkQueue workQueue; private final NotesMigration globalNotesMigration; @@ -99,16 +96,12 @@ public class NoteDbMigrator implements AutoCloseable { Builder( SitePaths sitePaths, SchemaFactory schemaFactory, - NoteDbUpdateManager.Factory updateManagerFactory, ChangeRebuilder rebuilder, - ChangeBundleReader bundleReader, WorkQueue workQueue, NotesMigration globalNotesMigration) { this.sitePaths = sitePaths; this.schemaFactory = schemaFactory; - this.updateManagerFactory = updateManagerFactory; this.rebuilder = rebuilder; - this.bundleReader = bundleReader; this.workQueue = workQueue; this.globalNotesMigration = globalNotesMigration; } @@ -208,9 +201,7 @@ public class NoteDbMigrator implements AutoCloseable { return new NoteDbMigrator( sitePaths, schemaFactory, - updateManagerFactory, rebuilder, - bundleReader, globalNotesMigration, threads > 1 ? MoreExecutors.listeningDecorator(workQueue.createQueue(threads, "RebuildChange")) @@ -225,9 +216,7 @@ public class NoteDbMigrator implements AutoCloseable { private final FileBasedConfig gerritConfig; private final SchemaFactory schemaFactory; - private final NoteDbUpdateManager.Factory updateManagerFactory; private final ChangeRebuilder rebuilder; - private final ChangeBundleReader bundleReader; private final NotesMigration globalNotesMigration; private final ListeningExecutorService executor; @@ -240,9 +229,7 @@ public class NoteDbMigrator implements AutoCloseable { private NoteDbMigrator( SitePaths sitePaths, SchemaFactory schemaFactory, - NoteDbUpdateManager.Factory updateManagerFactory, ChangeRebuilder rebuilder, - ChangeBundleReader bundleReader, NotesMigration globalNotesMigration, ListeningExecutorService executor, ImmutableList projects, @@ -251,9 +238,7 @@ public class NoteDbMigrator implements AutoCloseable { boolean trial, boolean forceRebuild) { this.schemaFactory = schemaFactory; - this.updateManagerFactory = updateManagerFactory; this.rebuilder = rebuilder; - this.bundleReader = bundleReader; this.globalNotesMigration = globalNotesMigration; boolean hasChanges = !changes.isEmpty(); @@ -399,6 +384,9 @@ public class NoteDbMigrator implements AutoCloseable { } public void rebuild() throws MigrationException, OrmException { + checkState( + globalNotesMigration.commitChangeWrites(), + "cannot rebuild without noteDb.changes.write=true"); boolean ok; Stopwatch sw = Stopwatch.createStarted(); log.info("Rebuilding changes in NoteDb"); @@ -468,27 +456,40 @@ public class NoteDbMigrator implements AutoCloseable { private boolean rebuildProject( ReviewDb db, ImmutableListMultimap allChanges, - Project.NameKey project) - throws IOException, OrmException { + Project.NameKey project) { checkArgument(allChanges.containsKey(project)); boolean ok = true; ProgressMonitor pm = new TextProgressMonitor( new PrintWriter(new BufferedWriter(new OutputStreamWriter(progressOut, UTF_8)))); pm.beginTask(FormatUtil.elide(project.get(), 50), allChanges.get(project).size()); - try (NoteDbUpdateManager manager = updateManagerFactory.create(project)) { + try { for (Change.Id changeId : allChanges.get(project)) { + // Update one change at a time, which ends up creating one NoteDbUpdateManager per change as + // well. This turns out to be no more expensive than batching, since each NoteDb operation + // is only writing single loose ref updates and loose objects. Plus we have to do one + // ReviewDb transaction per change due to the AtomicUpdate, so if we somehow batched NoteDb + // operations, ReviewDb would become the bottleneck. try { - rebuilder.buildUpdates(manager, bundleReader.fromReviewDb(db, changeId)); + rebuilder.rebuild(db, changeId); } catch (NoPatchSetsException e) { log.warn(e.getMessage()); + } catch (ConflictingUpdateException e) { + log.warn( + "Rebuilding detected a conflicting ReviewDb update for change {};" + + " will be auto-rebuilt at runtime", + changeId); + } catch (LockFailureException e) { + log.warn( + "Rebuilding detected a conflicting NoteDb update for change {};" + + " will be auto-rebuilt at runtime", + changeId); } catch (Throwable t) { log.error("Failed to rebuild change " + changeId, t); ok = false; } pm.update(1); } - manager.execute(); } finally { pm.endTask(); } diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/ConfigSuite.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/ConfigSuite.java index 97025c8e25..7a4b2b05ec 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/testutil/ConfigSuite.java +++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/ConfigSuite.java @@ -104,7 +104,7 @@ import org.junit.runners.model.InitializationError; * field annotated with {@code @ConfigSuite.Name}. */ public class ConfigSuite extends Suite { - private static final String DEFAULT = "default"; + public static final String DEFAULT = "default"; @Target({METHOD}) @Retention(RUNTIME) diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/TestNotesMigration.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/TestNotesMigration.java index c42fffa489..a89135898b 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/testutil/TestNotesMigration.java +++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/TestNotesMigration.java @@ -25,6 +25,7 @@ import com.google.inject.Singleton; public class TestNotesMigration extends NotesMigration { private volatile boolean readChanges; private volatile boolean writeChanges; + private volatile boolean readChangeSequence; private volatile PrimaryStorage changePrimaryStorage = PrimaryStorage.REVIEW_DB; private volatile boolean disableChangeReviewDb; private volatile boolean fuseUpdates; @@ -41,9 +42,7 @@ public class TestNotesMigration extends NotesMigration { @Override public boolean readChangeSequence() { - // Unlike ConfigNotesMigration, read change numbers from NoteDb by default - // when reads are enabled, to improve test coverage. - return readChanges; + return readChangeSequence; } @Override @@ -83,6 +82,11 @@ public class TestNotesMigration extends NotesMigration { return this; } + public TestNotesMigration setReadChangeSequence(boolean readChangeSequence) { + this.readChangeSequence = readChangeSequence; + return this; + } + public TestNotesMigration setChangePrimaryStorage(PrimaryStorage changePrimaryStorage) { this.changePrimaryStorage = checkNotNull(changePrimaryStorage); return this; @@ -115,6 +119,7 @@ public class TestNotesMigration extends NotesMigration { public TestNotesMigration setFrom(NotesMigration other) { setWriteChanges(other.rawWriteChangesSetting()); setReadChanges(other.readChanges()); + setReadChangeSequence(other.readChangeSequence()); setChangePrimaryStorage(other.changePrimaryStorage()); setDisableChangeReviewDb(other.disableChangeReviewDb()); setFuseUpdates(other.fuseUpdates());