Run BSA Validate without lock (#2399)

As a read-only action that tolerates staleness, locking is unnecessary.
This should help with the lock contention we are observing.

Also reduces the number of VM instances provisioned for BSA and increase
the idle timeout. This should reduce invocation delay. Longer delay may
cause AppEngine to return `Timeout` status to Cloud Scheduler even
though the cron job succeeds.

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

@@ -81,7 +81,6 @@ public class BsaValidateAction implements Runnable {
   private final int transactionBatchSize;
   private final Duration maxStaleness;
   private final Clock clock;
-  private final BsaLock bsaLock;
   private final Response response;
 
   @Inject
@@ -92,7 +91,6 @@ public class BsaValidateAction implements Runnable {
       @Config("bsaTxnBatchSize") int transactionBatchSize,
       @Config("bsaValidationMaxStaleness") Duration maxStaleness,
       Clock clock,
-      BsaLock bsaLock,
       Response response) {
     this.gcsClient = gcsClient;
     this.idnChecker = idnChecker;
@@ -100,18 +98,13 @@ public class BsaValidateAction implements Runnable {
     this.transactionBatchSize = transactionBatchSize;
     this.maxStaleness = maxStaleness;
     this.clock = clock;
-    this.bsaLock = bsaLock;
     this.response = response;
   }
 
   @Override
   public void run() {
     try {
-      if (!bsaLock.executeWithLock(this::runWithinLock)) {
-        String message = "BSA validation did not run: another BSA related task is running";
-        logger.atInfo().log("%s.", message);
-        emailSender.sendNotification(message, /* body= */ "");
-      }
+      validate();
     } catch (Throwable throwable) {
       logger.atWarning().withCause(throwable).log("Failed to validate block lists.");
       emailSender.sendNotification("BSA validation aborted", getStackTraceAsString(throwable));
@@ -121,15 +114,15 @@ public class BsaValidateAction implements Runnable {
     response.setStatus(SC_OK);
   }
 
-  /** Executes the validation action while holding the BSA lock. */
-  Void runWithinLock() {
+  /** Performs validation of BSA data in the database. */
+  void validate() {
     Optional<String> downloadJobName =
         bsaQuery(DownloadScheduler::fetchMostRecentDownloadJobIdIfCompleted);
     if (downloadJobName.isEmpty()) {
       logger.atInfo().log("Cannot validate: block list downloads not found.");
       emailSender.sendNotification(
           "BSA validation does not run: block list downloads not found", "");
-      return null;
+      return;
     }
     logger.atInfo().log("Validating BSA with latest download: %s", downloadJobName.get());
 
@@ -148,7 +141,6 @@ public class BsaValidateAction implements Runnable {
       emailValidationResults(resultSummary, downloadJobName.get(), errors);
     }
     logger.atInfo().log("%s (latest download: %s)", resultSummary, downloadJobName.get());
-    return null;
   }
 
   void emailValidationResults(String subject, String jobName, ImmutableList<String> results) {

core/src/main/java/google/registry/env/production/bsa/WEB-INF/appengine-web.xml

@@ -7,8 +7,8 @@
   <sessions-enabled>true</sessions-enabled>
   <instance-class>B4_1G</instance-class>
   <basic-scaling>
-    <max-instances>100</max-instances>
-    <idle-timeout>10m</idle-timeout>
+    <max-instances>3</max-instances>
+    <idle-timeout>60m</idle-timeout>
   </basic-scaling>
 
   <system-properties>

core/src/main/java/google/registry/env/sandbox/bsa/WEB-INF/appengine-web.xml

@@ -7,8 +7,8 @@
   <sessions-enabled>true</sessions-enabled>
   <instance-class>B4</instance-class>
   <basic-scaling>
-    <max-instances>100</max-instances>
-    <idle-timeout>10m</idle-timeout>
+    <max-instances>3</max-instances>
+    <idle-timeout>60m</idle-timeout>
   </basic-scaling>
 
   <system-properties>

core/src/test/java/google/registry/bsa/BsaValidateActionTest.java

@@ -31,6 +31,7 @@ import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.startsWith;
 import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
@@ -56,7 +57,6 @@ import google.registry.testing.FakeClock;
 import google.registry.tldconfig.idn.IdnTableEnum;
 import google.registry.util.EmailMessage;
 import java.util.Optional;
-import java.util.concurrent.Callable;
 import java.util.stream.Stream;
 import javax.mail.internet.InternetAddress;
 import org.joda.time.DateTime;
@@ -90,8 +90,6 @@ public class BsaValidateActionTest {
 
   @Mock Response response;
 
-  @Mock private BsaLock bsaLock;
-
   @Mock private InternetAddress emailRecipient;
 
   @Captor ArgumentCaptor<EmailMessage> emailCaptor = ArgumentCaptor.forClass(EmailMessage.class);
@@ -112,7 +110,6 @@ public class BsaValidateActionTest {
             /* transactionBatchSize= */ 500,
             MAX_STALENESS,
             fakeClock,
-            bsaLock,
             response);
     createTld("app");
   }
@@ -357,22 +354,11 @@ public class BsaValidateActionTest {
             "Registered domain registered-missing.app missing or not recorded as REGISTERED");
   }
 
-  @Test
-  void notificationSent_cannotAcquireLock() {
-    when(bsaLock.executeWithLock(any())).thenReturn(false);
-    action.run();
-    verify(gmailClient, times(1))
-        .sendEmail(
-            EmailMessage.create(
-                "BSA validation did not run: another BSA related task is running",
-                "",
-                emailRecipient));
-  }
-
   @Test
   void notificationSent_abortedByException() {
+    action = spy(action);
     RuntimeException throwable = new RuntimeException("Error");
-    when(bsaLock.executeWithLock(any())).thenThrow(throwable);
+    doThrow(throwable).when(action).validate();
     action.run();
     verify(gmailClient, times(1))
         .sendEmail(
@@ -382,12 +368,6 @@ public class BsaValidateActionTest {
 
   @Test
   void notificationSent_noDownloads() {
-    when(bsaLock.executeWithLock(any()))
-        .thenAnswer(
-            args -> {
-              args.getArgument(0, Callable.class).call();
-              return true;
-            });
     action.run();
     verify(gmailClient, times(1))
         .sendEmail(
@@ -397,12 +377,6 @@ public class BsaValidateActionTest {
 
   @Test
   void notificationSent_withValidationError() {
-    when(bsaLock.executeWithLock(any()))
-        .thenAnswer(
-            args -> {
-              args.getArgument(0, Callable.class).call();
-              return true;
-            });
     persistDownloadSchedule(DownloadStage.DONE);
     action = spy(action);
     doReturn(ImmutableList.of("Error line 1.", "Error line 2"))
@@ -420,12 +394,6 @@ public class BsaValidateActionTest {
 
   @Test
   void notification_notSent_WhenNoError() {
-    when(bsaLock.executeWithLock(any()))
-        .thenAnswer(
-            args -> {
-              args.getArgument(0, Callable.class).call();
-              return true;
-            });
     persistDownloadSchedule(DownloadStage.DONE);
     action = spy(action);
     doReturn(ImmutableList.of()).when(action).checkBsaLabels(anyString());