Merge changes I552aa95c,Ie65aeb40,I7cf2b92f
* changes: Add Javadoc and use a more general name for SchemaUpgradeTestEnvironment Use retry mechanism for group creations and name updates RetryHelper: Let callers care about exception handling
This commit is contained in:
@@ -19,6 +19,7 @@ import static com.google.common.base.Preconditions.checkState;
|
|||||||
|
|
||||||
import com.google.common.annotations.VisibleForTesting;
|
import com.google.common.annotations.VisibleForTesting;
|
||||||
import com.google.common.base.Strings;
|
import com.google.common.base.Strings;
|
||||||
|
import com.google.common.base.Throwables;
|
||||||
import com.google.common.collect.Iterables;
|
import com.google.common.collect.Iterables;
|
||||||
import com.google.common.collect.Lists;
|
import com.google.common.collect.Lists;
|
||||||
import com.google.common.util.concurrent.Runnables;
|
import com.google.common.util.concurrent.Runnables;
|
||||||
@@ -37,6 +38,7 @@ import com.google.gerrit.server.git.MetaDataUpdate;
|
|||||||
import com.google.gerrit.server.index.change.ReindexAfterRefUpdate;
|
import com.google.gerrit.server.index.change.ReindexAfterRefUpdate;
|
||||||
import com.google.gerrit.server.update.RefUpdateUtil;
|
import com.google.gerrit.server.update.RefUpdateUtil;
|
||||||
import com.google.gerrit.server.update.RetryHelper;
|
import com.google.gerrit.server.update.RetryHelper;
|
||||||
|
import com.google.gerrit.server.update.RetryHelper.Action;
|
||||||
import com.google.gerrit.server.update.RetryHelper.ActionType;
|
import com.google.gerrit.server.update.RetryHelper.ActionType;
|
||||||
import com.google.gwtorm.server.OrmDuplicateKeyException;
|
import com.google.gwtorm.server.OrmDuplicateKeyException;
|
||||||
import com.google.gwtorm.server.OrmException;
|
import com.google.gwtorm.server.OrmException;
|
||||||
@@ -498,8 +500,7 @@ public class AccountsUpdate {
|
|||||||
|
|
||||||
private Optional<AccountState> updateAccount(AccountUpdate accountUpdate)
|
private Optional<AccountState> updateAccount(AccountUpdate accountUpdate)
|
||||||
throws IOException, ConfigInvalidException, OrmException {
|
throws IOException, ConfigInvalidException, OrmException {
|
||||||
return retryHelper.execute(
|
return executeAccountUpdate(
|
||||||
ActionType.ACCOUNT_UPDATE,
|
|
||||||
() -> {
|
() -> {
|
||||||
try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) {
|
try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) {
|
||||||
UpdatedAccount updatedAccount = accountUpdate.update(allUsersRepo);
|
UpdatedAccount updatedAccount = accountUpdate.update(allUsersRepo);
|
||||||
@@ -513,6 +514,20 @@ public class AccountsUpdate {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private Optional<AccountState> executeAccountUpdate(Action<Optional<AccountState>> action)
|
||||||
|
throws IOException, ConfigInvalidException, OrmException {
|
||||||
|
try {
|
||||||
|
return retryHelper.execute(
|
||||||
|
ActionType.ACCOUNT_UPDATE, action, LockFailureException.class::isInstance);
|
||||||
|
} catch (Exception e) {
|
||||||
|
Throwables.throwIfUnchecked(e);
|
||||||
|
Throwables.throwIfInstanceOf(e, IOException.class);
|
||||||
|
Throwables.throwIfInstanceOf(e, ConfigInvalidException.class);
|
||||||
|
Throwables.throwIfInstanceOf(e, OrmException.class);
|
||||||
|
throw new OrmException(e);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private ExternalIdNotes createExternalIdNotes(
|
private ExternalIdNotes createExternalIdNotes(
|
||||||
Repository allUsersRepo,
|
Repository allUsersRepo,
|
||||||
Optional<ObjectId> rev,
|
Optional<ObjectId> rev,
|
||||||
|
@@ -41,6 +41,7 @@ import static org.eclipse.jgit.transport.ReceiveCommand.Result.REJECTED_OTHER_RE
|
|||||||
import com.google.common.base.Function;
|
import com.google.common.base.Function;
|
||||||
import com.google.common.base.Splitter;
|
import com.google.common.base.Splitter;
|
||||||
import com.google.common.base.Strings;
|
import com.google.common.base.Strings;
|
||||||
|
import com.google.common.base.Throwables;
|
||||||
import com.google.common.collect.BiMap;
|
import com.google.common.collect.BiMap;
|
||||||
import com.google.common.collect.HashBiMap;
|
import com.google.common.collect.HashBiMap;
|
||||||
import com.google.common.collect.ImmutableList;
|
import com.google.common.collect.ImmutableList;
|
||||||
@@ -149,6 +150,7 @@ import com.google.gerrit.server.update.Context;
|
|||||||
import com.google.gerrit.server.update.RepoContext;
|
import com.google.gerrit.server.update.RepoContext;
|
||||||
import com.google.gerrit.server.update.RepoOnlyOp;
|
import com.google.gerrit.server.update.RepoOnlyOp;
|
||||||
import com.google.gerrit.server.update.RetryHelper;
|
import com.google.gerrit.server.update.RetryHelper;
|
||||||
|
import com.google.gerrit.server.update.RetryHelper.Action;
|
||||||
import com.google.gerrit.server.update.RetryHelper.ActionType;
|
import com.google.gerrit.server.update.RetryHelper.ActionType;
|
||||||
import com.google.gerrit.server.update.UpdateException;
|
import com.google.gerrit.server.update.UpdateException;
|
||||||
import com.google.gerrit.server.util.LabelVote;
|
import com.google.gerrit.server.util.LabelVote;
|
||||||
@@ -2891,10 +2893,7 @@ class ReceiveCommits {
|
|||||||
for (Ref ref : byCommit.get(c.copy())) {
|
for (Ref ref : byCommit.get(c.copy())) {
|
||||||
PatchSet.Id psId = PatchSet.Id.fromRef(ref.getName());
|
PatchSet.Id psId = PatchSet.Id.fromRef(ref.getName());
|
||||||
Optional<ChangeData> cd =
|
Optional<ChangeData> cd =
|
||||||
retryHelper.execute(
|
executeIndexQuery(() -> byLegacyId(psId.getParentKey()));
|
||||||
ActionType.CHANGE_QUERY,
|
|
||||||
() -> byLegacyId(psId.getParentKey()),
|
|
||||||
t -> t instanceof OrmException);
|
|
||||||
if (cd.isPresent() && cd.get().change().getDest().equals(branch)) {
|
if (cd.isPresent() && cd.get().change().getDest().equals(branch)) {
|
||||||
existingPatchSets++;
|
existingPatchSets++;
|
||||||
bu.addOp(
|
bu.addOp(
|
||||||
@@ -2906,11 +2905,7 @@ class ReceiveCommits {
|
|||||||
|
|
||||||
for (String changeId : c.getFooterLines(CHANGE_ID)) {
|
for (String changeId : c.getFooterLines(CHANGE_ID)) {
|
||||||
if (byKey == null) {
|
if (byKey == null) {
|
||||||
byKey =
|
byKey = executeIndexQuery(() -> openChangesByKeyByBranch(branch));
|
||||||
retryHelper.execute(
|
|
||||||
ActionType.CHANGE_QUERY,
|
|
||||||
() -> openChangesByKeyByBranch(branch),
|
|
||||||
t -> t instanceof OrmException);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
ChangeNotes onto = byKey.get(new Change.Key(changeId.trim()));
|
ChangeNotes onto = byKey.get(new Change.Key(changeId.trim()));
|
||||||
@@ -2967,6 +2962,16 @@ class ReceiveCommits {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private <T> T executeIndexQuery(Action<T> action) throws OrmException {
|
||||||
|
try {
|
||||||
|
return retryHelper.execute(ActionType.INDEX_QUERY, action, OrmException.class::isInstance);
|
||||||
|
} catch (Exception e) {
|
||||||
|
Throwables.throwIfUnchecked(e);
|
||||||
|
Throwables.throwIfInstanceOf(e, OrmException.class);
|
||||||
|
throw new OrmException(e);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private void updateAccountInfo() {
|
private void updateAccountInfo() {
|
||||||
if (setFullNameTo == null) {
|
if (setFullNameTo == null) {
|
||||||
return;
|
return;
|
||||||
|
@@ -20,6 +20,7 @@ import static com.google.gerrit.server.group.db.Groups.getExistingGroupFromRevie
|
|||||||
import com.google.auto.value.AutoValue;
|
import com.google.auto.value.AutoValue;
|
||||||
import com.google.common.annotations.VisibleForTesting;
|
import com.google.common.annotations.VisibleForTesting;
|
||||||
import com.google.common.base.Strings;
|
import com.google.common.base.Strings;
|
||||||
|
import com.google.common.base.Throwables;
|
||||||
import com.google.common.collect.ImmutableList;
|
import com.google.common.collect.ImmutableList;
|
||||||
import com.google.common.collect.ImmutableSet;
|
import com.google.common.collect.ImmutableSet;
|
||||||
import com.google.common.collect.Sets;
|
import com.google.common.collect.Sets;
|
||||||
@@ -48,11 +49,13 @@ import com.google.gerrit.server.config.GerritServerConfig;
|
|||||||
import com.google.gerrit.server.config.GerritServerId;
|
import com.google.gerrit.server.config.GerritServerId;
|
||||||
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
|
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
|
||||||
import com.google.gerrit.server.git.GitRepositoryManager;
|
import com.google.gerrit.server.git.GitRepositoryManager;
|
||||||
|
import com.google.gerrit.server.git.LockFailureException;
|
||||||
import com.google.gerrit.server.git.MetaDataUpdate;
|
import com.google.gerrit.server.git.MetaDataUpdate;
|
||||||
import com.google.gerrit.server.git.RenameGroupOp;
|
import com.google.gerrit.server.git.RenameGroupOp;
|
||||||
import com.google.gerrit.server.group.InternalGroup;
|
import com.google.gerrit.server.group.InternalGroup;
|
||||||
import com.google.gerrit.server.notedb.GroupsMigration;
|
import com.google.gerrit.server.notedb.GroupsMigration;
|
||||||
import com.google.gerrit.server.update.RefUpdateUtil;
|
import com.google.gerrit.server.update.RefUpdateUtil;
|
||||||
|
import com.google.gerrit.server.update.RetryHelper;
|
||||||
import com.google.gwtorm.server.OrmDuplicateKeyException;
|
import com.google.gwtorm.server.OrmDuplicateKeyException;
|
||||||
import com.google.gwtorm.server.OrmException;
|
import com.google.gwtorm.server.OrmException;
|
||||||
import com.google.inject.Inject;
|
import com.google.inject.Inject;
|
||||||
@@ -111,6 +114,7 @@ public class GroupsUpdate {
|
|||||||
private final MetaDataUpdateFactory metaDataUpdateFactory;
|
private final MetaDataUpdateFactory metaDataUpdateFactory;
|
||||||
private final GroupsMigration groupsMigration;
|
private final GroupsMigration groupsMigration;
|
||||||
private final GitReferenceUpdated gitRefUpdated;
|
private final GitReferenceUpdated gitRefUpdated;
|
||||||
|
private final RetryHelper retryHelper;
|
||||||
private final boolean reviewDbUpdatesAreBlocked;
|
private final boolean reviewDbUpdatesAreBlocked;
|
||||||
|
|
||||||
@Inject
|
@Inject
|
||||||
@@ -129,6 +133,7 @@ public class GroupsUpdate {
|
|||||||
GroupsMigration groupsMigration,
|
GroupsMigration groupsMigration,
|
||||||
@GerritServerConfig Config config,
|
@GerritServerConfig Config config,
|
||||||
GitReferenceUpdated gitRefUpdated,
|
GitReferenceUpdated gitRefUpdated,
|
||||||
|
RetryHelper retryHelper,
|
||||||
@Assisted @Nullable IdentifiedUser currentUser) {
|
@Assisted @Nullable IdentifiedUser currentUser) {
|
||||||
this.repoManager = repoManager;
|
this.repoManager = repoManager;
|
||||||
this.allUsersName = allUsersName;
|
this.allUsersName = allUsersName;
|
||||||
@@ -141,6 +146,7 @@ public class GroupsUpdate {
|
|||||||
this.serverId = serverId;
|
this.serverId = serverId;
|
||||||
this.groupsMigration = groupsMigration;
|
this.groupsMigration = groupsMigration;
|
||||||
this.gitRefUpdated = gitRefUpdated;
|
this.gitRefUpdated = gitRefUpdated;
|
||||||
|
this.retryHelper = retryHelper;
|
||||||
this.currentUser = currentUser;
|
this.currentUser = currentUser;
|
||||||
metaDataUpdateFactory =
|
metaDataUpdateFactory =
|
||||||
getMetaDataUpdateFactory(metaDataUpdateInternalFactory, currentUser, serverIdent, serverId);
|
getMetaDataUpdateFactory(metaDataUpdateInternalFactory, currentUser, serverIdent, serverId);
|
||||||
@@ -220,8 +226,7 @@ public class GroupsUpdate {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// TODO(aliceks): Add retry mechanism.
|
InternalGroup createdGroup = createGroupInNoteDbWithRetry(groupCreation, groupUpdate);
|
||||||
InternalGroup createdGroup = createGroupInNoteDb(groupCreation, groupUpdate);
|
|
||||||
updateCachesOnGroupCreation(createdGroup);
|
updateCachesOnGroupCreation(createdGroup);
|
||||||
return createdGroup;
|
return createdGroup;
|
||||||
}
|
}
|
||||||
@@ -265,8 +270,8 @@ public class GroupsUpdate {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// TODO(aliceks): Add retry mechanism.
|
Optional<UpdateResult> noteDbUpdateResult =
|
||||||
Optional<UpdateResult> noteDbUpdateResult = updateGroupInNoteDb(groupUuid, groupUpdate);
|
updateGroupInNoteDbWithRetry(groupUuid, groupUpdate);
|
||||||
return noteDbUpdateResult.orElse(reviewDbUpdateResult);
|
return noteDbUpdateResult.orElse(reviewDbUpdateResult);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -476,9 +481,26 @@ public class GroupsUpdate {
|
|||||||
db.accountGroupById().delete(subgroupsToRemove);
|
db.accountGroupById().delete(subgroupsToRemove);
|
||||||
}
|
}
|
||||||
|
|
||||||
private InternalGroup createGroupInNoteDb(
|
private InternalGroup createGroupInNoteDbWithRetry(
|
||||||
InternalGroupCreation groupCreation, InternalGroupUpdate groupUpdate)
|
InternalGroupCreation groupCreation, InternalGroupUpdate groupUpdate)
|
||||||
throws IOException, ConfigInvalidException, OrmException {
|
throws IOException, ConfigInvalidException, OrmException {
|
||||||
|
try {
|
||||||
|
return retryHelper.execute(
|
||||||
|
RetryHelper.ActionType.GROUP_UPDATE,
|
||||||
|
() -> createGroupInNoteDb(groupCreation, groupUpdate),
|
||||||
|
LockFailureException.class::isInstance);
|
||||||
|
} catch (Exception e) {
|
||||||
|
Throwables.throwIfUnchecked(e);
|
||||||
|
Throwables.throwIfInstanceOf(e, IOException.class);
|
||||||
|
Throwables.throwIfInstanceOf(e, ConfigInvalidException.class);
|
||||||
|
Throwables.throwIfInstanceOf(e, OrmDuplicateKeyException.class);
|
||||||
|
throw new IOException(e);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private InternalGroup createGroupInNoteDb(
|
||||||
|
InternalGroupCreation groupCreation, InternalGroupUpdate groupUpdate)
|
||||||
|
throws IOException, ConfigInvalidException, OrmDuplicateKeyException {
|
||||||
try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) {
|
try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) {
|
||||||
AccountGroup.NameKey groupName = groupUpdate.getName().orElseGet(groupCreation::getNameKey);
|
AccountGroup.NameKey groupName = groupUpdate.getName().orElseGet(groupCreation::getNameKey);
|
||||||
GroupNameNotes groupNameNotes =
|
GroupNameNotes groupNameNotes =
|
||||||
@@ -496,6 +518,24 @@ public class GroupsUpdate {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private Optional<UpdateResult> updateGroupInNoteDbWithRetry(
|
||||||
|
AccountGroup.UUID groupUuid, InternalGroupUpdate groupUpdate)
|
||||||
|
throws IOException, ConfigInvalidException, OrmDuplicateKeyException, NoSuchGroupException {
|
||||||
|
try {
|
||||||
|
return retryHelper.execute(
|
||||||
|
RetryHelper.ActionType.GROUP_UPDATE,
|
||||||
|
() -> updateGroupInNoteDb(groupUuid, groupUpdate),
|
||||||
|
LockFailureException.class::isInstance);
|
||||||
|
} catch (Exception e) {
|
||||||
|
Throwables.throwIfUnchecked(e);
|
||||||
|
Throwables.throwIfInstanceOf(e, IOException.class);
|
||||||
|
Throwables.throwIfInstanceOf(e, ConfigInvalidException.class);
|
||||||
|
Throwables.throwIfInstanceOf(e, OrmDuplicateKeyException.class);
|
||||||
|
Throwables.throwIfInstanceOf(e, NoSuchGroupException.class);
|
||||||
|
throw new IOException(e);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private Optional<UpdateResult> updateGroupInNoteDb(
|
private Optional<UpdateResult> updateGroupInNoteDb(
|
||||||
AccountGroup.UUID groupUuid, InternalGroupUpdate groupUpdate)
|
AccountGroup.UUID groupUuid, InternalGroupUpdate groupUpdate)
|
||||||
throws IOException, ConfigInvalidException, OrmDuplicateKeyException, NoSuchGroupException {
|
throws IOException, ConfigInvalidException, OrmDuplicateKeyException, NoSuchGroupException {
|
||||||
|
@@ -40,14 +40,11 @@ import com.google.gerrit.metrics.MetricMaker;
|
|||||||
import com.google.gerrit.server.config.GerritServerConfig;
|
import com.google.gerrit.server.config.GerritServerConfig;
|
||||||
import com.google.gerrit.server.git.LockFailureException;
|
import com.google.gerrit.server.git.LockFailureException;
|
||||||
import com.google.gerrit.server.notedb.NotesMigration;
|
import com.google.gerrit.server.notedb.NotesMigration;
|
||||||
import com.google.gwtorm.server.OrmException;
|
|
||||||
import com.google.inject.Inject;
|
import com.google.inject.Inject;
|
||||||
import com.google.inject.Singleton;
|
import com.google.inject.Singleton;
|
||||||
import java.io.IOException;
|
|
||||||
import java.time.Duration;
|
import java.time.Duration;
|
||||||
import java.util.concurrent.ExecutionException;
|
import java.util.concurrent.ExecutionException;
|
||||||
import java.util.function.Consumer;
|
import java.util.function.Consumer;
|
||||||
import org.eclipse.jgit.errors.ConfigInvalidException;
|
|
||||||
import org.eclipse.jgit.lib.Config;
|
import org.eclipse.jgit.lib.Config;
|
||||||
|
|
||||||
@Singleton
|
@Singleton
|
||||||
@@ -64,8 +61,9 @@ public class RetryHelper {
|
|||||||
|
|
||||||
public enum ActionType {
|
public enum ActionType {
|
||||||
ACCOUNT_UPDATE,
|
ACCOUNT_UPDATE,
|
||||||
CHANGE_QUERY,
|
CHANGE_UPDATE,
|
||||||
CHANGE_UPDATE
|
GROUP_UPDATE,
|
||||||
|
INDEX_QUERY
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -182,22 +180,24 @@ public class RetryHelper {
|
|||||||
return defaultTimeout;
|
return defaultTimeout;
|
||||||
}
|
}
|
||||||
|
|
||||||
public <T> T execute(ActionType actionType, Action<T> action)
|
public <T> T execute(
|
||||||
throws IOException, ConfigInvalidException, OrmException {
|
ActionType actionType, Action<T> action, Predicate<Throwable> exceptionPredicate)
|
||||||
return execute(actionType, action, t -> t instanceof LockFailureException);
|
throws Exception {
|
||||||
|
return execute(actionType, action, defaults(), exceptionPredicate);
|
||||||
}
|
}
|
||||||
|
|
||||||
public <T> T execute(
|
public <T> T execute(
|
||||||
ActionType actionType, Action<T> action, Predicate<Throwable> exceptionPredicate)
|
ActionType actionType,
|
||||||
throws IOException, ConfigInvalidException, OrmException {
|
Action<T> action,
|
||||||
|
Options opts,
|
||||||
|
Predicate<Throwable> exceptionPredicate)
|
||||||
|
throws Exception {
|
||||||
try {
|
try {
|
||||||
return execute(actionType, action, defaults(), exceptionPredicate);
|
return executeWithAttemptAndTimeoutCount(actionType, action, opts, exceptionPredicate);
|
||||||
} catch (Throwable t) {
|
} catch (Throwable t) {
|
||||||
Throwables.throwIfUnchecked(t);
|
Throwables.throwIfUnchecked(t);
|
||||||
Throwables.throwIfInstanceOf(t, IOException.class);
|
Throwables.throwIfInstanceOf(t, Exception.class);
|
||||||
Throwables.throwIfInstanceOf(t, ConfigInvalidException.class);
|
throw new IllegalStateException(t);
|
||||||
Throwables.throwIfInstanceOf(t, OrmException.class);
|
|
||||||
throw new OrmException(t);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -212,7 +212,7 @@ public class RetryHelper {
|
|||||||
// Either we aren't full-NoteDb, or the underlying ref storage doesn't support atomic
|
// Either we aren't full-NoteDb, or the underlying ref storage doesn't support atomic
|
||||||
// transactions. Either way, retrying a partially-failed operation is not idempotent, so
|
// transactions. Either way, retrying a partially-failed operation is not idempotent, so
|
||||||
// don't do it automatically. Let the end user decide whether they want to retry.
|
// don't do it automatically. Let the end user decide whether they want to retry.
|
||||||
return execute(
|
return executeWithTimeoutCount(
|
||||||
ActionType.CHANGE_UPDATE,
|
ActionType.CHANGE_UPDATE,
|
||||||
() -> changeAction.call(updateFactory),
|
() -> changeAction.call(updateFactory),
|
||||||
RetryerBuilder.<T>newBuilder().build());
|
RetryerBuilder.<T>newBuilder().build());
|
||||||
@@ -237,7 +237,7 @@ public class RetryHelper {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Executes an action with a given retryer.
|
* Executes an action and records the number of attempts and the timeout as metrics.
|
||||||
*
|
*
|
||||||
* @param actionType the type of the action
|
* @param actionType the type of the action
|
||||||
* @param action the action which should be executed and retried on failure
|
* @param action the action which should be executed and retried on failure
|
||||||
@@ -247,7 +247,7 @@ public class RetryHelper {
|
|||||||
* @throws Throwable any error or exception that made the action fail, callers are expected to
|
* @throws Throwable any error or exception that made the action fail, callers are expected to
|
||||||
* catch and inspect this Throwable to decide carefully whether it should be re-thrown
|
* catch and inspect this Throwable to decide carefully whether it should be re-thrown
|
||||||
*/
|
*/
|
||||||
private <T> T execute(
|
private <T> T executeWithAttemptAndTimeoutCount(
|
||||||
ActionType actionType,
|
ActionType actionType,
|
||||||
Action<T> action,
|
Action<T> action,
|
||||||
Options opts,
|
Options opts,
|
||||||
@@ -257,14 +257,14 @@ public class RetryHelper {
|
|||||||
try {
|
try {
|
||||||
RetryerBuilder<T> retryerBuilder = createRetryerBuilder(opts, exceptionPredicate);
|
RetryerBuilder<T> retryerBuilder = createRetryerBuilder(opts, exceptionPredicate);
|
||||||
retryerBuilder.withRetryListener(listener);
|
retryerBuilder.withRetryListener(listener);
|
||||||
return execute(actionType, action, retryerBuilder.build());
|
return executeWithTimeoutCount(actionType, action, retryerBuilder.build());
|
||||||
} finally {
|
} finally {
|
||||||
metrics.attemptCounts.record(actionType, listener.getAttemptCount());
|
metrics.attemptCounts.record(actionType, listener.getAttemptCount());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Executes an action with a given retryer.
|
* Executes an action and records the timeout as metric.
|
||||||
*
|
*
|
||||||
* @param actionType the type of the action
|
* @param actionType the type of the action
|
||||||
* @param action the action which should be executed and retried on failure
|
* @param action the action which should be executed and retried on failure
|
||||||
@@ -273,7 +273,7 @@ public class RetryHelper {
|
|||||||
* @throws Throwable any error or exception that made the action fail, callers are expected to
|
* @throws Throwable any error or exception that made the action fail, callers are expected to
|
||||||
* catch and inspect this Throwable to decide carefully whether it should be re-thrown
|
* catch and inspect this Throwable to decide carefully whether it should be re-thrown
|
||||||
*/
|
*/
|
||||||
private <T> T execute(ActionType actionType, Action<T> action, Retryer<T> retryer)
|
private <T> T executeWithTimeoutCount(ActionType actionType, Action<T> action, Retryer<T> retryer)
|
||||||
throws Throwable {
|
throws Throwable {
|
||||||
try {
|
try {
|
||||||
return retryer.call(() -> action.call());
|
return retryer.call(() -> action.call());
|
||||||
|
@@ -35,7 +35,16 @@ import org.junit.rules.MethodRule;
|
|||||||
import org.junit.runners.model.FrameworkMethod;
|
import org.junit.runners.model.FrameworkMethod;
|
||||||
import org.junit.runners.model.Statement;
|
import org.junit.runners.model.Statement;
|
||||||
|
|
||||||
public final class SchemaUpgradeTestEnvironment implements MethodRule {
|
/**
|
||||||
|
* An in-memory test environment for integration tests.
|
||||||
|
*
|
||||||
|
* <p>This test environment emulates the internals of a Gerrit server without starting a Gerrit
|
||||||
|
* site. ReviewDb as well as NoteDb are represented by in-memory representations.
|
||||||
|
*
|
||||||
|
* <p>Each test is executed with a fresh and clean test environment. Hence, modifications applied in
|
||||||
|
* one test don't carry over to subsequent ones.
|
||||||
|
*/
|
||||||
|
public final class InMemoryTestEnvironment implements MethodRule {
|
||||||
private final Provider<Config> configProvider;
|
private final Provider<Config> configProvider;
|
||||||
|
|
||||||
@Inject private AccountManager accountManager;
|
@Inject private AccountManager accountManager;
|
||||||
@@ -50,7 +59,7 @@ public final class SchemaUpgradeTestEnvironment implements MethodRule {
|
|||||||
private LifecycleManager lifecycle;
|
private LifecycleManager lifecycle;
|
||||||
|
|
||||||
/** Create a test environment using an empty base config. */
|
/** Create a test environment using an empty base config. */
|
||||||
public SchemaUpgradeTestEnvironment() {
|
public InMemoryTestEnvironment() {
|
||||||
this(Config::new);
|
this(Config::new);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -62,7 +71,7 @@ public final class SchemaUpgradeTestEnvironment implements MethodRule {
|
|||||||
*
|
*
|
||||||
* @param configProvider possibly-lazy provider for the base config.
|
* @param configProvider possibly-lazy provider for the base config.
|
||||||
*/
|
*/
|
||||||
public SchemaUpgradeTestEnvironment(Provider<Config> configProvider) {
|
public InMemoryTestEnvironment(Provider<Config> configProvider) {
|
||||||
this.configProvider = configProvider;
|
this.configProvider = configProvider;
|
||||||
}
|
}
|
||||||
|
|
@@ -0,0 +1,154 @@
|
|||||||
|
// Copyright (C) 2018 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.api.group;
|
||||||
|
|
||||||
|
import static com.google.common.truth.Truth8.assertThat;
|
||||||
|
import static com.google.gerrit.server.notedb.NoteDbTable.GROUPS;
|
||||||
|
import static com.google.gerrit.server.notedb.NotesMigration.DISABLE_REVIEW_DB;
|
||||||
|
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.collect.ImmutableSet;
|
||||||
|
import com.google.gerrit.common.data.GroupReference;
|
||||||
|
import com.google.gerrit.reviewdb.client.Account;
|
||||||
|
import com.google.gerrit.reviewdb.client.AccountGroup;
|
||||||
|
import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||||
|
import com.google.gerrit.server.group.ServerInitiated;
|
||||||
|
import com.google.gerrit.server.group.db.Groups;
|
||||||
|
import com.google.gerrit.server.group.db.GroupsUpdate;
|
||||||
|
import com.google.gerrit.server.group.db.InternalGroupCreation;
|
||||||
|
import com.google.gerrit.server.group.db.InternalGroupUpdate;
|
||||||
|
import com.google.gerrit.testing.InMemoryTestEnvironment;
|
||||||
|
import com.google.gwtorm.server.OrmException;
|
||||||
|
import com.google.inject.Inject;
|
||||||
|
import com.google.inject.Provider;
|
||||||
|
import java.io.IOException;
|
||||||
|
import java.util.Set;
|
||||||
|
import java.util.stream.Stream;
|
||||||
|
import org.eclipse.jgit.errors.ConfigInvalidException;
|
||||||
|
import org.eclipse.jgit.lib.Config;
|
||||||
|
import org.junit.Rule;
|
||||||
|
import org.junit.Test;
|
||||||
|
|
||||||
|
public class GroupsUpdateIT {
|
||||||
|
|
||||||
|
private static Config createPureNoteDbConfig() {
|
||||||
|
Config config = new Config();
|
||||||
|
config.setBoolean(SECTION_NOTE_DB, GROUPS.key(), WRITE, true);
|
||||||
|
config.setBoolean(SECTION_NOTE_DB, GROUPS.key(), READ, true);
|
||||||
|
config.setBoolean(SECTION_NOTE_DB, GROUPS.key(), DISABLE_REVIEW_DB, true);
|
||||||
|
return config;
|
||||||
|
}
|
||||||
|
|
||||||
|
@Rule
|
||||||
|
public InMemoryTestEnvironment testEnvironment =
|
||||||
|
new InMemoryTestEnvironment(GroupsUpdateIT::createPureNoteDbConfig);
|
||||||
|
|
||||||
|
@Inject @ServerInitiated private Provider<GroupsUpdate> groupsUpdateProvider;
|
||||||
|
@Inject private Groups groups;
|
||||||
|
@Inject private ReviewDb reviewDb;
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void groupCreationIsRetriedWhenFailedDueToConcurrentNameModification() throws Exception {
|
||||||
|
InternalGroupCreation groupCreation = getGroupCreation("users", "users-UUID");
|
||||||
|
InternalGroupUpdate groupUpdate =
|
||||||
|
InternalGroupUpdate.builder()
|
||||||
|
.setMemberModification(
|
||||||
|
new CreateAnotherGroupOnceAsSideEffectOfMemberModification("verifiers"))
|
||||||
|
.build();
|
||||||
|
createGroup(groupCreation, groupUpdate);
|
||||||
|
|
||||||
|
Stream<String> allGroupNames = getAllGroupNames();
|
||||||
|
assertThat(allGroupNames).containsAllOf("users", "verifiers");
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void groupRenameIsRetriedWhenFailedDueToConcurrentNameModification() throws Exception {
|
||||||
|
createGroup("users", "users-UUID");
|
||||||
|
|
||||||
|
InternalGroupUpdate groupUpdate =
|
||||||
|
InternalGroupUpdate.builder()
|
||||||
|
.setName(new AccountGroup.NameKey("contributors"))
|
||||||
|
.setMemberModification(
|
||||||
|
new CreateAnotherGroupOnceAsSideEffectOfMemberModification("verifiers"))
|
||||||
|
.build();
|
||||||
|
updateGroup(new AccountGroup.UUID("users-UUID"), groupUpdate);
|
||||||
|
|
||||||
|
Stream<String> allGroupNames = getAllGroupNames();
|
||||||
|
assertThat(allGroupNames).containsAllOf("contributors", "verifiers");
|
||||||
|
}
|
||||||
|
|
||||||
|
private void createGroup(String groupName, String groupUuid) throws Exception {
|
||||||
|
InternalGroupCreation groupCreation = getGroupCreation(groupName, groupUuid);
|
||||||
|
InternalGroupUpdate groupUpdate = InternalGroupUpdate.builder().build();
|
||||||
|
|
||||||
|
createGroup(groupCreation, groupUpdate);
|
||||||
|
}
|
||||||
|
|
||||||
|
private void createGroup(InternalGroupCreation groupCreation, InternalGroupUpdate groupUpdate)
|
||||||
|
throws OrmException, IOException, ConfigInvalidException {
|
||||||
|
groupsUpdateProvider.get().createGroup(reviewDb, groupCreation, groupUpdate);
|
||||||
|
}
|
||||||
|
|
||||||
|
private void updateGroup(AccountGroup.UUID groupUuid, InternalGroupUpdate groupUpdate)
|
||||||
|
throws Exception {
|
||||||
|
groupsUpdateProvider.get().updateGroup(reviewDb, groupUuid, groupUpdate);
|
||||||
|
}
|
||||||
|
|
||||||
|
private Stream<String> getAllGroupNames()
|
||||||
|
throws OrmException, IOException, ConfigInvalidException {
|
||||||
|
return groups.getAllGroupReferences(reviewDb).map(GroupReference::getName);
|
||||||
|
}
|
||||||
|
|
||||||
|
private static InternalGroupCreation getGroupCreation(String groupName, String groupUuid) {
|
||||||
|
return InternalGroupCreation.builder()
|
||||||
|
.setGroupUUID(new AccountGroup.UUID(groupUuid))
|
||||||
|
.setNameKey(new AccountGroup.NameKey(groupName))
|
||||||
|
.setId(new AccountGroup.Id(Math.abs(groupName.hashCode())))
|
||||||
|
.build();
|
||||||
|
}
|
||||||
|
|
||||||
|
private class CreateAnotherGroupOnceAsSideEffectOfMemberModification
|
||||||
|
implements InternalGroupUpdate.MemberModification {
|
||||||
|
|
||||||
|
private boolean groupCreated = false;
|
||||||
|
private String groupName;
|
||||||
|
|
||||||
|
public CreateAnotherGroupOnceAsSideEffectOfMemberModification(String groupName) {
|
||||||
|
this.groupName = groupName;
|
||||||
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public Set<Account.Id> apply(ImmutableSet<Account.Id> members) {
|
||||||
|
if (!groupCreated) {
|
||||||
|
createGroup();
|
||||||
|
groupCreated = true;
|
||||||
|
}
|
||||||
|
|
||||||
|
return members;
|
||||||
|
}
|
||||||
|
|
||||||
|
private void createGroup() {
|
||||||
|
InternalGroupCreation groupCreation = getGroupCreation(groupName, groupName + "-UUID");
|
||||||
|
InternalGroupUpdate groupUpdate = InternalGroupUpdate.builder().build();
|
||||||
|
try {
|
||||||
|
groupsUpdateProvider.get().createGroup(reviewDb, groupCreation, groupUpdate);
|
||||||
|
} catch (OrmException | IOException | ConfigInvalidException e) {
|
||||||
|
throw new IllegalStateException(e);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
@@ -25,7 +25,7 @@ import com.google.gerrit.reviewdb.client.AccountGroup;
|
|||||||
import com.google.gerrit.reviewdb.client.AccountGroup.Id;
|
import com.google.gerrit.reviewdb.client.AccountGroup.Id;
|
||||||
import com.google.gerrit.reviewdb.server.ReviewDb;
|
import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||||
import com.google.gerrit.server.restapi.group.CreateGroup;
|
import com.google.gerrit.server.restapi.group.CreateGroup;
|
||||||
import com.google.gerrit.testing.SchemaUpgradeTestEnvironment;
|
import com.google.gerrit.testing.InMemoryTestEnvironment;
|
||||||
import com.google.gerrit.testing.TestUpdateUI;
|
import com.google.gerrit.testing.TestUpdateUI;
|
||||||
import com.google.gwtorm.jdbc.JdbcSchema;
|
import com.google.gwtorm.jdbc.JdbcSchema;
|
||||||
import com.google.inject.Inject;
|
import com.google.inject.Inject;
|
||||||
@@ -44,7 +44,7 @@ import org.junit.Test;
|
|||||||
|
|
||||||
public class Schema_150_to_151_Test {
|
public class Schema_150_to_151_Test {
|
||||||
|
|
||||||
@Rule public SchemaUpgradeTestEnvironment testEnv = new SchemaUpgradeTestEnvironment();
|
@Rule public InMemoryTestEnvironment testEnv = new InMemoryTestEnvironment();
|
||||||
|
|
||||||
@Inject private CreateGroup.Factory createGroupFactory;
|
@Inject private CreateGroup.Factory createGroupFactory;
|
||||||
@Inject private Schema_151 schema151;
|
@Inject private Schema_151 schema151;
|
||||||
|
@@ -33,7 +33,7 @@ import com.google.gerrit.server.account.AccountCache;
|
|||||||
import com.google.gerrit.server.account.VersionedAccountPreferences;
|
import com.google.gerrit.server.account.VersionedAccountPreferences;
|
||||||
import com.google.gerrit.server.config.AllUsersName;
|
import com.google.gerrit.server.config.AllUsersName;
|
||||||
import com.google.gerrit.server.git.GitRepositoryManager;
|
import com.google.gerrit.server.git.GitRepositoryManager;
|
||||||
import com.google.gerrit.testing.SchemaUpgradeTestEnvironment;
|
import com.google.gerrit.testing.InMemoryTestEnvironment;
|
||||||
import com.google.gerrit.testing.TestUpdateUI;
|
import com.google.gerrit.testing.TestUpdateUI;
|
||||||
import com.google.inject.Inject;
|
import com.google.inject.Inject;
|
||||||
import com.google.inject.Provider;
|
import com.google.inject.Provider;
|
||||||
@@ -46,7 +46,7 @@ import org.junit.Rule;
|
|||||||
import org.junit.Test;
|
import org.junit.Test;
|
||||||
|
|
||||||
public class Schema_159_to_160_Test {
|
public class Schema_159_to_160_Test {
|
||||||
@Rule public SchemaUpgradeTestEnvironment testEnv = new SchemaUpgradeTestEnvironment();
|
@Rule public InMemoryTestEnvironment testEnv = new InMemoryTestEnvironment();
|
||||||
|
|
||||||
@Inject private AccountCache accountCache;
|
@Inject private AccountCache accountCache;
|
||||||
@Inject private AllUsersName allUsersName;
|
@Inject private AllUsersName allUsersName;
|
||||||
|
@@ -26,7 +26,7 @@ import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
|
|||||||
import com.google.gerrit.server.git.GitRepositoryManager;
|
import com.google.gerrit.server.git.GitRepositoryManager;
|
||||||
import com.google.gerrit.server.git.MetaDataUpdate;
|
import com.google.gerrit.server.git.MetaDataUpdate;
|
||||||
import com.google.gerrit.server.git.ProjectConfig;
|
import com.google.gerrit.server.git.ProjectConfig;
|
||||||
import com.google.gerrit.testing.SchemaUpgradeTestEnvironment;
|
import com.google.gerrit.testing.InMemoryTestEnvironment;
|
||||||
import com.google.gerrit.testing.TestUpdateUI;
|
import com.google.gerrit.testing.TestUpdateUI;
|
||||||
import com.google.gwtorm.server.OrmException;
|
import com.google.gwtorm.server.OrmException;
|
||||||
import com.google.inject.Inject;
|
import com.google.inject.Inject;
|
||||||
@@ -39,7 +39,7 @@ import org.junit.Rule;
|
|||||||
import org.junit.Test;
|
import org.junit.Test;
|
||||||
|
|
||||||
public class Schema_161_to_162_Test {
|
public class Schema_161_to_162_Test {
|
||||||
@Rule public SchemaUpgradeTestEnvironment testEnv = new SchemaUpgradeTestEnvironment();
|
@Rule public InMemoryTestEnvironment testEnv = new InMemoryTestEnvironment();
|
||||||
|
|
||||||
@Inject private AllProjectsName allProjectsName;
|
@Inject private AllProjectsName allProjectsName;
|
||||||
@Inject private AllUsersName allUsersName;
|
@Inject private AllUsersName allUsersName;
|
||||||
|
Reference in New Issue
Block a user