From 0167dad85f2a86c0fce6c1d1ebff759323f1d642 Mon Sep 17 00:00:00 2001
From: Weimin Yu <weiminyu@google.com>
Date: Wed, 27 Aug 2025 15:47:13 +0000
Subject: [PATCH] Fix OOM error in BsaValidation (#2813)

Error happened in the case that an unblockable name reported with
'Registered' as reason has been deregistered. We tried to check the
deletion time of the domain to decide if this is a transient error
that is no worth reporting. However, we forgot that we do not have
the domain key in this case.

As best-effort action, and with a case that rarely happens, we decide
not to make the optimization (staleness check) in thise case.
---
 .../registry/bsa/BsaValidateAction.java       | 17 +++++-----
 .../registry/bsa/BsaValidateActionTest.java   | 31 +++++--------------
 2 files changed, 15 insertions(+), 33 deletions(-)

diff --git a/core/src/main/java/google/registry/bsa/BsaValidateAction.java b/core/src/main/java/google/registry/bsa/BsaValidateAction.java
index 98a042c16..158dd45f0 100644
--- a/core/src/main/java/google/registry/bsa/BsaValidateAction.java
+++ b/core/src/main/java/google/registry/bsa/BsaValidateAction.java
@@ -215,10 +215,12 @@ public class BsaValidateAction implements Runnable {
     if (Objects.equals(expectedReason, domain.reason())) {
       return Optional.empty();
     }
-    if (isRegistered || domain.reason().equals(Reason.REGISTERED)) {
-      if (isStalenessAllowed(isRegistered, activeDomains.get(domain.domainName()))) {
+    // Registered name still reported with other reasons: Don't report if registration is recent.
+    // Note that staleness is not tolerated if deregistered name is still reported as registered:
+    // in this case we do not have the VKey on hand, and it is not worth the effort to find it
+    // out.
+    if (isRegistered && isStalenessAllowed(activeDomains.get(domain.domainName()))) {
         return Optional.empty();
-      }
     }
     return Optional.of(
         String.format(
@@ -228,15 +230,10 @@ public class BsaValidateAction implements Runnable {
             domain.reason()));
   }
 
-  boolean isStalenessAllowed(boolean isNewDomain, VKey<Domain> domainVKey) {
+  boolean isStalenessAllowed(VKey<Domain> domainVKey) {
     Domain domain = bsaQuery(() -> replicaTm().loadByKey(domainVKey));
     var now = clock.nowUtc();
-    if (isNewDomain) {
-      return domain.getCreationTime().plus(maxStaleness).isAfter(now);
-    } else {
-      return domain.getDeletionTime().isBefore(now)
-          && domain.getDeletionTime().plus(maxStaleness).isAfter(now);
-    }
+    return domain.getCreationTime().plus(maxStaleness).isAfter(now);
   }
 
   /** Returns unique labels across all block lists in the download specified by {@code jobName}. */
diff --git a/core/src/test/java/google/registry/bsa/BsaValidateActionTest.java b/core/src/test/java/google/registry/bsa/BsaValidateActionTest.java
index bae897f97..ea3f92e3b 100644
--- a/core/src/test/java/google/registry/bsa/BsaValidateActionTest.java
+++ b/core/src/test/java/google/registry/bsa/BsaValidateActionTest.java
@@ -24,7 +24,6 @@ import static google.registry.bsa.persistence.BsaTestingUtils.persistDownloadSch
 import static google.registry.bsa.persistence.BsaTestingUtils.persistUnblockableDomain;
 import static google.registry.testing.DatabaseHelper.createTld;
 import static google.registry.testing.DatabaseHelper.persistActiveDomain;
-import static google.registry.testing.DatabaseHelper.persistDeletedDomain;
 import static google.registry.testing.DatabaseHelper.persistResource;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
 import static org.mockito.ArgumentMatchers.any;
@@ -218,7 +217,8 @@ public class BsaValidateActionTest {
         test1,1;2
         test2,3
         """;
-    String blockPlusContent = """
+    String blockPlusContent =
+        """
         domainLabel,orderIDs
         test2,4
         """;
@@ -239,7 +239,7 @@ public class BsaValidateActionTest {
     persistBsaLabel("label");
     Domain domain = persistActiveDomain("label.app", fakeClock.nowUtc());
     fakeClock.advanceBy(MAX_STALENESS.minus(Duration.standardSeconds(1)));
-    assertThat(action.isStalenessAllowed(true, domain.createVKey())).isTrue();
+    assertThat(action.isStalenessAllowed(domain.createVKey())).isTrue();
   }
 
   @Test
@@ -247,23 +247,7 @@ public class BsaValidateActionTest {
     persistBsaLabel("label");
     Domain domain = persistActiveDomain("label.app", fakeClock.nowUtc());
     fakeClock.advanceBy(MAX_STALENESS);
-    assertThat(action.isStalenessAllowed(true, domain.createVKey())).isFalse();
-  }
-
-  @Test
-  void isStalenessAllowed_deletedDomain_allowed() {
-    persistBsaLabel("label");
-    Domain domain = persistDeletedDomain("label.app", fakeClock.nowUtc());
-    fakeClock.advanceBy(MAX_STALENESS.minus(Duration.standardSeconds(1)));
-    assertThat(action.isStalenessAllowed(false, domain.createVKey())).isTrue();
-  }
-
-  @Test
-  void isStalenessAllowed_deletedDomain_notAllowed() {
-    persistBsaLabel("label");
-    Domain domain = persistDeletedDomain("label.app", fakeClock.nowUtc());
-    fakeClock.advanceBy(MAX_STALENESS);
-    assertThat(action.isStalenessAllowed(false, domain.createVKey())).isFalse();
+    assertThat(action.isStalenessAllowed(domain.createVKey())).isFalse();
   }
 
   @Test
@@ -405,10 +389,11 @@ public class BsaValidateActionTest {
     assertThat(message.body())
         .isEqualTo(
             """
-                Most recent download is 2023-11-09t020857.880z.
+            Most recent download is 2023-11-09t020857.880z.
 
-                Error line 1.
-                Error line 2""");
+            Error line 1.
+            Error line 2\
+            """);
   }
 
   @Test