Ensure 2.16 upgrade is run before 3.x upgrade
Before upgrading to 3.x, we require that the admin at least run init
once to upgrade to 2.16. Because 3.x contains no ReviewDb code, it can't
possibly run the schema migrations included in 2.16.
Change-Id: I5b20b31dbd309219e23945a44ab4c1f69247f66f
diff --git a/java/com/google/gerrit/server/notedb/NotesMigration.java b/java/com/google/gerrit/server/notedb/NotesMigration.java
index 9cee2cd..c7acaf1 100644
--- a/java/com/google/gerrit/server/notedb/NotesMigration.java
+++ b/java/com/google/gerrit/server/notedb/NotesMigration.java
@@ -57,9 +57,8 @@
public static final String READ = "read";
public static final String WRITE = "write";
public static final String DISABLE_REVIEW_DB = "disableReviewDb";
-
- private static final String PRIMARY_STORAGE = "primaryStorage";
- private static final String SEQUENCE = "sequence";
+ public static final String PRIMARY_STORAGE = "primaryStorage";
+ public static final String SEQUENCE = "sequence";
public static class Module extends AbstractModule {
@Override
diff --git a/java/com/google/gerrit/server/schema/NoteDbSchemaUpdater.java b/java/com/google/gerrit/server/schema/NoteDbSchemaUpdater.java
index e671d68..436b67f 100644
--- a/java/com/google/gerrit/server/schema/NoteDbSchemaUpdater.java
+++ b/java/com/google/gerrit/server/schema/NoteDbSchemaUpdater.java
@@ -15,18 +15,36 @@
package com.google.gerrit.server.schema;
import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.gerrit.server.notedb.NoteDbTable.CHANGES;
+import static com.google.gerrit.server.notedb.NotesMigration.DISABLE_REVIEW_DB;
+import static com.google.gerrit.server.notedb.NotesMigration.PRIMARY_STORAGE;
+import static com.google.gerrit.server.notedb.NotesMigration.READ;
+import static com.google.gerrit.server.notedb.NotesMigration.SECTION_NOTE_DB;
+import static com.google.gerrit.server.notedb.NotesMigration.WRITE;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSortedMap;
import com.google.common.collect.ImmutableSortedSet;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.Sequences;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.notedb.NoteDbSchemaVersionManager;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
+import java.io.IOException;
import java.util.stream.IntStream;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.Repository;
public class NoteDbSchemaUpdater {
+ private final Config cfg;
+ private final AllUsersName allUsersName;
+ private final GitRepositoryManager repoManager;
private final NotesMigration notesMigration;
private final NoteDbSchemaVersionManager versionManager;
private final NoteDbSchemaVersion.Arguments args;
@@ -34,17 +52,33 @@
@Inject
NoteDbSchemaUpdater(
+ @GerritServerConfig Config cfg,
NotesMigration notesMigration,
NoteDbSchemaVersionManager versionManager,
- NoteDbSchemaVersion.Arguments args) {
- this(notesMigration, versionManager, args, NoteDbSchemaVersions.ALL);
+ NoteDbSchemaVersion.Arguments args,
+ GitRepositoryManager repoManager,
+ AllUsersName allUsersName) {
+ this(
+ cfg,
+ allUsersName,
+ repoManager,
+ notesMigration,
+ versionManager,
+ args,
+ NoteDbSchemaVersions.ALL);
}
NoteDbSchemaUpdater(
+ Config cfg,
+ AllUsersName allUsersName,
+ GitRepositoryManager repoManager,
NotesMigration notesMigration,
NoteDbSchemaVersionManager versionManager,
NoteDbSchemaVersion.Arguments args,
ImmutableSortedMap<Integer, Class<? extends NoteDbSchemaVersion>> schemaVersions) {
+ this.cfg = cfg;
+ this.allUsersName = allUsersName;
+ this.repoManager = repoManager;
this.notesMigration = notesMigration;
this.versionManager = versionManager;
this.args = args;
@@ -57,7 +91,15 @@
// only option.
return;
}
- for (int nextVersion : requiredUpgrades(versionManager.read(), schemaVersions.keySet())) {
+ int currentVersion = versionManager.read();
+ if (currentVersion == 0) {
+ // The only valid case where there is no refs/meta/version is when running 3.x init for the
+ // first time on a site that previously ran init on 2.16. A freshly created 3.x site will have
+ // seeded refs/meta/version during AllProjectsCreator, so it won't hit this block.
+ checkNoteDbConfigFor216();
+ }
+
+ for (int nextVersion : requiredUpgrades(currentVersion, schemaVersions.keySet())) {
try {
ui.message(String.format("Migrating data to schema %d ...", nextVersion));
NoteDbSchemaVersions.get(schemaVersions, nextVersion, args).upgrade(ui);
@@ -69,6 +111,49 @@
}
}
+ private void checkNoteDbConfigFor216() throws OrmException {
+ // Check that the NoteDb migration config matches what we expect from a site that both:
+ // * Completed the change migration to NoteDB.
+ // * Ran schema upgrades from a 2.16 final release.
+
+ if (!cfg.getBoolean(SECTION_NOTE_DB, CHANGES.key(), WRITE, false)
+ || !cfg.getBoolean(SECTION_NOTE_DB, CHANGES.key(), READ, false)
+ || cfg.getEnum(SECTION_NOTE_DB, CHANGES.key(), PRIMARY_STORAGE, PrimaryStorage.REVIEW_DB)
+ != PrimaryStorage.NOTE_DB
+ || !cfg.getBoolean(SECTION_NOTE_DB, CHANGES.key(), DISABLE_REVIEW_DB, false)) {
+ throw new OrmException(
+ "You appear to be upgrading from a 2.x site, but the NoteDb change migration was"
+ + " not completed. See documentation:\n"
+ + "https://gerrit-review.googlesource.com/Documentation/note-db.html#migration");
+ }
+
+ // We don't have a direct way to check that 2.16 init was run; the most obvious side effect
+ // would be upgrading the *ReviewDb* schema to the latest 2.16 schema version. But in 3.x we can
+ // no longer access ReviewDb, so we can't check that directly.
+ //
+ // Instead, check for a NoteDb-specific side effect of the migration process: the presence of
+ // the NoteDb group sequence ref. This is created by the schema 163 migration, which was part of
+ // 2.16 and not 2.15.
+ //
+ // There are a few corner cases where we will proceed even if the schema is not fully up to
+ // date:
+ // * If a user happened to run init from master after schema 163 was added but before 2.16
+ // final. We assume that someone savvy enough to do that has followed the documented
+ // requirement of upgrading to 2.16 final before 3.0.
+ // * If a user ran init in 2.16.x and the upgrade to 163 succeeded but a later update failed.
+ // In this case the server literally will not start under 2.16. We assume the user will fix
+ // this and get 2.16 running rather than abandoning 2.16 and jumping to 3.0 at this point.
+ try (Repository allUsers = repoManager.openRepository(allUsersName)) {
+ if (allUsers.exactRef(RefNames.REFS_SEQUENCES + Sequences.NAME_GROUPS) == null) {
+ throw new OrmException(
+ "You appear to be upgrading to 3.x from a version prior to 2.16; you must upgrade to"
+ + " 2.16.x first");
+ }
+ } catch (IOException e) {
+ throw new OrmException("Failed to check NoteDb migration state", e);
+ }
+ }
+
@VisibleForTesting
static ImmutableList<Integer> requiredUpgrades(
int currentVersion, ImmutableSortedSet<Integer> allVersions) throws OrmException {
diff --git a/javatests/com/google/gerrit/server/schema/NoteDbSchemaUpdaterTest.java b/javatests/com/google/gerrit/server/schema/NoteDbSchemaUpdaterTest.java
index fe41280..9af7b1b 100644
--- a/javatests/com/google/gerrit/server/schema/NoteDbSchemaUpdaterTest.java
+++ b/javatests/com/google/gerrit/server/schema/NoteDbSchemaUpdaterTest.java
@@ -23,18 +23,25 @@
import com.google.common.collect.ImmutableSortedMap;
import com.google.common.collect.ImmutableSortedSet;
import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.Sequences;
import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.IntBlob;
import com.google.gerrit.server.notedb.MutableNotesMigration;
import com.google.gerrit.server.notedb.NoteDbSchemaVersionManager;
import com.google.gerrit.server.notedb.NotesMigrationState;
+import com.google.gerrit.server.notedb.RepoSequence;
import com.google.gerrit.testing.GerritBaseTests;
import com.google.gerrit.testing.InMemoryRepositoryManager;
import com.google.gerrit.testing.TestUpdateUI;
import com.google.gwtorm.server.OrmException;
+import java.util.ArrayList;
+import java.util.List;
import java.util.Optional;
import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Repository;
import org.junit.Test;
@@ -62,7 +69,7 @@
} catch (OrmException e) {
assertThat(e)
.hasMessageThat()
- .isEqualTo("Cannot downgrade NoteDb schema from version 14 to 13");
+ .contains("Cannot downgrade NoteDb schema from version 14 to 13");
}
}
@@ -74,18 +81,23 @@
requiredUpgrades(8, versions);
assert_().fail("expected OrmException");
} catch (OrmException e) {
- assertThat(e).hasMessageThat().isEqualTo("Cannot skip NoteDb schema from version 8 to 10");
+ assertThat(e).hasMessageThat().contains("Cannot skip NoteDb schema from version 8 to 10");
}
}
private static class TestUpdate {
- private final AllProjectsName allProjectsName;
- private final NoteDbSchemaUpdater updater;
- private final GitRepositoryManager repoManager;
- private final NoteDbSchemaVersion.Arguments args;
+ protected final Config cfg;
+ protected final AllProjectsName allProjectsName;
+ protected final AllUsersName allUsersName;
+ protected final NoteDbSchemaUpdater updater;
+ protected final GitRepositoryManager repoManager;
+ protected final NoteDbSchemaVersion.Arguments args;
+ private final List<String> messages;
TestUpdate(Optional<Integer> initialVersion) throws Exception {
+ cfg = new Config();
allProjectsName = new AllProjectsName("The-Projects");
+ allUsersName = new AllUsersName("The-Users");
repoManager = new InMemoryRepositoryManager();
try (Repository repo = repoManager.createRepository(allProjectsName)) {
if (initialVersion.isPresent()) {
@@ -93,6 +105,9 @@
tr.update(RefNames.REFS_VERSION, tr.blob(initialVersion.get().toString()));
}
}
+ repoManager.createRepository(allUsersName).close();
+
+ setUp();
args = new NoteDbSchemaVersion.Arguments(repoManager, allProjectsName);
NoteDbSchemaVersionManager versionManager =
@@ -101,14 +116,37 @@
notesMigration.setFrom(NotesMigrationState.NOTE_DB);
updater =
new NoteDbSchemaUpdater(
+ cfg,
+ allUsersName,
+ repoManager,
notesMigration,
versionManager,
args,
ImmutableSortedMap.of(10, TestSchema_10.class, 11, TestSchema_11.class));
+ messages = new ArrayList<>();
}
+ protected void setNotesMigrationConfig() {
+ cfg.setString("noteDb", "changes", "write", "true");
+ cfg.setString("noteDb", "changes", "read", "true");
+ cfg.setString("noteDb", "changes", "primaryStorage", "NOTE_DB");
+ cfg.setString("noteDb", "changes", "disableReviewDb", "true");
+ }
+
+ protected void seedGroupSequenceRef() throws Exception {
+ new RepoSequence(
+ repoManager,
+ GitReferenceUpdated.DISABLED,
+ allUsersName,
+ Sequences.NAME_GROUPS,
+ () -> 1,
+ 1)
+ .next();
+ }
+
+ protected void setUp() throws Exception {}
+
ImmutableList<String> update() throws Exception {
- ImmutableList.Builder<String> messages = ImmutableList.builder();
updater.update(
new TestUpdateUI() {
@Override
@@ -116,7 +154,11 @@
messages.add(m);
}
});
- return messages.build();
+ return getMessages();
+ }
+
+ ImmutableList<String> getMessages() {
+ return ImmutableList.copyOf(messages);
}
Optional<Integer> readVersion() throws Exception {
@@ -151,8 +193,15 @@
}
@Test
- public void bootstrapUpdate() throws Exception {
- TestUpdate u = new TestUpdate(Optional.empty());
+ public void bootstrapUpdateWith216Prerequisites() throws Exception {
+ TestUpdate u =
+ new TestUpdate(Optional.empty()) {
+ @Override
+ public void setUp() throws Exception {
+ setNotesMigrationConfig();
+ seedGroupSequenceRef();
+ }
+ };
assertThat(u.update())
.containsExactly(
"Migrating data to schema 10 ...",
@@ -164,6 +213,44 @@
}
@Test
+ public void bootstrapUpdateFailsWithoutNotesMigrationConfig() throws Exception {
+ TestUpdate u =
+ new TestUpdate(Optional.empty()) {
+ @Override
+ public void setUp() throws Exception {
+ seedGroupSequenceRef();
+ }
+ };
+ try {
+ u.update();
+ assert_().fail("expected OrmException");
+ } catch (OrmException e) {
+ assertThat(e).hasMessageThat().contains("NoteDb change migration was not completed");
+ }
+ assertThat(u.getMessages()).isEmpty();
+ assertThat(u.readVersion()).isEmpty();
+ }
+
+ @Test
+ public void bootstrapUpdateFailsWithoutGroupSequenceRef() throws Exception {
+ TestUpdate u =
+ new TestUpdate(Optional.empty()) {
+ @Override
+ public void setUp() throws Exception {
+ setNotesMigrationConfig();
+ }
+ };
+ try {
+ u.update();
+ assert_().fail("expected OrmException");
+ } catch (OrmException e) {
+ assertThat(e).hasMessageThat().contains("upgrade to 2.16.x first");
+ }
+ assertThat(u.getMessages()).isEmpty();
+ assertThat(u.readVersion()).isEmpty();
+ }
+
+ @Test
public void updateTwoVersions() throws Exception {
TestUpdate u = new TestUpdate(Optional.of(9));
assertThat(u.update())
@@ -187,6 +274,8 @@
@Test
public void updateNoOp() throws Exception {
+ // This test covers the state when running the updater after initializing a new 3.x site, which
+ // seeds the schema version ref with the latest version.
TestUpdate u = new TestUpdate(Optional.of(11));
assertThat(u.update()).isEmpty();
assertThat(u.readVersion()).hasValue(11);
