zydronium/google-nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus.git synced 2025-05-15 17:07:15 +02:00
Code Issues Projects Releases Packages Wiki Activity

Batch contact/host loads when checking pending delete

Browse source 
I'm actually surprised that we had this in our code, as it seems like a huge
oversight, but we were individually loading each and every referenced contact
and host during domain/application create/update/allocate flows. Loading them
all as a single batch should reduce round trips to Datastore by a good deal,
thus improving performance.

We aren't giving up any transactional consistency in doing so and the only
potential downside I can think of is that we're always loading all contacts/
hosts instead of only some of them, in the rare case that one of the earlier
contacts/hosts is actually in pending delete (which allowed short-circuiting).
However, the gains from only making one round-trip should swamp the potential
losses in occasionally loading more data than is strictly necessary.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=186656118
This commit is contained in:
mcilwain 2018-02-22 12:25:09 -08:00 • committed by jianglai
parent ff221fba96
commit cb80a0df1e
1 changed files with 17 additions and 13 deletions
Download patch file Download diff file Expand all files Collapse all files

30
java/google/registry/flows/domain/DomainFlowUtils.java
View file

@ -17,6 +17,7 @@ package google.registry.flows.domain;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.base.Predicates.equalTo;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.common.collect.Iterables.any;
import static com.google.common.collect.Sets.difference;
@ -270,22 +271,25 @@ public class DomainFlowUtils {
Key<ContactResource> registrant,
Set<Key<HostResource>> nameservers)
throws EppException {
for (DesignatedContact contact : nullToEmpty(contacts)) {
verifyNotInPendingDelete(contact.getContactKey());
}
if (registrant != null) {
verifyNotInPendingDelete(registrant);
}
for (Key<HostResource> host : nullToEmpty(nameservers)) {
verifyNotInPendingDelete(host);
}
ImmutableList.Builder<Key<? extends EppResource>> keysToLoad = new ImmutableList.Builder<>();
nullToEmpty(contacts).stream().map(DesignatedContact::getContactKey).forEach(keysToLoad::add);
Optional.ofNullable(registrant).ifPresent(keysToLoad::add);
keysToLoad.addAll(nullToEmpty(nameservers));
// This cast is safe because, in Objectify, Key<? extends EppResource> can also be
// treated as a Key<EppResource>.
@SuppressWarnings("unchecked")
ImmutableList<Key<EppResource>> typedKeys =
keysToLoad.build().stream().map(key -> (Key<EppResource>) key).collect(toImmutableList());
verifyNotInPendingDelete(ofy().load().keys(typedKeys).values());
}
private static void verifyNotInPendingDelete(Key<? extends EppResource> resourceKey)
private static void verifyNotInPendingDelete(Iterable<EppResource> resources)
throws EppException {
EppResource resource = ofy().load().key(resourceKey).now();
if (resource.getStatusValues().contains(StatusValue.PENDING_DELETE)) {
throw new LinkedResourceInPendingDeleteProhibitsOperationException(resource.getForeignKey());
for (EppResource resource : resources) {
if (resource.getStatusValues().contains(StatusValue.PENDING_DELETE)) {
throw new LinkedResourceInPendingDeleteProhibitsOperationException(
resource.getForeignKey());
}
}
}