Move account sequence to NoteDb
Accounts have been migrated to NoteDb, hence also the account sequence should be moved to NoteDb. In NoteDb the current account sequence number is stored as UTF-8 text in a blob pointed to by the 'refs/sequences/accounts' ref in the 'All-Users' repository. Multiple processes share the same sequence by incrementing the counter using normal git ref updates. To amortize the cost of these ref updates, processes can increment the counter by a larger number and hand out numbers from that range in memory until they run out. The size of the account ID batch that each process retrieves at once is controlled by the 'notedb.accounts.sequenceBatchSize' configuration parameter in 'gerrit.config'. By default the value is 1 since it's unlikely that a process ever creates more than one account. This follows the example of storing the change sequence in NoteDb. A difference is that the account sequence is stored in the 'All-Users' repository while the change sequence is stored in the 'All-Projects' repository. Storing the account sequence in the 'All-Users' repository makes more sense since this repository already contains all of the other account data. Injecting the Sequences class that provides new sequences numbers requires request scope. There are 2 places outsite of request scope where new account sequence numbers are required, AccountManager and AccountCreator (only used for tests). These classes need to create a request context to get an account sequence number. For AccountManager the request scope is only created when a new account is created and not when an existing account is authenticated. Since there is an init step that creates an initial admin user we must make the account sequence available during the init phase. For this the class SequencesOnInit is added which can only generate account IDs, but only depends on classes that are available during the init phase. For this class the account ID batch size is hard-coded to 1, since init only creates a single account and we don't want to waste account IDs when a new Gerrit server is initialized. This change also contains a schema migration that ensures that the account sequence is created in NoteDb. To support a live migration on a multi-master Gerrit installation, there is a configuration parameter ('notedb.accounts.readSequenceFromNoteDb') that controls whether account sequence numbers are read from NoteDb or ReviewDb. By default the value for this parameter is `false` so account sequence numbers are read from ReviewDb. If account sequence numbers are read from ReviewDb the sequence numbers in NoteDb will be kept in sync. This is achieved by writing the next available sequence number to NoteDb whenever a sequence number from ReviewDb is retrieved. If writing to NoteDb fails, an exception is raised to the caller and the sequence number that was retrieved from ReviewDb is not used. Writing to NoteDb is retried several times so that the caller only gets an exception if writing to NoteDb fails permanently. For the case where two threads try to update the sequence number in NoteDb concurrently we must make sure that the value in NoteDb is never decreased. E.g.: 1. Thread 1 retrieves account ID 14 from ReviewDb 2. Thread 2 retrieves account ID 15 from ReviewDb 3. Thread 2 writes the next available account ID 16 to NoteDb 4. Thread 1 tries to write the next available account ID 15 to NoteDb but fails since Thread 2 updated the value concurrently. 5. Thread 1 finds that it doesn't need to update the account ID in NoteDb anymore since Thread 2 already updated the account ID to a higher value This means at any point in time it is safe to switch to reading account IDs from NoteDb. However once this switch is done it is not possible to switch back to reading account IDs from ReviewDb, since ReviewDb will be out of sync as soon as the first account ID was retrieved from NoteDb. The migration on a multi-master Gerrit installation will be done with the following steps: 1. rollout this change to all nodes: - account sequence numbers are read from ReviewDb - the sequence numbers in NoteDb are kept in sync 2. wait some time until we are sure that we don't need to roll back to a release that doesn't contain this change 3. run an offline migration to ensure that the account sequence number in NoteDb is initialized on all nodes 4. set 'notedb.accounts.readSequenceFromNoteDb' to true so that account sequence numbers are now read from NoteDb (this setting cannot be reverted since the account sequence in ReviewDb will be outdated once account IDs are retrieved from NoteDb) After this is done a follow-up change can remove the handling for 'notedb.accounts.readSequenceFromNoteDb' so that account IDs are now always retrieved from NoteDb. Change-Id: I023d2de643ed0c15197c09fa19105cc2acb5091e Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
parent
11f049367f
commit
5be9c31001
@ -3285,6 +3285,33 @@ NoteDb is the next generation of Gerrit storage backend, currently powering
|
||||
`googlesource.com`. It is not (yet) recommended for general use, but if you want
|
||||
to learn more, see the link:dev-note-db.html[developer documentation].
|
||||
|
||||
[[notedb.accounts.readSequenceFromNoteDb]]notedb.accounts.readSequenceFromNoteDb::
|
||||
+
|
||||
Whether the account sequence should be read from NoteDb.
|
||||
+
|
||||
Once set to `true` this parameter cannot be set back to `false` because
|
||||
the account sequence in ReviewDb will no longer be updated when account
|
||||
IDs are retrieved from NoteDb, and hence the account sequence in
|
||||
ReviewDb will be outdated.
|
||||
+
|
||||
By default `false`.
|
||||
|
||||
[[notedb.accounts.sequenceBatchSize]]notedb.accounts.sequenceBatchSize::
|
||||
+
|
||||
The current account sequence number is stored as UTF-8 text in a blob
|
||||
pointed to by the `refs/sequences/accounts` ref in the `All-Users`
|
||||
repository. Multiple processes share the same sequence by incrementing
|
||||
the counter using normal git ref updates. To amortize the cost of these
|
||||
ref updates, processes increment the counter by a larger number and
|
||||
hand out numbers from that range in memory until they run out. This
|
||||
configuration parameter controls the size of the account ID batch that
|
||||
each process retrieves at once.
|
||||
+
|
||||
Only relevant if link:#notedb.accounts.readSequenceFromNoteDb[
|
||||
notedb.accounts.readSequenceFromNoteDb] is set to `true`.
|
||||
+
|
||||
By default, 1.
|
||||
|
||||
[[oauth]]
|
||||
=== Section oauth
|
||||
|
||||
|
@ -23,6 +23,7 @@ import com.google.gerrit.reviewdb.client.Account;
|
||||
import com.google.gerrit.reviewdb.client.AccountGroup;
|
||||
import com.google.gerrit.reviewdb.client.AccountGroupMember;
|
||||
import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||
import com.google.gerrit.server.Sequences;
|
||||
import com.google.gerrit.server.account.AccountByEmailCache;
|
||||
import com.google.gerrit.server.account.AccountCache;
|
||||
import com.google.gerrit.server.account.AccountsUpdate;
|
||||
@ -31,8 +32,11 @@ import com.google.gerrit.server.account.VersionedAuthorizedKeys;
|
||||
import com.google.gerrit.server.account.externalids.ExternalId;
|
||||
import com.google.gerrit.server.account.externalids.ExternalIdsUpdate;
|
||||
import com.google.gerrit.server.ssh.SshKeyCache;
|
||||
import com.google.gerrit.server.util.ManualRequestContext;
|
||||
import com.google.gerrit.server.util.OneOffRequestContext;
|
||||
import com.google.gwtorm.server.SchemaFactory;
|
||||
import com.google.inject.Inject;
|
||||
import com.google.inject.Provider;
|
||||
import com.google.inject.Singleton;
|
||||
import com.jcraft.jsch.JSch;
|
||||
import com.jcraft.jsch.JSchException;
|
||||
@ -50,6 +54,8 @@ public class AccountCreator {
|
||||
private final Map<String, TestAccount> accounts;
|
||||
|
||||
private final SchemaFactory<ReviewDb> reviewDbProvider;
|
||||
private final OneOffRequestContext oneOffRequestContext;
|
||||
private final Provider<Sequences> sequencesProvider;
|
||||
private final AccountsUpdate.Server accountsUpdate;
|
||||
private final VersionedAuthorizedKeys.Accessor authorizedKeys;
|
||||
private final GroupCache groupCache;
|
||||
@ -62,6 +68,8 @@ public class AccountCreator {
|
||||
@Inject
|
||||
AccountCreator(
|
||||
SchemaFactory<ReviewDb> schema,
|
||||
OneOffRequestContext oneOffRequestContext,
|
||||
Provider<Sequences> sequencesProvider,
|
||||
AccountsUpdate.Server accountsUpdate,
|
||||
VersionedAuthorizedKeys.Accessor authorizedKeys,
|
||||
GroupCache groupCache,
|
||||
@ -72,6 +80,8 @@ public class AccountCreator {
|
||||
@SshEnabled boolean sshEnabled) {
|
||||
accounts = new HashMap<>();
|
||||
reviewDbProvider = schema;
|
||||
this.oneOffRequestContext = oneOffRequestContext;
|
||||
this.sequencesProvider = sequencesProvider;
|
||||
this.accountsUpdate = accountsUpdate;
|
||||
this.authorizedKeys = authorizedKeys;
|
||||
this.groupCache = groupCache;
|
||||
@ -93,8 +103,9 @@ public class AccountCreator {
|
||||
if (account != null) {
|
||||
return account;
|
||||
}
|
||||
try (ReviewDb db = reviewDbProvider.open()) {
|
||||
Account.Id id = new Account.Id(db.nextAccountId());
|
||||
try (ReviewDb db = reviewDbProvider.open();
|
||||
ManualRequestContext ctx = oneOffRequestContext.open()) {
|
||||
Account.Id id = new Account.Id(sequencesProvider.get().nextAccountId());
|
||||
|
||||
List<ExternalId> extIds = new ArrayList<>(2);
|
||||
String httpPass = null;
|
||||
|
@ -214,6 +214,15 @@ public class GerritServer implements AutoCloseable {
|
||||
.isNotEqualTo(NoteDbMode.FUSED);
|
||||
Config cfg = desc.buildConfig(baseConfig);
|
||||
Map<String, Config> pluginConfigs = desc.buildPluginConfigs();
|
||||
|
||||
MergeableFileBasedConfig gerritConfig =
|
||||
new MergeableFileBasedConfig(
|
||||
site.resolve("etc").resolve("gerrit.config").toFile(), FS.DETECTED);
|
||||
gerritConfig.load();
|
||||
gerritConfig.merge(cfg);
|
||||
mergeTestConfig(gerritConfig);
|
||||
gerritConfig.save();
|
||||
|
||||
Init init = new Init();
|
||||
int rc =
|
||||
init.main(
|
||||
@ -224,14 +233,6 @@ public class GerritServer implements AutoCloseable {
|
||||
throw new RuntimeException("Couldn't initialize site");
|
||||
}
|
||||
|
||||
MergeableFileBasedConfig gerritConfig =
|
||||
new MergeableFileBasedConfig(
|
||||
site.resolve("etc").resolve("gerrit.config").toFile(), FS.DETECTED);
|
||||
gerritConfig.load();
|
||||
gerritConfig.merge(cfg);
|
||||
mergeTestConfig(gerritConfig);
|
||||
gerritConfig.save();
|
||||
|
||||
for (String pluginName : pluginConfigs.keySet()) {
|
||||
MergeableFileBasedConfig pluginCfg =
|
||||
new MergeableFileBasedConfig(
|
||||
@ -258,6 +259,7 @@ public class GerritServer implements AutoCloseable {
|
||||
public static GerritServer initAndStart(Description desc, Config baseConfig) throws Exception {
|
||||
Path site = TempFileUtil.createTempDirectory().toPath();
|
||||
baseConfig = new Config(baseConfig);
|
||||
baseConfig.setString("gerrit", null, "basePath", site.resolve("git").toString());
|
||||
baseConfig.setString("gerrit", null, "tempSiteDir", site.toString());
|
||||
try {
|
||||
if (!desc.memory()) {
|
||||
|
@ -75,6 +75,7 @@ import com.google.gerrit.gpg.PublicKeyStore;
|
||||
import com.google.gerrit.gpg.testutil.TestKey;
|
||||
import com.google.gerrit.reviewdb.client.Account;
|
||||
import com.google.gerrit.reviewdb.client.RefNames;
|
||||
import com.google.gerrit.server.Sequences;
|
||||
import com.google.gerrit.server.account.AccountByEmailCache;
|
||||
import com.google.gerrit.server.account.AccountConfig;
|
||||
import com.google.gerrit.server.account.AccountsUpdate;
|
||||
@ -148,6 +149,8 @@ public class AccountIT extends AbstractDaemonTest {
|
||||
|
||||
@Inject private DynamicSet<AccountIndexedListener> accountIndexedListeners;
|
||||
|
||||
@Inject private Sequences seq;
|
||||
|
||||
private AccountIndexedCounter accountIndexedCounter;
|
||||
private RegistrationHandle accountIndexEventCounterHandle;
|
||||
private ExternalIdsUpdate externalIdsUpdate;
|
||||
@ -894,7 +897,7 @@ public class AccountIT extends AbstractDaemonTest {
|
||||
grant(allUsers, RefNames.REFS_USERS + "*", Permission.CREATE);
|
||||
grant(allUsers, RefNames.REFS_USERS + "*", Permission.PUSH);
|
||||
|
||||
String userRef = RefNames.refsUsers(new Account.Id(db.nextAccountId()));
|
||||
String userRef = RefNames.refsUsers(new Account.Id(seq.nextAccountId()));
|
||||
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
|
||||
PushOneCommit.Result r = pushFactory.create(db, admin.getIdent(), allUsersRepo).to(userRef);
|
||||
r.assertErrorStatus();
|
||||
@ -912,7 +915,7 @@ public class AccountIT extends AbstractDaemonTest {
|
||||
grant(allUsers, RefNames.REFS_USERS + "*", Permission.CREATE);
|
||||
grant(allUsers, RefNames.REFS_USERS + "*", Permission.PUSH);
|
||||
|
||||
String userRef = RefNames.refsUsers(new Account.Id(db.nextAccountId()));
|
||||
String userRef = RefNames.refsUsers(new Account.Id(seq.nextAccountId()));
|
||||
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
|
||||
pushFactory.create(db, admin.getIdent(), allUsersRepo).to(userRef).assertOkStatus();
|
||||
|
||||
|
@ -29,6 +29,7 @@ BASE_JETTY_DEPS = [
|
||||
|
||||
DEPS = BASE_JETTY_DEPS + [
|
||||
"//gerrit-reviewdb:server",
|
||||
"//lib:gwtorm",
|
||||
"//lib/log:jsonevent-layout",
|
||||
]
|
||||
|
||||
@ -53,7 +54,6 @@ java_library(
|
||||
"//lib:args4j",
|
||||
"//lib:derby",
|
||||
"//lib:gwtjsonrpc",
|
||||
"//lib:gwtorm",
|
||||
"//lib:h2",
|
||||
"//lib/commons:validator",
|
||||
"//lib/mina:sshd",
|
||||
@ -64,7 +64,6 @@ REST_UTIL_DEPS = [
|
||||
"//gerrit-cache-h2:cache-h2",
|
||||
"//gerrit-util-cli:cli",
|
||||
"//lib:args4j",
|
||||
"//lib:gwtorm",
|
||||
"//lib/commons:dbcp",
|
||||
]
|
||||
|
||||
@ -119,7 +118,6 @@ REST_PGM_DEPS = [
|
||||
"//gerrit-oauth:oauth",
|
||||
"//gerrit-openid:openid",
|
||||
"//lib:args4j",
|
||||
"//lib:gwtorm",
|
||||
"//lib:protobuf",
|
||||
"//lib:servlet-api-3_1-without-neverlink",
|
||||
"//lib/prolog:cafeteria",
|
||||
|
@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkArgument;
|
||||
import static java.nio.charset.StandardCharsets.UTF_8;
|
||||
|
||||
import com.google.common.collect.ImmutableSet;
|
||||
import com.google.gerrit.pgm.init.api.AllUsersNameOnInitProvider;
|
||||
import com.google.gerrit.pgm.init.api.InitFlags;
|
||||
import com.google.gerrit.reviewdb.client.Account;
|
||||
import com.google.gerrit.reviewdb.client.RefNames;
|
||||
|
@ -36,9 +36,11 @@ import com.google.gerrit.pgm.init.index.lucene.LuceneIndexModuleOnInit;
|
||||
import com.google.gerrit.pgm.util.SiteProgram;
|
||||
import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||
import com.google.gerrit.server.config.GerritServerConfigModule;
|
||||
import com.google.gerrit.server.config.RepositoryConfig;
|
||||
import com.google.gerrit.server.config.SitePath;
|
||||
import com.google.gerrit.server.config.SitePaths;
|
||||
import com.google.gerrit.server.git.GitRepositoryManager;
|
||||
import com.google.gerrit.server.git.GitRepositoryManagerModule;
|
||||
import com.google.gerrit.server.index.IndexModule;
|
||||
import com.google.gerrit.server.plugins.JarScanner;
|
||||
import com.google.gerrit.server.schema.SchemaUpdater;
|
||||
@ -453,6 +455,7 @@ public class BaseInit extends SiteProgram {
|
||||
bind(InitFlags.class).toInstance(init.flags);
|
||||
}
|
||||
});
|
||||
modules.add(new GitRepositoryManagerModule(new RepositoryConfig(init.flags.cfg)));
|
||||
Injector dbInjector = createDbInjector(SINGLE_USER);
|
||||
switch (IndexModule.getIndexType(dbInjector)) {
|
||||
case LUCENE:
|
||||
|
@ -14,6 +14,7 @@
|
||||
|
||||
package com.google.gerrit.pgm.init;
|
||||
|
||||
import com.google.gerrit.pgm.init.api.AllUsersNameOnInitProvider;
|
||||
import com.google.gerrit.pgm.init.api.InitFlags;
|
||||
import com.google.gerrit.server.GerritPersonIdentProvider;
|
||||
import com.google.gerrit.server.account.externalids.ExternalId;
|
||||
|
@ -22,6 +22,7 @@ import com.google.gerrit.extensions.client.AuthType;
|
||||
import com.google.gerrit.pgm.init.api.ConsoleUI;
|
||||
import com.google.gerrit.pgm.init.api.InitFlags;
|
||||
import com.google.gerrit.pgm.init.api.InitStep;
|
||||
import com.google.gerrit.pgm.init.api.SequencesOnInit;
|
||||
import com.google.gerrit.reviewdb.client.Account;
|
||||
import com.google.gerrit.reviewdb.client.AccountGroup;
|
||||
import com.google.gerrit.reviewdb.client.AccountGroupMember;
|
||||
@ -50,6 +51,7 @@ public class InitAdminUser implements InitStep {
|
||||
private final AccountsOnInit accounts;
|
||||
private final VersionedAuthorizedKeysOnInit.Factory authorizedKeysFactory;
|
||||
private final ExternalIdsOnInit externalIds;
|
||||
private final SequencesOnInit sequencesOnInit;
|
||||
private SchemaFactory<ReviewDb> dbFactory;
|
||||
private AccountIndexCollection indexCollection;
|
||||
|
||||
@ -59,12 +61,14 @@ public class InitAdminUser implements InitStep {
|
||||
ConsoleUI ui,
|
||||
AccountsOnInit accounts,
|
||||
VersionedAuthorizedKeysOnInit.Factory authorizedKeysFactory,
|
||||
ExternalIdsOnInit externalIds) {
|
||||
ExternalIdsOnInit externalIds,
|
||||
SequencesOnInit sequencesOnInit) {
|
||||
this.flags = flags;
|
||||
this.ui = ui;
|
||||
this.accounts = accounts;
|
||||
this.authorizedKeysFactory = authorizedKeysFactory;
|
||||
this.externalIds = externalIds;
|
||||
this.sequencesOnInit = sequencesOnInit;
|
||||
}
|
||||
|
||||
@Override
|
||||
@ -91,7 +95,7 @@ public class InitAdminUser implements InitStep {
|
||||
if (!accounts.hasAnyAccount()) {
|
||||
ui.header("Gerrit Administrator");
|
||||
if (ui.yesno(true, "Create administrator user")) {
|
||||
Account.Id id = new Account.Id(db.nextAccountId());
|
||||
Account.Id id = new Account.Id(sequencesOnInit.nextAccountId(db));
|
||||
String username = ui.readString("admin", "username");
|
||||
String name = ui.readString("Administrator", "name");
|
||||
String httpPassword = ui.readString("secret", "HTTP password");
|
||||
|
@ -17,6 +17,7 @@ package com.google.gerrit.pgm.init;
|
||||
import static com.google.common.base.Preconditions.checkState;
|
||||
|
||||
import com.google.common.base.Strings;
|
||||
import com.google.gerrit.pgm.init.api.AllUsersNameOnInitProvider;
|
||||
import com.google.gerrit.pgm.init.api.InitFlags;
|
||||
import com.google.gerrit.pgm.init.api.VersionedMetaDataOnInit;
|
||||
import com.google.gerrit.reviewdb.client.Account;
|
||||
|
@ -12,11 +12,10 @@
|
||||
// See the License for the specific language governing permissions and
|
||||
// limitations under the License.
|
||||
|
||||
package com.google.gerrit.pgm.init;
|
||||
package com.google.gerrit.pgm.init.api;
|
||||
|
||||
import com.google.common.base.MoreObjects;
|
||||
import com.google.common.base.Strings;
|
||||
import com.google.gerrit.pgm.init.api.Section;
|
||||
import com.google.gerrit.server.config.AllUsersNameProvider;
|
||||
import com.google.inject.Inject;
|
||||
import com.google.inject.Provider;
|
@ -0,0 +1,49 @@
|
||||
// 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.pgm.init.api;
|
||||
|
||||
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.git.GitRepositoryManager;
|
||||
import com.google.gerrit.server.notedb.RepoSequence;
|
||||
import com.google.gwtorm.server.OrmException;
|
||||
import com.google.inject.Inject;
|
||||
import com.google.inject.Singleton;
|
||||
|
||||
@Singleton
|
||||
public class SequencesOnInit {
|
||||
private final GitRepositoryManager repoManager;
|
||||
private final AllUsersNameOnInitProvider allUsersName;
|
||||
|
||||
@Inject
|
||||
SequencesOnInit(GitRepositoryManager repoManager, AllUsersNameOnInitProvider allUsersName) {
|
||||
this.repoManager = repoManager;
|
||||
this.allUsersName = allUsersName;
|
||||
}
|
||||
|
||||
public int nextAccountId(ReviewDb db) throws OrmException {
|
||||
@SuppressWarnings("deprecation")
|
||||
RepoSequence.Seed accountSeed = () -> db.nextAccountId();
|
||||
RepoSequence accountSeq =
|
||||
new RepoSequence(
|
||||
repoManager,
|
||||
new Project.NameKey(allUsersName.get()),
|
||||
Sequences.NAME_ACCOUNTS,
|
||||
accountSeed,
|
||||
1);
|
||||
return accountSeq.next();
|
||||
}
|
||||
}
|
@ -188,7 +188,7 @@ public final class Account {
|
||||
/**
|
||||
* Create a new account.
|
||||
*
|
||||
* @param newId unique id, see {@link com.google.gerrit.reviewdb.server.ReviewDb#nextAccountId()}.
|
||||
* @param newId unique id, see {@link com.google.gerrit.server.Sequences#nextAccountId()}.
|
||||
* @param registeredOn when the account was registered.
|
||||
*/
|
||||
public Account(Account.Id newId, Timestamp registeredOn) {
|
||||
|
@ -99,8 +99,15 @@ public interface ReviewDb extends Schema {
|
||||
@Relation(id = 30)
|
||||
AccountGroupByIdAudAccess accountGroupByIdAud();
|
||||
|
||||
/** Create the next unique id for an {@link Account}. */
|
||||
@Sequence(startWith = 1000000)
|
||||
int FIRST_ACCOUNT_ID = 1000000;
|
||||
|
||||
/**
|
||||
* Next unique id for a {@link Account}.
|
||||
*
|
||||
* @deprecated use {@link com.google.gerrit.server.Sequences#nextAccountId()}.
|
||||
*/
|
||||
@Sequence(startWith = FIRST_ACCOUNT_ID)
|
||||
@Deprecated
|
||||
int nextAccountId() throws OrmException;
|
||||
|
||||
/** Next unique id for a {@link AccountGroup}. */
|
||||
|
@ -137,6 +137,7 @@ public class ReviewDbWrapper implements ReviewDb {
|
||||
}
|
||||
|
||||
@Override
|
||||
@SuppressWarnings("deprecation")
|
||||
public int nextAccountId() throws OrmException {
|
||||
return delegate.nextAccountId();
|
||||
}
|
||||
|
@ -25,6 +25,7 @@ import com.google.gerrit.metrics.MetricMaker;
|
||||
import com.google.gerrit.metrics.Timer2;
|
||||
import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||
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.git.GitRepositoryManager;
|
||||
import com.google.gerrit.server.notedb.NotesMigration;
|
||||
@ -39,6 +40,7 @@ import org.eclipse.jgit.lib.Config;
|
||||
|
||||
@Singleton
|
||||
public class Sequences {
|
||||
public static final String NAME_ACCOUNTS = "accounts";
|
||||
public static final String NAME_CHANGES = "changes";
|
||||
|
||||
public static int getChangeSequenceGap(Config cfg) {
|
||||
@ -46,11 +48,14 @@ public class Sequences {
|
||||
}
|
||||
|
||||
private enum SequenceType {
|
||||
ACCOUNTS,
|
||||
CHANGES;
|
||||
}
|
||||
|
||||
private final Provider<ReviewDb> db;
|
||||
private final NotesMigration migration;
|
||||
private final boolean readAccountSeqFromNoteDb;
|
||||
private final RepoSequence accountSeq;
|
||||
private final RepoSequence changeSeq;
|
||||
private final Timer2<SequenceType, Boolean> nextIdLatency;
|
||||
|
||||
@ -61,15 +66,30 @@ public class Sequences {
|
||||
NotesMigration migration,
|
||||
GitRepositoryManager repoManager,
|
||||
AllProjectsName allProjects,
|
||||
AllUsersName allUsers,
|
||||
MetricMaker metrics) {
|
||||
this.db = db;
|
||||
this.migration = migration;
|
||||
|
||||
readAccountSeqFromNoteDb =
|
||||
cfg.getBoolean("noteDb", "accounts", "readSequenceFromNoteDb", false);
|
||||
|
||||
// It's intentional to not use any configured gap for the account sequence unlike we do for the
|
||||
// change sequence. For the account sequence we have a different migration strategy that
|
||||
// doesn't require any gap.
|
||||
@SuppressWarnings("deprecation")
|
||||
RepoSequence.Seed accountSeed = () -> db.get().nextAccountId();
|
||||
|
||||
int accountBatchSize = cfg.getInt("noteDb", "accounts", "sequenceBatchSize", 1);
|
||||
accountSeq =
|
||||
new RepoSequence(repoManager, allUsers, NAME_ACCOUNTS, accountSeed, accountBatchSize);
|
||||
|
||||
int gap = getChangeSequenceGap(cfg);
|
||||
@SuppressWarnings("deprecation")
|
||||
RepoSequence.Seed seed = () -> db.get().nextChangeId() + gap;
|
||||
int batchSize = cfg.getInt("noteDb", "changes", "sequenceBatchSize", 20);
|
||||
changeSeq = new RepoSequence(repoManager, allProjects, NAME_CHANGES, seed, batchSize);
|
||||
RepoSequence.Seed changeSeed = () -> db.get().nextChangeId() + gap;
|
||||
int changeBatchSize = cfg.getInt("noteDb", "changes", "sequenceBatchSize", 20);
|
||||
changeSeq =
|
||||
new RepoSequence(repoManager, allProjects, NAME_CHANGES, changeSeed, changeBatchSize);
|
||||
|
||||
nextIdLatency =
|
||||
metrics.newTimer(
|
||||
@ -81,6 +101,18 @@ public class Sequences {
|
||||
Field.ofBoolean("multiple"));
|
||||
}
|
||||
|
||||
public int nextAccountId() throws OrmException {
|
||||
if (readAccountSeqFromNoteDb) {
|
||||
try (Timer2.Context timer = nextIdLatency.start(SequenceType.ACCOUNTS, false)) {
|
||||
return accountSeq.next();
|
||||
}
|
||||
}
|
||||
|
||||
int accountId = nextAccountId(db.get());
|
||||
accountSeq.increaseTo(accountId + 1); // NoteDb stores next available account ID.
|
||||
return accountId;
|
||||
}
|
||||
|
||||
public int nextChangeId() throws OrmException {
|
||||
if (!migration.readChangeSequence()) {
|
||||
return nextChangeId(db.get());
|
||||
@ -114,6 +146,11 @@ public class Sequences {
|
||||
return changeSeq;
|
||||
}
|
||||
|
||||
@SuppressWarnings("deprecation")
|
||||
private static int nextAccountId(ReviewDb db) throws OrmException {
|
||||
return db.nextAccountId();
|
||||
}
|
||||
|
||||
@SuppressWarnings("deprecation")
|
||||
private static int nextChangeId(ReviewDb db) throws OrmException {
|
||||
return db.nextChangeId();
|
||||
|
@ -27,12 +27,15 @@ import com.google.gerrit.reviewdb.client.AccountGroup;
|
||||
import com.google.gerrit.reviewdb.client.AccountGroupMember;
|
||||
import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||
import com.google.gerrit.server.IdentifiedUser;
|
||||
import com.google.gerrit.server.Sequences;
|
||||
import com.google.gerrit.server.account.externalids.ExternalId;
|
||||
import com.google.gerrit.server.account.externalids.ExternalIds;
|
||||
import com.google.gerrit.server.account.externalids.ExternalIdsUpdate;
|
||||
import com.google.gerrit.server.config.GerritServerConfig;
|
||||
import com.google.gerrit.server.project.ProjectCache;
|
||||
import com.google.gerrit.server.query.account.InternalAccountQuery;
|
||||
import com.google.gerrit.server.util.ManualRequestContext;
|
||||
import com.google.gerrit.server.util.OneOffRequestContext;
|
||||
import com.google.gwtorm.server.OrmException;
|
||||
import com.google.gwtorm.server.SchemaFactory;
|
||||
import com.google.inject.Inject;
|
||||
@ -57,6 +60,8 @@ public class AccountManager {
|
||||
private static final Logger log = LoggerFactory.getLogger(AccountManager.class);
|
||||
|
||||
private final SchemaFactory<ReviewDb> schema;
|
||||
private final OneOffRequestContext oneOffRequestContext;
|
||||
private final Provider<Sequences> sequencesProvider;
|
||||
private final Accounts accounts;
|
||||
private final AccountsUpdate.Server accountsUpdateFactory;
|
||||
private final AccountCache byIdCache;
|
||||
@ -74,6 +79,8 @@ public class AccountManager {
|
||||
@Inject
|
||||
AccountManager(
|
||||
SchemaFactory<ReviewDb> schema,
|
||||
OneOffRequestContext oneOffRequestContext,
|
||||
Provider<Sequences> sequencesProvider,
|
||||
@GerritServerConfig Config cfg,
|
||||
Accounts accounts,
|
||||
AccountsUpdate.Server accountsUpdateFactory,
|
||||
@ -88,6 +95,8 @@ public class AccountManager {
|
||||
ExternalIds externalIds,
|
||||
ExternalIdsUpdate.Server externalIdsUpdateFactory) {
|
||||
this.schema = schema;
|
||||
this.oneOffRequestContext = oneOffRequestContext;
|
||||
this.sequencesProvider = sequencesProvider;
|
||||
this.accounts = accounts;
|
||||
this.accountsUpdateFactory = accountsUpdateFactory;
|
||||
this.byIdCache = byIdCache;
|
||||
@ -205,7 +214,10 @@ public class AccountManager {
|
||||
|
||||
private AuthResult create(ReviewDb db, AuthRequest who)
|
||||
throws OrmException, AccountException, IOException, ConfigInvalidException {
|
||||
Account.Id newId = new Account.Id(db.nextAccountId());
|
||||
Account.Id newId;
|
||||
try (ManualRequestContext ctx = oneOffRequestContext.open()) {
|
||||
newId = new Account.Id(sequencesProvider.get().nextAccountId());
|
||||
}
|
||||
|
||||
ExternalId extId =
|
||||
ExternalId.createWithEmail(who.getExternalIdKey(), newId, who.getEmailAddress());
|
||||
|
@ -36,6 +36,7 @@ import com.google.gerrit.reviewdb.client.AccountGroup;
|
||||
import com.google.gerrit.reviewdb.client.AccountGroupMember;
|
||||
import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||
import com.google.gerrit.server.IdentifiedUser;
|
||||
import com.google.gerrit.server.Sequences;
|
||||
import com.google.gerrit.server.account.externalids.ExternalId;
|
||||
import com.google.gerrit.server.account.externalids.ExternalIds;
|
||||
import com.google.gerrit.server.account.externalids.ExternalIdsUpdate;
|
||||
@ -63,6 +64,7 @@ public class CreateAccount implements RestModifyView<TopLevelResource, AccountIn
|
||||
}
|
||||
|
||||
private final ReviewDb db;
|
||||
private final Sequences seq;
|
||||
private final Provider<IdentifiedUser> currentUser;
|
||||
private final GroupsCollection groupsCollection;
|
||||
private final VersionedAuthorizedKeys.Accessor authorizedKeys;
|
||||
@ -81,6 +83,7 @@ public class CreateAccount implements RestModifyView<TopLevelResource, AccountIn
|
||||
@Inject
|
||||
CreateAccount(
|
||||
ReviewDb db,
|
||||
Sequences seq,
|
||||
Provider<IdentifiedUser> currentUser,
|
||||
GroupsCollection groupsCollection,
|
||||
VersionedAuthorizedKeys.Accessor authorizedKeys,
|
||||
@ -96,6 +99,7 @@ public class CreateAccount implements RestModifyView<TopLevelResource, AccountIn
|
||||
OutgoingEmailValidator validator,
|
||||
@Assisted String username) {
|
||||
this.db = db;
|
||||
this.seq = seq;
|
||||
this.currentUser = currentUser;
|
||||
this.groupsCollection = groupsCollection;
|
||||
this.authorizedKeys = authorizedKeys;
|
||||
@ -133,7 +137,7 @@ public class CreateAccount implements RestModifyView<TopLevelResource, AccountIn
|
||||
|
||||
Set<AccountGroup.Id> groups = parseGroups(input.groups);
|
||||
|
||||
Account.Id id = new Account.Id(db.nextAccountId());
|
||||
Account.Id id = new Account.Id(seq.nextAccountId());
|
||||
|
||||
ExternalId extUser = ExternalId.createUsername(username, id, input.httpPassword);
|
||||
if (externalIds.get(extUser.key()) != null) {
|
||||
|
@ -33,6 +33,7 @@ public final class SitePaths {
|
||||
public final Path site_path;
|
||||
public final Path bin_dir;
|
||||
public final Path etc_dir;
|
||||
public final Path git_dir;
|
||||
public final Path lib_dir;
|
||||
public final Path tmp_dir;
|
||||
public final Path logs_dir;
|
||||
@ -78,6 +79,7 @@ public final class SitePaths {
|
||||
|
||||
bin_dir = p.resolve("bin");
|
||||
etc_dir = p.resolve("etc");
|
||||
git_dir = p.resolve("git");
|
||||
lib_dir = p.resolve("lib");
|
||||
tmp_dir = p.resolve("tmp");
|
||||
plugins_dir = p.resolve("plugins");
|
||||
|
@ -194,6 +194,27 @@ public class RepoSequence {
|
||||
}
|
||||
}
|
||||
|
||||
public void increaseTo(int val) throws OrmException {
|
||||
counterLock.lock();
|
||||
try {
|
||||
try (Repository repo = repoManager.openRepository(projectName);
|
||||
RevWalk rw = new RevWalk(repo)) {
|
||||
TryIncreaseTo attempt = new TryIncreaseTo(repo, rw, val);
|
||||
checkResult(retryer.call(attempt));
|
||||
counter = limit;
|
||||
} catch (ExecutionException | RetryException e) {
|
||||
if (e.getCause() != null) {
|
||||
Throwables.throwIfInstanceOf(e.getCause(), OrmException.class);
|
||||
}
|
||||
throw new OrmException(e);
|
||||
} catch (IOException e) {
|
||||
throw new OrmException(e);
|
||||
}
|
||||
} finally {
|
||||
counterLock.unlock();
|
||||
}
|
||||
}
|
||||
|
||||
private void acquire(int count) throws OrmException {
|
||||
try (Repository repo = repoManager.openRepository(projectName);
|
||||
RevWalk rw = new RevWalk(repo)) {
|
||||
@ -213,7 +234,9 @@ public class RepoSequence {
|
||||
}
|
||||
|
||||
private void checkResult(RefUpdate.Result result) throws OrmException {
|
||||
if (result != RefUpdate.Result.NEW && result != RefUpdate.Result.FORCED) {
|
||||
if (result != RefUpdate.Result.NEW
|
||||
&& result != RefUpdate.Result.FORCED
|
||||
&& result != RefUpdate.Result.NO_CHANGE) {
|
||||
throw new OrmException("failed to update " + refName + ": " + result);
|
||||
}
|
||||
}
|
||||
@ -241,25 +264,55 @@ public class RepoSequence {
|
||||
next = seed.get();
|
||||
} else {
|
||||
oldId = ref.getObjectId();
|
||||
next = parse(oldId);
|
||||
next = parse(rw, oldId);
|
||||
}
|
||||
return store(repo, rw, oldId, next + count);
|
||||
}
|
||||
}
|
||||
|
||||
private int parse(ObjectId id) throws IOException, OrmException {
|
||||
ObjectLoader ol = rw.getObjectReader().open(id, OBJ_BLOB);
|
||||
if (ol.getType() != OBJ_BLOB) {
|
||||
// In theory this should be thrown by open but not all implementations
|
||||
// may do it properly (certainly InMemoryRepository doesn't).
|
||||
throw new IncorrectObjectTypeException(id, OBJ_BLOB);
|
||||
}
|
||||
String str = CharMatcher.whitespace().trimFrom(new String(ol.getCachedBytes(), UTF_8));
|
||||
Integer val = Ints.tryParse(str);
|
||||
if (val == null) {
|
||||
throw new OrmException("invalid value in " + refName + " blob at " + id.name());
|
||||
}
|
||||
return val;
|
||||
private class TryIncreaseTo implements Callable<RefUpdate.Result> {
|
||||
private final Repository repo;
|
||||
private final RevWalk rw;
|
||||
private final int value;
|
||||
|
||||
private TryIncreaseTo(Repository repo, RevWalk rw, int value) {
|
||||
this.repo = repo;
|
||||
this.rw = rw;
|
||||
this.value = value;
|
||||
}
|
||||
|
||||
@Override
|
||||
public RefUpdate.Result call() throws Exception {
|
||||
Ref ref = repo.exactRef(refName);
|
||||
afterReadRef.run();
|
||||
ObjectId oldId;
|
||||
if (ref == null) {
|
||||
oldId = ObjectId.zeroId();
|
||||
} else {
|
||||
oldId = ref.getObjectId();
|
||||
int next = parse(rw, oldId);
|
||||
if (next >= value) {
|
||||
// a concurrent write updated the ref already to this or a higher value
|
||||
return RefUpdate.Result.NO_CHANGE;
|
||||
}
|
||||
}
|
||||
return store(repo, rw, oldId, value);
|
||||
}
|
||||
}
|
||||
|
||||
private int parse(RevWalk rw, ObjectId id) throws IOException, OrmException {
|
||||
ObjectLoader ol = rw.getObjectReader().open(id, OBJ_BLOB);
|
||||
if (ol.getType() != OBJ_BLOB) {
|
||||
// In theory this should be thrown by open but not all implementations
|
||||
// may do it properly (certainly InMemoryRepository doesn't).
|
||||
throw new IncorrectObjectTypeException(id, OBJ_BLOB);
|
||||
}
|
||||
String str = CharMatcher.whitespace().trimFrom(new String(ol.getCachedBytes(), UTF_8));
|
||||
Integer val = Ints.tryParse(str);
|
||||
if (val == null) {
|
||||
throw new OrmException("invalid value in " + refName + " blob at " + id.name());
|
||||
}
|
||||
return val;
|
||||
}
|
||||
|
||||
private RefUpdate.Result store(Repository repo, RevWalk rw, @Nullable ObjectId oldId, int val)
|
||||
|
@ -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_154> C = Schema_154.class;
|
||||
public static final Class<Schema_155> C = Schema_155.class;
|
||||
|
||||
public static int getBinaryVersion() {
|
||||
return guessVersion(C);
|
||||
|
@ -0,0 +1,50 @@
|
||||
// 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.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 account sequence in NoteDb */
|
||||
public class Schema_155 extends SchemaVersion {
|
||||
private final GitRepositoryManager repoManager;
|
||||
private final AllUsersName allUsersName;
|
||||
|
||||
@Inject
|
||||
Schema_155(
|
||||
Provider<Schema_154> 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 accountSeed = () -> db.nextAccountId();
|
||||
RepoSequence accountSeq =
|
||||
new RepoSequence(repoManager, allUsersName, Sequences.NAME_ACCOUNTS, accountSeed, 1);
|
||||
|
||||
// consume one account ID to ensure that the account sequence is initialized in NoteDb
|
||||
accountSeq.next();
|
||||
}
|
||||
}
|
@ -253,6 +253,95 @@ public class RepoSequenceTest {
|
||||
assertThat(s2.acquireCount).isEqualTo(1);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void increaseTo() throws Exception {
|
||||
// Seed existing ref value.
|
||||
writeBlob("id", "1");
|
||||
|
||||
RepoSequence s = newSequence("id", 1, 10);
|
||||
|
||||
s.increaseTo(2);
|
||||
assertThat(s.next()).isEqualTo(2);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void increaseToLowerValueIsIgnored() throws Exception {
|
||||
// Seed existing ref value.
|
||||
writeBlob("id", "2");
|
||||
|
||||
RepoSequence s = newSequence("id", 1, 10);
|
||||
|
||||
s.increaseTo(1);
|
||||
assertThat(s.next()).isEqualTo(2);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void increaseToRetryOnLockFailureV1() throws Exception {
|
||||
// Seed existing ref value.
|
||||
writeBlob("id", "1");
|
||||
|
||||
AtomicBoolean doneBgUpdate = new AtomicBoolean(false);
|
||||
Runnable bgUpdate =
|
||||
() -> {
|
||||
if (!doneBgUpdate.getAndSet(true)) {
|
||||
writeBlob("id", "2");
|
||||
}
|
||||
};
|
||||
|
||||
RepoSequence s = newSequence("id", 1, 10, bgUpdate, RETRYER);
|
||||
assertThat(doneBgUpdate.get()).isFalse();
|
||||
|
||||
// Increase the value to 3. The background thread increases the value to 2, which makes the
|
||||
// increase to value 3 fail once with LockFailure. The increase to 3 is then retried and is
|
||||
// expected to succeed.
|
||||
s.increaseTo(3);
|
||||
assertThat(s.next()).isEqualTo(3);
|
||||
|
||||
assertThat(doneBgUpdate.get()).isTrue();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void increaseToRetryOnLockFailureV2() throws Exception {
|
||||
// Seed existing ref value.
|
||||
writeBlob("id", "1");
|
||||
|
||||
AtomicBoolean doneBgUpdate = new AtomicBoolean(false);
|
||||
Runnable bgUpdate =
|
||||
() -> {
|
||||
if (!doneBgUpdate.getAndSet(true)) {
|
||||
writeBlob("id", "3");
|
||||
}
|
||||
};
|
||||
|
||||
RepoSequence s = newSequence("id", 1, 10, bgUpdate, RETRYER);
|
||||
assertThat(doneBgUpdate.get()).isFalse();
|
||||
|
||||
// Increase the value to 2. The background thread increases the value to 3, which makes the
|
||||
// increase to value 2 fail with LockFailure. The increase to 2 is then not retried because the
|
||||
// current value is already higher and it should be preserved.
|
||||
s.increaseTo(2);
|
||||
assertThat(s.next()).isEqualTo(3);
|
||||
|
||||
assertThat(doneBgUpdate.get()).isTrue();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void increaseToFailAfterRetryerGivesUp() throws Exception {
|
||||
AtomicInteger bgCounter = new AtomicInteger(1234);
|
||||
RepoSequence s =
|
||||
newSequence(
|
||||
"id",
|
||||
1,
|
||||
10,
|
||||
() -> writeBlob("id", Integer.toString(bgCounter.getAndAdd(1000))),
|
||||
RetryerBuilder.<RefUpdate.Result>newBuilder()
|
||||
.withStopStrategy(StopStrategies.stopAfterAttempt(3))
|
||||
.build());
|
||||
exception.expect(OrmException.class);
|
||||
exception.expectMessage("failed to update refs/sequences/id: LOCK_FAILURE");
|
||||
s.increaseTo(2);
|
||||
}
|
||||
|
||||
private RepoSequence newSequence(String name, int start, int batchSize) {
|
||||
return newSequence(name, start, batchSize, Runnables.doNothing(), RETRYER);
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user