Skip to content

Commit 1106afd

Browse files
committed
Modify the way we load resources via foreign keys
Previously, we would have separate database calls for mapping from foreign key to repo ID and then from repo ID to object. This PR modifies those calls to load the resource directly (the old system was an artifact of the Datastore key-value storage system). In this PR, we merge the load-resource-by-foreign-key calls into a single database load, as well as adding a separate cache object for (foreign key) -> (resource). Now we cache, and have separate cleaner code paths, for fk -> resource, fk -> repo ID, and repo ID -> resource. Also removes the unused RdeFragmenter class
1 parent 6863f67 commit 1106afd

File tree

63 files changed

+498
-592
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

63 files changed

+498
-592
lines changed

core/src/main/java/google/registry/bsa/BsaValidateAction.java

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
import static google.registry.bsa.persistence.Queries.queryUnblockableDomainByLabels;
2929
import static google.registry.model.tld.Tld.isEnrolledWithBsa;
3030
import static google.registry.model.tld.Tlds.getTldEntitiesOfType;
31-
import static google.registry.persistence.transaction.TransactionManagerFactory.replicaTm;
3231
import static google.registry.request.Action.Method.GET;
3332
import static google.registry.request.Action.Method.POST;
3433
import static google.registry.util.BatchedStreams.toBatches;
@@ -53,7 +52,6 @@
5352
import google.registry.model.domain.Domain;
5453
import google.registry.model.tld.Tld;
5554
import google.registry.model.tld.Tld.TldType;
56-
import google.registry.persistence.VKey;
5755
import google.registry.request.Action;
5856
import google.registry.request.Action.GaeService;
5957
import google.registry.request.Response;
@@ -185,8 +183,8 @@ ImmutableList<String> checkWronglyReportedUnblockableDomains() {
185183
ImmutableList<UnblockableDomain> batch;
186184
do {
187185
batch = Queries.batchReadUnblockableDomains(lastRead, transactionBatchSize);
188-
ImmutableMap<String, VKey<Domain>> activeDomains =
189-
ForeignKeyUtils.load(
186+
ImmutableMap<String, Domain> activeDomains =
187+
ForeignKeyUtils.loadResources(
190188
Domain.class,
191189
batch.stream().map(UnblockableDomain::domainName).collect(toImmutableList()),
192190
clock.nowUtc());
@@ -201,7 +199,7 @@ ImmutableList<String> checkWronglyReportedUnblockableDomains() {
201199
}
202200

203201
Optional<String> verifyDomainStillUnblockableWithReason(
204-
UnblockableDomain domain, ImmutableMap<String, VKey<Domain>> activeDomains) {
202+
UnblockableDomain domain, ImmutableMap<String, Domain> activeDomains) {
205203
DateTime now = clock.nowUtc();
206204
boolean isRegistered = activeDomains.containsKey(domain.domainName());
207205
boolean isReserved = isReservedDomain(domain.domainName(), now);
@@ -230,10 +228,8 @@ Optional<String> verifyDomainStillUnblockableWithReason(
230228
domain.reason()));
231229
}
232230

233-
boolean isStalenessAllowed(VKey<Domain> domainVKey) {
234-
Domain domain = bsaQuery(() -> replicaTm().loadByKey(domainVKey));
235-
var now = clock.nowUtc();
236-
return domain.getCreationTime().plus(maxStaleness).isAfter(now);
231+
boolean isStalenessAllowed(Domain domain) {
232+
return domain.getCreationTime().plus(maxStaleness).isAfter(clock.nowUtc());
237233
}
238234

239235
/** Returns unique labels across all block lists in the download specified by {@code jobName}. */

core/src/main/java/google/registry/bsa/persistence/DomainsRefresher.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ ImmutableSet<UnblockableDomainChange> recheckStaleDomainsBatch(
156156
.collect(toImmutableSet());
157157
ImmutableSet<String> currRegistered =
158158
ImmutableSet.copyOf(
159-
ForeignKeyUtils.load(Domain.class, nameToEntity.keySet(), now).keySet());
159+
ForeignKeyUtils.loadKeys(Domain.class, nameToEntity.keySet(), now).keySet());
160160
SetView<String> noLongerRegistered = Sets.difference(prevRegistered, currRegistered);
161161
SetView<String> newlyRegistered = Sets.difference(currRegistered, prevRegistered);
162162

core/src/main/java/google/registry/bsa/persistence/LabelDiffUpdates.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -145,11 +145,10 @@ static ImmutableList<UnblockableDomain> tallyUnblockableDomainsForNewLabels(
145145

146146
ImmutableSet<String> validDomainNames =
147147
labels.stream()
148-
.map(label -> validDomainNamesForLabel(label, idnChecker))
149-
.flatMap(x -> x)
148+
.flatMap(label -> validDomainNamesForLabel(label, idnChecker))
150149
.collect(toImmutableSet());
151150
ImmutableSet<String> registeredDomainNames =
152-
ImmutableSet.copyOf(ForeignKeyUtils.load(Domain.class, validDomainNames, now).keySet());
151+
ForeignKeyUtils.loadKeys(Domain.class, validDomainNames, now).keySet();
153152
for (String domain : registeredDomainNames) {
154153
nonBlockedDomains.add(new UnblockableDomain(domain, Reason.REGISTERED));
155154
tm().put(BsaUnblockableDomain.of(domain, BsaUnblockableDomain.Reason.REGISTERED));

core/src/main/java/google/registry/dns/PublishDnsUpdatesAction.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import static google.registry.dns.DnsUtils.DNS_PUBLISH_PUSH_QUEUE_NAME;
2626
import static google.registry.dns.DnsUtils.requestDomainDnsRefresh;
2727
import static google.registry.dns.DnsUtils.requestHostDnsRefresh;
28-
import static google.registry.model.EppResourceUtils.loadByForeignKey;
2928
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
3029
import static google.registry.request.Action.Method.POST;
3130
import static google.registry.request.RequestParameters.PARAM_TLD;
@@ -46,6 +45,7 @@
4645
import google.registry.dns.DnsMetrics.PublishStatus;
4746
import google.registry.dns.writer.DnsWriter;
4847
import google.registry.groups.GmailClient;
48+
import google.registry.model.ForeignKeyUtils;
4949
import google.registry.model.domain.Domain;
5050
import google.registry.model.host.Host;
5151
import google.registry.model.registrar.Registrar;
@@ -237,7 +237,8 @@ public Void call() {
237237
.findFirst()
238238
.ifPresent(
239239
dn -> {
240-
Optional<Domain> domain = loadByForeignKey(Domain.class, dn, clock.nowUtc());
240+
Optional<Domain> domain =
241+
ForeignKeyUtils.loadResource(Domain.class, dn, clock.nowUtc());
241242
if (domain.isPresent()) {
242243
notifyWithEmailAboutDnsUpdateFailure(
243244
domain.get().getCurrentSponsorRegistrarId(), dn, false);
@@ -250,7 +251,8 @@ public Void call() {
250251
.findFirst()
251252
.ifPresent(
252253
hn -> {
253-
Optional<Host> host = loadByForeignKey(Host.class, hn, clock.nowUtc());
254+
Optional<Host> host =
255+
ForeignKeyUtils.loadResource(Host.class, hn, clock.nowUtc());
254256
if (host.isPresent()) {
255257
notifyWithEmailAboutDnsUpdateFailure(
256258
host.get().getPersistedCurrentSponsorRegistrarId(), hn, true);

core/src/main/java/google/registry/dns/RefreshDnsAction.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,12 @@
1616

1717
import static google.registry.dns.DnsUtils.requestDomainDnsRefresh;
1818
import static google.registry.dns.DnsUtils.requestHostDnsRefresh;
19-
import static google.registry.model.EppResourceUtils.loadByForeignKey;
2019
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
2120

2221
import google.registry.dns.DnsUtils.TargetType;
2322
import google.registry.model.EppResource;
2423
import google.registry.model.EppResource.ForeignKeyedEppResource;
24+
import google.registry.model.ForeignKeyUtils;
2525
import google.registry.model.annotations.ExternalMessagingName;
2626
import google.registry.model.domain.Domain;
2727
import google.registry.model.host.Host;
@@ -79,7 +79,7 @@ public void run() {
7979

8080
private <T extends EppResource & ForeignKeyedEppResource>
8181
T loadAndVerifyExistence(Class<T> clazz, String foreignKey) {
82-
return loadByForeignKey(clazz, foreignKey, clock.nowUtc())
82+
return ForeignKeyUtils.loadResource(clazz, foreignKey, clock.nowUtc())
8383
.orElseThrow(
8484
() ->
8585
new NotFoundException(

core/src/main/java/google/registry/dns/writer/clouddns/CloudDnsWriter.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
import static com.google.common.base.Preconditions.checkArgument;
1818
import static com.google.common.collect.ImmutableSet.toImmutableSet;
1919
import static google.registry.dns.DnsUtils.getDnsAPlusAAAATtlForHost;
20-
import static google.registry.model.EppResourceUtils.loadByForeignKey;
2120
import static google.registry.util.DomainNameUtils.getSecondLevelDomain;
2221

2322
import com.google.api.client.googleapis.json.GoogleJsonError;
@@ -37,6 +36,7 @@
3736
import google.registry.dns.writer.BaseDnsWriter;
3837
import google.registry.dns.writer.DnsWriter;
3938
import google.registry.dns.writer.DnsWriterZone;
39+
import google.registry.model.ForeignKeyUtils;
4040
import google.registry.model.domain.Domain;
4141
import google.registry.model.domain.secdns.DomainDsData;
4242
import google.registry.model.host.Host;
@@ -123,7 +123,8 @@ public void publishDomain(String domainName) {
123123
String absoluteDomainName = getAbsoluteHostName(domainName);
124124

125125
// Load the target domain. Note that it can be absent if this domain was just deleted.
126-
Optional<Domain> domain = loadByForeignKey(Domain.class, domainName, clock.nowUtc());
126+
Optional<Domain> domain =
127+
ForeignKeyUtils.loadResource(Domain.class, domainName, clock.nowUtc());
127128

128129
// Return early if no DNS records should be published.
129130
// desiredRecordsBuilder is populated with an empty set to indicate that all existing records
@@ -189,7 +190,7 @@ private void publishSubordinateHost(String hostName) {
189190
// Load the target host. Note that it can be absent if this host was just deleted.
190191
// desiredRecords is populated with an empty set to indicate that all existing records
191192
// should be deleted.
192-
Optional<Host> host = loadByForeignKey(Host.class, hostName, clock.nowUtc());
193+
Optional<Host> host = ForeignKeyUtils.loadResource(Host.class, hostName, clock.nowUtc());
193194

194195
// Return early if the host is deleted.
195196
if (host.isEmpty()) {

core/src/main/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriter.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import static com.google.common.collect.Sets.intersection;
2020
import static com.google.common.collect.Sets.union;
2121
import static google.registry.dns.DnsUtils.getDnsAPlusAAAATtlForHost;
22-
import static google.registry.model.EppResourceUtils.loadByForeignKey;
2322

2423
import com.google.common.base.Joiner;
2524
import com.google.common.collect.ImmutableList;
@@ -28,6 +27,7 @@
2827
import google.registry.config.RegistryConfig.Config;
2928
import google.registry.dns.writer.BaseDnsWriter;
3029
import google.registry.dns.writer.DnsWriterZone;
30+
import google.registry.model.ForeignKeyUtils;
3131
import google.registry.model.domain.Domain;
3232
import google.registry.model.domain.secdns.DomainDsData;
3333
import google.registry.model.host.Host;
@@ -129,7 +129,8 @@ public DnsUpdateWriter(
129129
* this domain refresh request
130130
*/
131131
private void publishDomain(String domainName, String requestingHostName) {
132-
Optional<Domain> domainOptional = loadByForeignKey(Domain.class, domainName, clock.nowUtc());
132+
Optional<Domain> domainOptional =
133+
ForeignKeyUtils.loadResource(Domain.class, domainName, clock.nowUtc());
133134
update.delete(toAbsoluteName(domainName), Type.ANY);
134135
// If the domain is now deleted, then don't update DNS for it.
135136
if (domainOptional.isPresent()) {
@@ -218,7 +219,7 @@ private void deleteSubordinateHostAddressSet(
218219
private void addInBailiwickNameServerSet(Domain domain, Update update) {
219220
for (String hostName :
220221
intersection(domain.loadNameserverHostNames(), domain.getSubordinateHosts())) {
221-
Optional<Host> host = loadByForeignKey(Host.class, hostName, clock.nowUtc());
222+
Optional<Host> host = ForeignKeyUtils.loadResource(Host.class, hostName, clock.nowUtc());
222223
checkState(host.isPresent(), "Host %s cannot be loaded", hostName);
223224
update.add(makeAddressSet(host.get()));
224225
update.add(makeV6AddressSet(host.get()));

core/src/main/java/google/registry/flows/CheckApiAction.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838
import static google.registry.util.DomainNameUtils.canonicalizeHostname;
3939
import static org.json.simple.JSONValue.toJSONString;
4040

41-
import com.google.common.collect.ImmutableList;
4241
import com.google.common.collect.ImmutableMap;
4342
import com.google.common.collect.ImmutableSet;
4443
import com.google.common.flogger.FluentLogger;
@@ -185,8 +184,7 @@ private Map<String, Object> checkDomainName(InternetDomainName domainName) {
185184
}
186185

187186
private boolean checkExists(String domainString, DateTime now) {
188-
return !ForeignKeyUtils.loadByCache(Domain.class, ImmutableList.of(domainString), now)
189-
.isEmpty();
187+
return ForeignKeyUtils.loadKeyByCache(Domain.class, domainString, now).isPresent();
190188
}
191189

192190
private Optional<String> checkReserved(InternetDomainName domainName) {

core/src/main/java/google/registry/flows/ResourceFlowUtils.java

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
import static com.google.common.collect.Sets.intersection;
1818
import static google.registry.model.EppResourceUtils.isLinked;
19-
import static google.registry.model.EppResourceUtils.loadByForeignKey;
2019
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
2120

2221
import com.google.common.collect.ImmutableSet;
@@ -72,11 +71,11 @@ public static void verifyResourceOwnership(String myRegistrarId, EppResource res
7271
*/
7372
public static <R extends EppResource> void checkLinkedDomains(
7473
final String targetId, final DateTime now, final Class<R> resourceClass) throws EppException {
75-
VKey<R> key = ForeignKeyUtils.load(resourceClass, targetId, now);
76-
if (key == null) {
74+
Optional<VKey<R>> key = ForeignKeyUtils.loadKey(resourceClass, targetId, now);
75+
if (key.isEmpty()) {
7776
throw new ResourceDoesNotExistException(resourceClass, targetId);
7877
}
79-
if (isLinked(key, now)) {
78+
if (isLinked(key.get(), now)) {
8079
throw new ResourceToDeleteIsReferencedException();
8180
}
8281
}
@@ -97,7 +96,7 @@ public static <R extends EppResource & ResourceWithTransferData> void verifyTran
9796

9897
public static <R extends EppResource & ForeignKeyedEppResource> R loadAndVerifyExistence(
9998
Class<R> clazz, String targetId, DateTime now) throws ResourceDoesNotExistException {
100-
return verifyExistence(clazz, targetId, loadByForeignKey(clazz, targetId, now));
99+
return verifyExistence(clazz, targetId, ForeignKeyUtils.loadResource(clazz, targetId, now));
101100
}
102101

103102
public static <R extends EppResource> R verifyExistence(
@@ -107,11 +106,10 @@ public static <R extends EppResource> R verifyExistence(
107106

108107
public static <R extends EppResource> void verifyResourceDoesNotExist(
109108
Class<R> clazz, String targetId, DateTime now, String registrarId) throws EppException {
110-
VKey<R> key = ForeignKeyUtils.load(clazz, targetId, now);
111-
if (key != null) {
112-
R resource = tm().loadByKey(key);
109+
Optional<R> resource = ForeignKeyUtils.loadResource(clazz, targetId, now);
110+
if (resource.isPresent()) {
113111
// These are similar exceptions, but we can track them internally as log-based metrics.
114-
if (Objects.equals(registrarId, resource.getPersistedCurrentSponsorRegistrarId())) {
112+
if (Objects.equals(registrarId, resource.get().getPersistedCurrentSponsorRegistrarId())) {
115113
throw new ResourceAlreadyExistsForThisClientException(targetId);
116114
} else {
117115
throw new ResourceCreateContentionException(targetId);

core/src/main/java/google/registry/flows/contact/ContactCheckFlow.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
import static google.registry.flows.FlowUtils.validateRegistrarIsLoggedIn;
1818
import static google.registry.flows.ResourceFlowUtils.verifyTargetIdCount;
19-
import static google.registry.model.EppResourceUtils.checkResourcesExist;
2019

2120
import com.google.common.collect.ImmutableList;
2221
import com.google.common.collect.ImmutableSet;
@@ -26,6 +25,7 @@
2625
import google.registry.flows.FlowModule.RegistrarId;
2726
import google.registry.flows.TransactionalFlow;
2827
import google.registry.flows.annotations.ReportingSpec;
28+
import google.registry.model.ForeignKeyUtils;
2929
import google.registry.model.contact.Contact;
3030
import google.registry.model.contact.ContactCommand.Check;
3131
import google.registry.model.eppinput.ResourceCommand;
@@ -62,7 +62,7 @@ public EppResponse run() throws EppException {
6262
ImmutableList<String> targetIds = ((Check) resourceCommand).getTargetIds();
6363
verifyTargetIdCount(targetIds, maxChecks);
6464
ImmutableSet<String> existingIds =
65-
checkResourcesExist(Contact.class, targetIds, clock.nowUtc());
65+
ForeignKeyUtils.loadKeys(Contact.class, targetIds, clock.nowUtc()).keySet();
6666
ImmutableList.Builder<ContactCheck> checks = new ImmutableList.Builder<>();
6767
for (String id : targetIds) {
6868
boolean unused = !existingIds.contains(id);

0 commit comments

Comments
 (0)