Move groups sequence to NoteDB

This mirrors commit 5be9c31 ("Move account sequence to NoteDb"), with
the following differences:

* The group IDs go into refs/sequences/groups

* SchemaCreator creates groups, so there, we manually instantiate
  Sequences.

* Since group creation is rare, there is no tuning for batch size.

This is the first step in the procedure to move the group IDs to
NoteDb. The next steps are:

* Ensure that this code is stable and rolled out

* Run the schema upgrade

* Flip the readSequenceFromNoteDb flag

* Ensure that code is stable

* Delete readSequenceFromNoteDb flag, and associated dead code.

Change-Id: I3c8f8c0e86de93184b3ea5e801d0f2b25121a10d

Documentation/config-gerrit.txt

@@ -3396,6 +3396,18 @@ contention (aka lock failure) on the underlying ref storage.
 Defaults to 20 seconds; unit suffixes are supported, and assumes milliseconds if
 not specified.
 
+[[noteDb.groups.readSequenceFromNoteDb]]noteDb.groups.readSequenceFromNoteDb::
++
+Whether the group sequence should be read from NoteDb.
++
+Once set to `true` this parameter cannot be set back to `false` because
+the group sequence in ReviewDb will no longer be updated when group
+IDs are retrieved from NoteDb, and hence the group sequence in
+ReviewDb will be outdated.
++
+By default `false`.
+
+
 [[oauth]]
 === Section oauth
 

java/com/google/gerrit/reviewdb/server/ReviewDb.java

