opendev/gerrit
Code Issues Proposed changes

Merge "Make RetryHelper reusable for updates of non-change entities"

Browse Source
This commit is contained in:
Edwin Kempin
2017-12-19 14:11:13 +00:00
committed by Gerrit Code Review
parent 7c8599ed29 350e3e263f
commit 8a9873b703
5 changed files with 185 additions and 115 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
java/com/google/gerrit/acceptance/AbstractDaemonTest.java
View File

@@ -29,6 +29,7 @@ import static java.util.stream.Collectors.toList;
import static org.eclipse.jgit.lib.Constants.HEAD;
import static org.eclipse.jgit.lib.Constants.R_TAGS;
import com.github.rholder.retry.BlockStrategy;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@@ -254,6 +255,7 @@ public abstract class AbstractDaemonTest {
protected String resourcePrefix;
protected Description description;
protected boolean testRequiresSsh;
protected BlockStrategy noSleepBlockStrategy = t -> {}; // Don't sleep in tests.
@Inject private ChangeIndexCollection changeIndexes;
@Inject private EventRecorder.Factory eventRecorderFactory;

3
java/com/google/gerrit/acceptance/BUILD
View File

@@ -108,8 +108,9 @@ java_library2(
visibility = ["//visibility:public"],
deps = PROVIDED + [
# We want these deps to be exported_deps
"//lib/greenmail:greenmail",
"//lib:guava-retrying",
"//lib:gwtorm",
"//lib/greenmail:greenmail",
"//lib/guice:guice",
"//lib/guice:guice-assistedinject",
"//lib/guice:guice-servlet",

96
java/com/google/gerrit/server/account/externalids/ExternalIdsUpdate.java
View File

@@ -24,14 +24,8 @@ import static java.util.stream.Collectors.toSet;
import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
import static org.eclipse.jgit.lib.Constants.OBJ_TREE;
import com.github.rholder.retry.RetryException;
import com.github.rholder.retry.Retryer;
import com.github.rholder.retry.RetryerBuilder;
import com.github.rholder.retry.StopStrategies;
import com.github.rholder.retry.WaitStrategies;
import com.google.auto.value.AutoValue;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Streams;
@@ -50,6 +44,7 @@ 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.git.LockFailureException;
import com.google.gerrit.server.update.RetryHelper;
import com.google.gwtorm.server.OrmDuplicateKeyException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -60,8 +55,6 @@ import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.stream.Stream;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.CommitBuilder;
@@ -114,6 +107,7 @@ public class ExternalIdsUpdate {
private final ExternalIdCache externalIdCache;
private final Provider<PersonIdent> serverIdent;
private final GitReferenceUpdated gitRefUpdated;
private final RetryHelper retryHelper;
@Inject
public Server(
@@ -124,7 +118,8 @@ public class ExternalIdsUpdate {
ExternalIds externalIds,
ExternalIdCache externalIdCache,
@GerritPersonIdent Provider<PersonIdent> serverIdent,
GitReferenceUpdated gitRefUpdated) {
GitReferenceUpdated gitRefUpdated,
RetryHelper retryHelper) {
this.repoManager = repoManager;
this.accountCache = accountCache;
this.allUsersName = allUsersName;
@@ -133,6 +128,7 @@ public class ExternalIdsUpdate {
this.externalIdCache = externalIdCache;
this.serverIdent = serverIdent;
this.gitRefUpdated = gitRefUpdated;
this.retryHelper = retryHelper;
}
public ExternalIdsUpdate create() {
@@ -147,7 +143,8 @@ public class ExternalIdsUpdate {
i,
i,
null,
gitRefUpdated);
gitRefUpdated,
retryHelper);
}
}
@@ -169,6 +166,7 @@ public class ExternalIdsUpdate {
private final ExternalIdCache externalIdCache;
private final Provider<PersonIdent> serverIdent;
private final GitReferenceUpdated gitRefUpdated;
private final RetryHelper retryHelper;
@Inject
public ServerNoReindex(
@@ -178,7 +176,8 @@ public class ExternalIdsUpdate {
ExternalIds externalIds,
ExternalIdCache externalIdCache,
@GerritPersonIdent Provider<PersonIdent> serverIdent,
GitReferenceUpdated gitRefUpdated) {
GitReferenceUpdated gitRefUpdated,
RetryHelper retryHelper) {
this.repoManager = repoManager;
this.allUsersName = allUsersName;
this.metricMaker = metricMaker;
@@ -186,6 +185,7 @@ public class ExternalIdsUpdate {
this.externalIdCache = externalIdCache;
this.serverIdent = serverIdent;
this.gitRefUpdated = gitRefUpdated;
this.retryHelper = retryHelper;
}
public ExternalIdsUpdate create() {
@@ -200,7 +200,8 @@ public class ExternalIdsUpdate {
i,
i,
null,
gitRefUpdated);
gitRefUpdated,
retryHelper);
}
}
@@ -221,6 +222,7 @@ public class ExternalIdsUpdate {
private final Provider<PersonIdent> serverIdent;
private final Provider<IdentifiedUser> identifiedUser;
private final GitReferenceUpdated gitRefUpdated;
private final RetryHelper retryHelper;
@Inject
public User(
@@ -232,7 +234,8 @@ public class ExternalIdsUpdate {
ExternalIdCache externalIdCache,
@GerritPersonIdent Provider<PersonIdent> serverIdent,
Provider<IdentifiedUser> identifiedUser,
GitReferenceUpdated gitRefUpdated) {
GitReferenceUpdated gitRefUpdated,
RetryHelper retryHelper) {
this.repoManager = repoManager;
this.accountCache = accountCache;
this.allUsersName = allUsersName;
@@ -242,6 +245,7 @@ public class ExternalIdsUpdate {
this.serverIdent = serverIdent;
this.identifiedUser = identifiedUser;
this.gitRefUpdated = gitRefUpdated;
this.retryHelper = retryHelper;
}
public ExternalIdsUpdate create() {
@@ -257,7 +261,8 @@ public class ExternalIdsUpdate {
createPersonIdent(i, user),
i,
user,
gitRefUpdated);
gitRefUpdated,
retryHelper);
}
private PersonIdent createPersonIdent(PersonIdent ident, IdentifiedUser user) {
@@ -265,19 +270,6 @@ public class ExternalIdsUpdate {
}
}
@VisibleForTesting
public static RetryerBuilder<RefsMetaExternalIdsUpdate> retryerBuilder() {
return RetryerBuilder.<RefsMetaExternalIdsUpdate>newBuilder()
.retryIfException(e -> e instanceof LockFailureException)
.withWaitStrategy(
WaitStrategies.join(
WaitStrategies.exponentialWait(2, TimeUnit.SECONDS),
WaitStrategies.randomWait(50, TimeUnit.MILLISECONDS)))
.withStopStrategy(StopStrategies.stopAfterDelay(10, TimeUnit.SECONDS));
}
private static final Retryer<RefsMetaExternalIdsUpdate> RETRYER = retryerBuilder().build();
private final GitRepositoryManager repoManager;
@Nullable private final AccountCache accountCache;
private final AllUsersName allUsersName;
@@ -287,8 +279,8 @@ public class ExternalIdsUpdate {
private final PersonIdent authorIdent;
@Nullable private final IdentifiedUser currentUser;
private final GitReferenceUpdated gitRefUpdated;
private final RetryHelper retryHelper;
private final Runnable afterReadRevision;
private final Retryer<RefsMetaExternalIdsUpdate> retryer;
private final Counter0 updateCount;
private ExternalIdsUpdate(
@@ -301,7 +293,8 @@ public class ExternalIdsUpdate {
PersonIdent committerIdent,
PersonIdent authorIdent,
@Nullable IdentifiedUser currentUser,
GitReferenceUpdated gitRefUpdated) {
GitReferenceUpdated gitRefUpdated,
RetryHelper retryHelper) {
this(
repoManager,
accountCache,
@@ -313,8 +306,8 @@ public class ExternalIdsUpdate {
authorIdent,
currentUser,
gitRefUpdated,
Runnables.doNothing(),
RETRYER);
retryHelper,
Runnables.doNothing());
}
@VisibleForTesting
@@ -329,8 +322,8 @@ public class ExternalIdsUpdate {
PersonIdent authorIdent,
@Nullable IdentifiedUser currentUser,
GitReferenceUpdated gitRefUpdated,
Runnable afterReadRevision,
Retryer<RefsMetaExternalIdsUpdate> retryer) {
RetryHelper retryHelper,
Runnable afterReadRevision) {
this.repoManager = checkNotNull(repoManager, "repoManager");
this.accountCache = accountCache;
this.allUsersName = checkNotNull(allUsersName, "allUsersName");
@@ -340,8 +333,8 @@ public class ExternalIdsUpdate {
this.authorIdent = checkNotNull(authorIdent, "authorIdent");
this.currentUser = currentUser;
this.gitRefUpdated = checkNotNull(gitRefUpdated, "gitRefUpdated");
this.retryHelper = checkNotNull(retryHelper, "retryHelper");
this.afterReadRevision = checkNotNull(afterReadRevision, "afterReadRevision");
this.retryer = checkNotNull(retryer, "retryer");
this.updateCount =
metricMaker.newCounter(
"notedb/external_id_update_count",
@@ -738,32 +731,23 @@ public class ExternalIdsUpdate {
private RefsMetaExternalIdsUpdate updateNoteMap(ExternalIdUpdater updater)
throws IOException, ConfigInvalidException, OrmException {
try {
return retryer.call(
() -> {
try (Repository repo = repoManager.openRepository(allUsersName);
ObjectInserter ins = repo.newObjectInserter()) {
ObjectId rev = readRevision(repo);
return retryHelper.execute(
() -> {
try (Repository repo = repoManager.openRepository(allUsersName);
ObjectInserter ins = repo.newObjectInserter()) {
ObjectId rev = readRevision(repo);
afterReadRevision.run();
afterReadRevision.run();
try (RevWalk rw = new RevWalk(repo)) {
NoteMap noteMap = readNoteMap(rw, rev);
UpdatedExternalIds updatedExtIds =
updater.update(OpenRepo.create(repo, rw, ins, noteMap));
try (RevWalk rw = new RevWalk(repo)) {
NoteMap noteMap = readNoteMap(rw, rev);
UpdatedExternalIds updatedExtIds =
updater.update(OpenRepo.create(repo, rw, ins, noteMap));
return commit(repo, rw, ins, rev, noteMap, updatedExtIds);
}
return commit(repo, rw, ins, rev, noteMap, updatedExtIds);
}
});
} catch (ExecutionException | RetryException e) {
if (e.getCause() != null) {
Throwables.throwIfInstanceOf(e.getCause(), IOException.class);
Throwables.throwIfInstanceOf(e.getCause(), ConfigInvalidException.class);
Throwables.throwIfInstanceOf(e.getCause(), OrmException.class);
}
throw new OrmException(e);
}
}
});
}
private RefsMetaExternalIdsUpdate commit(

157
java/com/google/gerrit/server/update/RetryHelper.java
View File

@@ -21,11 +21,14 @@ import static java.util.concurrent.TimeUnit.SECONDS;
import com.github.rholder.retry.Attempt;
import com.github.rholder.retry.RetryException;
import com.github.rholder.retry.RetryListener;
import com.github.rholder.retry.Retryer;
import com.github.rholder.retry.RetryerBuilder;
import com.github.rholder.retry.StopStrategies;
import com.github.rholder.retry.WaitStrategies;
import com.github.rholder.retry.WaitStrategy;
import com.google.auto.value.AutoValue;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Predicate;
import com.google.common.base.Throwables;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -36,16 +39,26 @@ import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.LockFailureException;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
import java.time.Duration;
import java.util.concurrent.ExecutionException;
import java.util.function.Consumer;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Config;
@Singleton
public class RetryHelper {
@FunctionalInterface
public interface ChangeAction<T> {
T call(BatchUpdate.Factory batchUpdateFactory) throws Exception;
}
@FunctionalInterface
public interface Action<T> {
T call(BatchUpdate.Factory updateFactory) throws Exception;
T call() throws Exception;
}
/**
@@ -80,8 +93,9 @@ public class RetryHelper {
}
}
@VisibleForTesting
@Singleton
private static class Metrics {
public static class Metrics {
final Histogram0 attemptCounts;
final Counter0 timeoutCount;
@@ -108,7 +122,7 @@ public class RetryHelper {
return new AutoValue_RetryHelper_Options.Builder();
}
public static Options defaults() {
private static Options defaults() {
return options().build();
}
@@ -117,6 +131,7 @@ public class RetryHelper {
private final BatchUpdate.Factory updateFactory;
private final Duration defaultTimeout;
private final WaitStrategy waitStrategy;
@Nullable private final Consumer<RetryerBuilder<?>> overwriteDefaultRetryerStrategySetup;
@Inject
RetryHelper(
@@ -125,6 +140,17 @@ public class RetryHelper {
NotesMigration migration,
ReviewDbBatchUpdate.AssistedFactory reviewDbBatchUpdateFactory,
NoteDbBatchUpdate.AssistedFactory noteDbBatchUpdateFactory) {
this(cfg, metrics, migration, reviewDbBatchUpdateFactory, noteDbBatchUpdateFactory, null);
}
@VisibleForTesting
public RetryHelper(
@GerritServerConfig Config cfg,
Metrics metrics,
NotesMigration migration,
ReviewDbBatchUpdate.AssistedFactory reviewDbBatchUpdateFactory,
NoteDbBatchUpdate.AssistedFactory noteDbBatchUpdateFactory,
@Nullable Consumer<RetryerBuilder<?>> overwriteDefaultRetryerStrategySetup) {
this.metrics = metrics;
this.migration = migration;
this.updateFactory =
@@ -138,73 +164,132 @@ public class RetryHelper {
cfg.getTimeUnit("noteDb", null, "retryMaxWait", SECONDS.toMillis(5), MILLISECONDS),
MILLISECONDS),
WaitStrategies.randomWait(50, MILLISECONDS));
this.overwriteDefaultRetryerStrategySetup = overwriteDefaultRetryerStrategySetup;
}
public Duration getDefaultTimeout() {
return defaultTimeout;
}
public <T> T execute(Action<T> action) throws RestApiException, UpdateException {
return execute(action, defaults());
public <T> T execute(Action<T> action) throws IOException, ConfigInvalidException, OrmException {
try {
return execute(action, defaults(), t -> t instanceof LockFailureException);
} catch (Throwable t) {
Throwables.throwIfUnchecked(t);
Throwables.throwIfInstanceOf(t, IOException.class);
Throwables.throwIfInstanceOf(t, ConfigInvalidException.class);
Throwables.throwIfInstanceOf(t, OrmException.class);
throw new OrmException(t);
}
}
public <T> T execute(Action<T> action, Options opts) throws RestApiException, UpdateException {
MetricListener listener = null;
public <T> T execute(ChangeAction<T> changeAction) throws RestApiException, UpdateException {
return execute(changeAction, defaults());
}
public <T> T execute(ChangeAction<T> changeAction, Options opts)
throws RestApiException, UpdateException {
try {
RetryerBuilder<T> builder = RetryerBuilder.newBuilder();
if (migration.disableChangeReviewDb()) {
listener = new MetricListener(opts.listener());
builder
.withRetryListener(listener)
.withStopStrategy(
StopStrategies.stopAfterDelay(
firstNonNull(opts.timeout(), defaultTimeout).toMillis(), MILLISECONDS))
.withWaitStrategy(waitStrategy)
.retryIfException(RetryHelper::isLockFailure);
} else {
if (!migration.disableChangeReviewDb()) {
// 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
// don't do it automatically. Let the end user decide whether they want to retry.
return execute(
() -> changeAction.call(updateFactory), RetryerBuilder.<T>newBuilder().build());
}
return builder.build().call(() -> action.call(updateFactory));
return execute(
() -> changeAction.call(updateFactory),
opts,
t -> {
if (t instanceof UpdateException) {
t = t.getCause();
}
return t instanceof LockFailureException;
});
} catch (Throwable t) {
Throwables.throwIfUnchecked(t);
Throwables.throwIfInstanceOf(t, UpdateException.class);
Throwables.throwIfInstanceOf(t, RestApiException.class);
throw new UpdateException(t);
}
}
/**
* Executes an action with a given retryer.
*
* @param action the action which should be executed and retried on failure
* @param opts options for retrying the action on failure
* @param exceptionPredicate predicate to control on which exception the action should be retried
* @return the result of executing the action
* @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
*/
private <T> T execute(Action<T> action, Options opts, Predicate<Throwable> exceptionPredicate)
throws Throwable {
MetricListener listener = new MetricListener();
try {
RetryerBuilder<T> retryerBuilder = createRetryerBuilder(opts, exceptionPredicate);
retryerBuilder.withRetryListener(listener);
return execute(action, retryerBuilder.build());
} finally {
metrics.attemptCounts.record(listener.getAttemptCount());
}
}
/**
* Executes an action with a given retryer.
*
* @param action the action which should be executed and retried on failure
* @param retryer the retryer
* @return the result of executing the action
* @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
*/
private <T> T execute(Action<T> action, Retryer<T> retryer) throws Throwable {
try {
return retryer.call(() -> action.call());
} catch (ExecutionException | RetryException e) {
if (e instanceof RetryException) {
metrics.timeoutCount.increment();
}
if (e.getCause() != null) {
Throwables.throwIfInstanceOf(e.getCause(), UpdateException.class);
Throwables.throwIfInstanceOf(e.getCause(), RestApiException.class);
}
throw new UpdateException(e);
} finally {
if (listener != null) {
metrics.attemptCounts.record(listener.getAttemptCount());
throw e.getCause();
}
throw e;
}
}
private static boolean isLockFailure(Throwable t) {
if (t instanceof UpdateException) {
t = t.getCause();
private <O> RetryerBuilder<O> createRetryerBuilder(
Options opts, Predicate<Throwable> exceptionPredicate) {
RetryerBuilder<O> retryerBuilder =
RetryerBuilder.<O>newBuilder().retryIfException(exceptionPredicate);
if (opts.listener() != null) {
retryerBuilder.withRetryListener(opts.listener());
}
return t instanceof LockFailureException;
if (overwriteDefaultRetryerStrategySetup != null) {
overwriteDefaultRetryerStrategySetup.accept(retryerBuilder);
return retryerBuilder;
}
return retryerBuilder
.withStopStrategy(
StopStrategies.stopAfterDelay(
firstNonNull(opts.timeout(), defaultTimeout).toMillis(), MILLISECONDS))
.withWaitStrategy(waitStrategy);
}
private static class MetricListener implements RetryListener {
private final RetryListener delegate;
private long attemptCount;
MetricListener(@Nullable RetryListener delegate) {
this.delegate = delegate;
MetricListener() {
attemptCount = 1;
}
@Override
public <V> void onRetry(Attempt<V> attempt) {
attemptCount = attempt.getAttemptNumber();
if (delegate != null) {
delegate.onRetry(attempt);
}
}
long getAttemptCount() {

42
javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java
View File

@@ -25,9 +25,6 @@ import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.toList;
import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
import com.github.rholder.retry.BlockStrategy;
import com.github.rholder.retry.Retryer;
import com.github.rholder.retry.RetryerBuilder;
import com.github.rholder.retry.StopStrategies;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
@@ -51,9 +48,9 @@ import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.account.externalids.ExternalIdReader;
import com.google.gerrit.server.account.externalids.ExternalIds;
import com.google.gerrit.server.account.externalids.ExternalIdsUpdate;
import com.google.gerrit.server.account.externalids.ExternalIdsUpdate.RefsMetaExternalIdsUpdate;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.LockFailureException;
import com.google.gerrit.server.update.RetryHelper;
import com.google.gson.reflect.TypeToken;
import com.google.gwtorm.server.OrmDuplicateKeyException;
import com.google.gwtorm.server.OrmException;
@@ -89,6 +86,7 @@ public class ExternalIdIT extends AbstractDaemonTest {
@Inject private ExternalIds externalIds;
@Inject private ExternalIdReader externalIdReader;
@Inject private MetricMaker metricMaker;
@Inject private RetryHelper.Metrics retryMetrics;
@Test
public void getExternalIds() throws Exception {
@@ -713,17 +711,6 @@ public class ExternalIdIT extends AbstractDaemonTest {
@Test
public void retryOnLockFailure() throws Exception {
Retryer<RefsMetaExternalIdsUpdate> retryer =
ExternalIdsUpdate.retryerBuilder()
.withBlockStrategy(
new BlockStrategy() {
@Override
public void block(long sleepTime) {
// Don't sleep in tests.
}
})
.build();
ExternalId.Key fooId = ExternalId.Key.create("foo", "foo");
ExternalId.Key barId = ExternalId.Key.create("bar", "bar");
@@ -740,6 +727,13 @@ public class ExternalIdIT extends AbstractDaemonTest {
serverIdent.get(),
null,
GitReferenceUpdated.DISABLED,
new RetryHelper(
cfg,
retryMetrics,
null,
null,
null,
r -> r.withBlockStrategy(noSleepBlockStrategy)),
() -> {
if (!doneBgUpdate.getAndSet(true)) {
try {
@@ -748,8 +742,7 @@ public class ExternalIdIT extends AbstractDaemonTest {
// Ignore, the successful insertion of the external ID is asserted later
}
}
},
retryer);
});
assertThat(doneBgUpdate.get()).isFalse();
update.insert(ExternalId.create(fooId, admin.id));
assertThat(doneBgUpdate.get()).isTrue();
@@ -778,6 +771,15 @@ public class ExternalIdIT extends AbstractDaemonTest {
serverIdent.get(),
null,
GitReferenceUpdated.DISABLED,
new RetryHelper(
cfg,
retryMetrics,
null,
null,
null,
r ->
r.withStopStrategy(StopStrategies.stopAfterAttempt(extIdsKeys.length))
.withBlockStrategy(noSleepBlockStrategy)),
() -> {
try {
extIdsUpdate
@@ -786,11 +788,7 @@ public class ExternalIdIT extends AbstractDaemonTest {
} catch (IOException | ConfigInvalidException | OrmException e) {
// Ignore, the successful insertion of the external ID is asserted later
}
},
RetryerBuilder.<RefsMetaExternalIdsUpdate>newBuilder()
.retryIfException(e -> e instanceof LockFailureException)
.withStopStrategy(StopStrategies.stopAfterAttempt(extIdsKeys.length))
.build());
});
assertThat(bgCounter.get()).isEqualTo(0);
try {
update.insert(ExternalId.create(ExternalId.Key.create("abc", "abc"), admin.id));