From dfe0ba32cb6fef3735b06f1f2a3e7ada2e4e8970 Mon Sep 17 00:00:00 2001
From: mcilwain <mcilwain@google.com>
Date: Mon, 10 Oct 2016 12:35:27 -0700
Subject: [PATCH] Reduce Datastore load of delete prober data mapreduce

This eliminates the transactional load of ForeignKeyIndexes and
EppResourceIndexes, the latter of which was problematic because it is parented
on the EppResourceIndexBucket entity group, and can cause concurrent
modification exceptions on live code paths. By removing the transactional load
and only touching that entity group on the delete, the number of potential
concurrent modification exceptions is significantly reduced.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=135706974
---
 .../batch/DeleteProberDataAction.java         | 37 +++++++++----------
 1 file changed, 17 insertions(+), 20 deletions(-)

diff --git a/java/google/registry/batch/DeleteProberDataAction.java b/java/google/registry/batch/DeleteProberDataAction.java
index cc0c8f059..d150e4f86 100644
--- a/java/google/registry/batch/DeleteProberDataAction.java
+++ b/java/google/registry/batch/DeleteProberDataAction.java
@@ -14,7 +14,6 @@
 
 package google.registry.batch;
 
-import static com.google.common.base.Verify.verifyNotNull;
 import static google.registry.mapreduce.MapreduceRunner.PARAM_DRY_RUN;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.model.registry.Registries.getTldsOfType;
@@ -136,33 +135,31 @@ public class DeleteProberDataAction implements Runnable {
         getContext().incrementCounter("skipped, NIC domain");
         return;
       }
-      int dependentsDeleted = ofy().transact(new Work<Integer>() {
+
+      final Key<EppResourceIndex> eppIndex = Key.create(EppResourceIndex.create(domainKey));
+      final Key<ForeignKeyIndex<?>> fki = ForeignKeyDomainIndex.createKey(domain);
+
+      int entitiesDeleted = ofy().transact(new Work<Integer>() {
         @Override
         public Integer run() {
-          EppResourceIndex eppIndex = ofy().load().entity(EppResourceIndex.create(domainKey)).now();
-          verifyNotNull(eppIndex, "Missing EppResourceIndex for domain %s", domain);
-          ForeignKeyIndex<?> fki = ofy().load().key(ForeignKeyDomainIndex.createKey(domain)).now();
-          verifyNotNull(fki, "Missing ForeignKeyDomainIndex for domain %s", domain);
-          // This ancestor query selects all descendant HistoryEntries, BillingEvents, and
-          // PollMessages, as well as the domain itself.
+          // This ancestor query selects all descendant HistoryEntries, BillingEvents, PollMessages,
+          // and TLD-specific entities, as well as the domain itself.
           List<Key<Object>> domainAndDependentKeys = ofy().load().ancestor(domainKey).keys().list();
+          ImmutableSet<Key<?>> allKeys = new ImmutableSet.Builder<Key<?>>()
+              .add(fki)
+              .add(eppIndex)
+              .addAll(domainAndDependentKeys)
+              .build();
           if (isDryRun) {
-            logger.infofmt(
-                "Would delete the following entities: %s",
-                new ImmutableList.Builder<Object>()
-                    .add(fki)
-                    .add(eppIndex)
-                    .addAll(domainAndDependentKeys)
-                    .build());
+            logger.infofmt("Would delete the following entities: %s", allKeys);
           } else {
-            ofy().deleteWithoutBackup().keys(domainAndDependentKeys);
-            ofy().deleteWithoutBackup().entities(eppIndex, fki);
+            ofy().deleteWithoutBackup().keys(allKeys);
           }
-          return domainAndDependentKeys.size() - 1;
+          return allKeys.size();
         }
       });
-      getContext().incrementCounter(String.format("deleted, kind %s", domainKey.getKind()));
-      getContext().incrementCounter("deleted, dependent keys", dependentsDeleted);
+      getContext().incrementCounter("domains deleted");
+      getContext().incrementCounter("total entities deleted", entitiesDeleted);
     }
   }
 }