diff --git a/core/src/main/java/google/registry/beam/spec11/SafeBrowsingTransforms.java b/core/src/main/java/google/registry/beam/spec11/SafeBrowsingTransforms.java index 1a5fa856a..bf876e986 100644 --- a/core/src/main/java/google/registry/beam/spec11/SafeBrowsingTransforms.java +++ b/core/src/main/java/google/registry/beam/spec11/SafeBrowsingTransforms.java @@ -102,12 +102,12 @@ public class SafeBrowsingTransforms { * because class methods are generally serializable, especially a static function such as {@link * HttpClients#createDefault()}. */ - @SuppressWarnings("unchecked") EvaluateSafeBrowsingFn(String apiKey, Retrier retrier, Clock clock) { - this.apiKey = apiKey; - this.retrier = retrier; - this.clock = clock; - closeableHttpClientSupplier = (Supplier & Serializable) HttpClients::createDefault; + this( + apiKey, + retrier, + clock, + (Supplier & Serializable) HttpClients::createDefault); } /** @@ -122,7 +122,7 @@ public class SafeBrowsingTransforms { this.apiKey = apiKey; this.retrier = retrier; this.clock = clock; - closeableHttpClientSupplier = clientSupplier; + this.closeableHttpClientSupplier = clientSupplier; } /** Evaluates any buffered {@link DomainNameInfo} objects upon completing the bundle. */ diff --git a/core/src/main/java/google/registry/beam/spec11/Spec11Pipeline.java b/core/src/main/java/google/registry/beam/spec11/Spec11Pipeline.java index 370bb88ac..f4412c15a 100644 --- a/core/src/main/java/google/registry/beam/spec11/Spec11Pipeline.java +++ b/core/src/main/java/google/registry/beam/spec11/Spec11Pipeline.java @@ -29,6 +29,7 @@ import google.registry.model.reporting.Spec11ThreatMatch.ThreatType; import google.registry.persistence.PersistenceModule.TransactionIsolationLevel; import google.registry.util.Clock; import google.registry.util.Retrier; +import google.registry.util.Sleeper; import google.registry.util.UtilsModule; import jakarta.inject.Singleton; import java.io.Serializable; @@ -238,8 +239,11 @@ public class Spec11Pipeline implements Serializable { @Provides EvaluateSafeBrowsingFn provideSafeBrowsingFn( - Spec11PipelineOptions options, Retrier retrier, Clock clock) { - return new EvaluateSafeBrowsingFn(options.getSafeBrowsingApiKey(), retrier, clock); + Spec11PipelineOptions options, Clock clock, Sleeper sleeper) { + // Have a noticeably longer backoff for SafeBrowsing retries to mitigate any 429s + Retrier safeBrowsingRetrier = new Retrier(sleeper, 4, 1000L); + return new EvaluateSafeBrowsingFn( + options.getSafeBrowsingApiKey(), safeBrowsingRetrier, clock); } @Provides diff --git a/core/src/main/java/google/registry/config/RegistryConfig.java b/core/src/main/java/google/registry/config/RegistryConfig.java index 9ce647915..b4f963825 100644 --- a/core/src/main/java/google/registry/config/RegistryConfig.java +++ b/core/src/main/java/google/registry/config/RegistryConfig.java @@ -999,6 +999,12 @@ public final class RegistryConfig { return config.misc.transientFailureRetries; } + @Provides + @Named("transientFailureBaseIntervalMillis") + public static long provideTransientFailureBaseIntervalMillis() { + return 100L; + } + /** * Maximum number of results to return for an RDAP search query * diff --git a/util/src/main/java/google/registry/util/Retrier.java b/util/src/main/java/google/registry/util/Retrier.java index 0a3e45400..52ccf8826 100644 --- a/util/src/main/java/google/registry/util/Retrier.java +++ b/util/src/main/java/google/registry/util/Retrier.java @@ -19,6 +19,7 @@ import static com.google.common.base.Throwables.throwIfUnchecked; import static com.google.common.math.IntMath.pow; import static google.registry.util.PredicateUtils.supertypeOf; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableSet; import com.google.common.flogger.FluentLogger; import jakarta.inject.Inject; @@ -41,6 +42,7 @@ public class Retrier implements Serializable { private final Sleeper sleeper; private final int attempts; + private final long baseIntervalMillis; /** Holds functions to call whenever the code being retried fails. */ public interface FailureReporter { @@ -55,11 +57,21 @@ public class Retrier implements Serializable { void beforeRetry(Throwable thrown, int failures, int maxAttempts); } + @VisibleForTesting + public Retrier(Sleeper sleeper, int transientFailureRetries) { + this(sleeper, transientFailureRetries, 100L); + } + @Inject - public Retrier(Sleeper sleeper, @Named("transientFailureRetries") int transientFailureRetries) { + public Retrier( + Sleeper sleeper, + @Named("transientFailureRetries") int transientFailureRetries, + @Named("transientFailureBaseIntervalMillis") long baseIntervalMillis) { this.sleeper = sleeper; checkArgument(transientFailureRetries > 0, "Number of attempts must be positive"); this.attempts = transientFailureRetries; + checkArgument(baseIntervalMillis > 0, "Base interval millis must be positive"); + this.baseIntervalMillis = baseIntervalMillis; } /** @@ -160,8 +172,8 @@ public class Retrier implements Serializable { throw new RuntimeException(e); } failureReporter.beforeRetry(e, failures, attempts); - // Wait (skewed) 100ms on the first attempt, doubling on each subsequent attempt. - long backoffMillis = pow(2, failures) * 100L; + // Wait (skewed) baseIntervalMillis on the first attempt, doubling on each attempt + long backoffMillis = pow(2, failures) * baseIntervalMillis; long sleepDurationMillis = Math.round(randomForSkew.nextDouble(0.8, 1.2) * backoffMillis); try { sleeper.sleep(Duration.ofMillis(sleepDurationMillis));