@@ -109,8 +109,11 @@ public interface ReviewDb extends Schema {
   @Deprecated
   int nextAccountId() throws OrmException;
 
+  int FIRST_GROUP_ID = 1;
+
   /** Next unique id for a {@link AccountGroup}. */
-  @Sequence
+  @Sequence(startWith = FIRST_GROUP_ID)
+  @Deprecated
   int nextAccountGroupId() throws OrmException;
 
   int FIRST_CHANGE_ID = 1;

java/com/google/gerrit/server/Sequences.java

@@ -42,6 +42,7 @@ import org.eclipse.jgit.lib.Config;
 @Singleton
 public class Sequences {
   public static final String NAME_ACCOUNTS = "accounts";
+  public static final String NAME_GROUPS = "groups";
   public static final String NAME_CHANGES = "changes";
 
   public static int getChangeSequenceGap(Config cfg) {
@@ -50,17 +51,20 @@ public class Sequences {
 
   private enum SequenceType {
     ACCOUNTS,
-    CHANGES;
+    CHANGES,
+    GROUPS;
   }
 
   private final Provider<ReviewDb> db;
   private final NotesMigration migration;
   private final RepoSequence accountSeq;
   private final RepoSequence changeSeq;
+  private final RepoSequence groupSeq;
+  private final boolean readGroupSeqFromNoteDb;
   private final Timer2<SequenceType, Boolean> nextIdLatency;
 
   @Inject
-  Sequences(
+  public Sequences(
       @GerritServerConfig Config cfg,
       Provider<ReviewDb> db,
       NotesMigration migration,
@@ -90,6 +94,13 @@ public class Sequences {
         new RepoSequence(
             repoManager, gitRefUpdated, allProjects, NAME_CHANGES, changeSeed, changeBatchSize);
 
+    RepoSequence.Seed groupSeed = () -> nextGroupId(db.get());
+    int groupBatchSize = 1;
+    groupSeq =
+        new RepoSequence(
+            repoManager, gitRefUpdated, allUsers, NAME_GROUPS, groupSeed, groupBatchSize);
+    readGroupSeqFromNoteDb = readGroupFromNoteDbSetting(cfg);
+
     nextIdLatency =
         metrics.newTimer(
             "sequence/next_id_latency",
@@ -100,6 +111,10 @@ public class Sequences {
             Field.ofBoolean("multiple"));
   }
 
+  public static boolean readGroupFromNoteDbSetting(Config cfg) {
+    return cfg.getBoolean("noteDb", "groups", "readSequenceFromNoteDb", false);
+  }
+
   public int nextAccountId() throws OrmException {
     try (Timer2.Context timer = nextIdLatency.start(SequenceType.ACCOUNTS, false)) {
       return accountSeq.next();
@@ -134,6 +149,17 @@ public class Sequences {
     return ImmutableList.copyOf(ids);
   }
 
+  public int nextGroupId() throws OrmException {
+    if (readGroupSeqFromNoteDb) {
+      try (Timer2.Context timer = nextIdLatency.start(SequenceType.GROUPS, false)) {
+        return groupSeq.next();
+      }
+    }
+    int groupId = nextGroupId(db.get());
+    groupSeq.increaseTo(groupId + 1); // NoteDb stores next available group ID.
+    return groupId;
+  }
+
   @VisibleForTesting
   public RepoSequence getChangeIdRepoSequence() {
     return changeSeq;
@@ -143,4 +169,9 @@ public class Sequences {
   private static int nextChangeId(ReviewDb db) throws OrmException {
     return db.nextChangeId();
   }
+
+  @SuppressWarnings("deprecation")
+  static int nextGroupId(ReviewDb db) throws OrmException {
+    return db.nextAccountGroupId();
+  }
 }

java/com/google/gerrit/server/group/CreateGroup.java

@@ -39,6 +39,7 @@ import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.GerritPersonIdent;
 import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.Sequences;
 import com.google.gerrit.server.account.CreateGroupArgs;
 import com.google.gerrit.server.account.GroupCache;
 import com.google.gerrit.server.account.GroupUUID;
@@ -82,6 +83,7 @@ public class CreateGroup implements RestModifyView<TopLevelResource, GroupInput>
   private final SystemGroupBackend systemGroupBackend;
   private final boolean defaultVisibleToAll;
   private final String name;
+  private final Sequences sequences;
 
   @Inject
   CreateGroup(
@@ -96,7 +98,8 @@ public class CreateGroup implements RestModifyView<TopLevelResource, GroupInput>
       AddMembers addMembers,
       SystemGroupBackend systemGroupBackend,
       @GerritServerConfig Config cfg,
-      @Assisted String name) {
+      @Assisted String name,
+      Sequences sequences) {
     this.self = self;
     this.serverIdent = serverIdent;
     this.db = db;
@@ -109,6 +112,7 @@ public class CreateGroup implements RestModifyView<TopLevelResource, GroupInput>
     this.systemGroupBackend = systemGroupBackend;
     this.defaultVisibleToAll = cfg.getBoolean("groups", "newGroupsVisibleToAll", false);
     this.name = name;
+    this.sequences = sequences;
   }
 
   public CreateGroup addOption(ListGroupsOption o) {
@@ -193,7 +197,7 @@ public class CreateGroup implements RestModifyView<TopLevelResource, GroupInput>
       }
     }
 
-    AccountGroup.Id groupId = new AccountGroup.Id(db.nextAccountGroupId());
+    AccountGroup.Id groupId = new AccountGroup.Id(sequences.nextGroupId());
     AccountGroup.UUID uuid =
         GroupUUID.make(
             createGroupArgs.getGroupName(),

java/com/google/gerrit/server/schema/SchemaCreator.java

@@ -18,6 +18,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.common.data.GroupReference;
+import com.google.gerrit.metrics.MetricMaker;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.client.AccountGroupName;
@@ -26,7 +27,9 @@ import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.SystemConfig;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.GerritPersonIdent;
+import com.google.gerrit.server.Sequences;
 import com.google.gerrit.server.account.GroupUUID;
+import com.google.gerrit.server.config.AllProjectsName;
 import com.google.gerrit.server.config.AllUsersName;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.SitePath;
@@ -41,6 +44,7 @@ import com.google.gerrit.server.group.db.InternalGroupCreation;
 import com.google.gerrit.server.group.db.InternalGroupUpdate;
 import com.google.gerrit.server.index.group.GroupIndex;
 import com.google.gerrit.server.index.group.GroupIndexCollection;
+import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gwtorm.jdbc.JdbcExecutor;
 import com.google.gwtorm.jdbc.JdbcSchema;
 import com.google.gwtorm.server.OrmDuplicateKeyException;
@@ -67,6 +71,11 @@ public class SchemaCreator {
   private final GroupIndexCollection indexCollection;
   private final boolean writeGroupsToNoteDb;
 
+  private final Config config;
+  private final MetricMaker metricMaker;
+  private final NotesMigration migration;
+  private final AllProjectsName allProjectsName;
+
   @Inject
   public SchemaCreator(
       SitePaths site,
@@ -77,8 +86,23 @@ public class SchemaCreator {
       @GerritPersonIdent PersonIdent au,
       DataSourceType dst,
       GroupIndexCollection ic,
-      @GerritServerConfig Config config) {
-    this(site.site_path, repoManager, ap, auc, allUsersName, au, dst, ic, config);
+      @GerritServerConfig Config config,
+      MetricMaker metricMaker,
+      NotesMigration migration,
+      AllProjectsName apName) {
+    this(
+        site.site_path,
+        repoManager,
+        ap,
+        auc,
+        allUsersName,
+        au,
+        dst,
+        ic,
+        config,
+        metricMaker,
+        migration,
+        apName);
   }
 
   public SchemaCreator(
@@ -90,7 +114,10 @@ public class SchemaCreator {
       @GerritPersonIdent PersonIdent au,
       DataSourceType dst,
       GroupIndexCollection ic,
-      Config config) {
+      Config config,
+      MetricMaker metricMaker,
+      NotesMigration migration,
+      AllProjectsName apName) {
     site_path = site;
     this.repoManager = repoManager;
     allProjectsCreator = ap;
@@ -104,6 +131,11 @@ public class SchemaCreator {
     // Don't flip this flag in a production setting! We only added it to spread the implementation
     // of groups in NoteDb among several changes which are gradually merged.
     writeGroupsToNoteDb = config.getBoolean("user", null, "writeGroupsToNoteDb", false);
+
+    this.config = config;
+    this.allProjectsName = apName;
+    this.migration = migration;
+    this.metricMaker = metricMaker;
   }
 
   public void create(ReviewDb db) throws OrmException, IOException, ConfigInvalidException {
@@ -124,17 +156,29 @@ public class SchemaCreator {
     // We have to create the All-Users repository before we can use it to store the groups in it.
     allUsersCreator.setAdministrators(admins).create();
 
+    // Don't rely on injection to construct Sequences, as it requires ReviewDb.
+    Sequences seqs =
+        new Sequences(
+            config,
+            () -> db,
+            migration,
+            repoManager,
+            GitReferenceUpdated.DISABLED,
+            allProjectsName,
+            allUsersName,
+            metricMaker);
     try (Repository repository = repoManager.openRepository(allUsersName)) {
-      createAdminsGroup(db, repository, admins);
-      createBatchUsersGroup(db, repository, batchUsers, admins);
+      createAdminsGroup(db, seqs, repository, admins);
+      createBatchUsersGroup(db, seqs, repository, batchUsers, admins);
     }
 
     dataSourceType.getIndexScript().run(db);
   }
 
-  private void createAdminsGroup(ReviewDb db, Repository repository, GroupReference groupReference)
+  private void createAdminsGroup(
+      ReviewDb db, Sequences seqs, Repository repository, GroupReference groupReference)
       throws OrmException, IOException, ConfigInvalidException {
-    InternalGroupCreation groupCreation = getGroupCreation(db, groupReference);
+    InternalGroupCreation groupCreation = getGroupCreation(seqs, groupReference);
     InternalGroupUpdate groupUpdate =
         InternalGroupUpdate.builder().setDescription("Gerrit Site Administrators").build();
 
@@ -142,9 +186,13 @@ public class SchemaCreator {
   }
 
   private void createBatchUsersGroup(
-      ReviewDb db, Repository repository, GroupReference groupReference, GroupReference admins)
+      ReviewDb db,
+      Sequences seqs,
+      Repository repository,
+      GroupReference groupReference,
+      GroupReference admins)
       throws OrmException, IOException, ConfigInvalidException {
-    InternalGroupCreation groupCreation = getGroupCreation(db, groupReference);
+    InternalGroupCreation groupCreation = getGroupCreation(seqs, groupReference);
     InternalGroupUpdate groupUpdate =
         InternalGroupUpdate.builder()
             .setDescription("Users who perform batch actions on Gerrit")
@@ -222,11 +270,12 @@ public class SchemaCreator {
     return new GroupReference(groupUuid, name);
   }
 
-  private static InternalGroupCreation getGroupCreation(ReviewDb db, GroupReference groupReference)
+  private InternalGroupCreation getGroupCreation(Sequences seqs, GroupReference groupReference)
       throws OrmException {
+    int next = seqs.nextGroupId();
     return InternalGroupCreation.builder()
         .setNameKey(new AccountGroup.NameKey(groupReference.getName()))
-        .setId(new AccountGroup.Id(db.nextAccountGroupId()))
+        .setId(new AccountGroup.Id(next))
         .setGroupUUID(groupReference.getUUID())
         .setCreatedOn(TimeUtil.nowTs())
         .build();

java/com/google/gerrit/server/schema/SchemaVersion.java

@@ -35,7 +35,7 @@ import java.util.concurrent.TimeUnit;
 /** A version of the database schema. */
 public abstract class SchemaVersion {
   /** The current schema version. */
-  public static final Class<Schema_162> C = Schema_162.class;
+  public static final Class<Schema_163> C = Schema_163.class;
 
   public static int getBinaryVersion() {
     return guessVersion(C);

java/com/google/gerrit/server/schema/Schema_163.java

@@ -0,0 +1,57 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.schema;
+
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.Sequences;
+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.RepoSequence;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import java.sql.SQLException;
+
+/** Create group sequence in NoteDb */
+public class Schema_163 extends SchemaVersion {
+  private final GitRepositoryManager repoManager;
+  private final AllUsersName allUsersName;
+
+  @Inject
+  Schema_163(
+      Provider<Schema_162> prior, GitRepositoryManager repoManager, AllUsersName allUsersName) {
+    super(prior);
+    this.repoManager = repoManager;
+    this.allUsersName = allUsersName;
+  }
+
+  @Override
+  protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException, SQLException {
+    @SuppressWarnings("deprecation")
+    RepoSequence.Seed groupSeed = () -> db.nextAccountGroupId();
+    RepoSequence groupSeq =
+        new RepoSequence(
+            repoManager,
+            GitReferenceUpdated.DISABLED,
+            allUsersName,
+            Sequences.NAME_GROUPS,
+            groupSeed,
+            1);
+
+    // consume one account ID to ensure that the group sequence is initialized in NoteDb
+    groupSeq.next();
+  }
+}

javatests/com/google/gerrit/server/schema/SchemaUpdaterTest.java

@@ -18,6 +18,8 @@ import static com.google.common.truth.Truth.assertThat;
 
 import com.google.gerrit.extensions.config.FactoryModule;
 import com.google.gerrit.lifecycle.LifecycleManager;
+import com.google.gerrit.metrics.DisabledMetricMaker;
+import com.google.gerrit.metrics.MetricMaker;
 import com.google.gerrit.reviewdb.client.SystemConfig;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.GerritPersonIdent;
@@ -113,6 +115,7 @@ public class SchemaUpdaterTest {
 
                     bind(SystemGroupBackend.class);
                     install(new NotesMigration.Module());
+                    bind(MetricMaker.class).to(DisabledMetricMaker.class);
                   }
                 })
             .getInstance(SchemaUpdater.class);