From 1fc4a281c00198033118cf2e121d38e8ca0491a9 Mon Sep 17 00:00:00 2001 From: gbrodman Date: Mon, 3 Aug 2026 16:17:12 -0400 Subject: [PATCH] Add an IncrementalMetric for sync-cache-action runs (#3195) This is configured to run every 5 minutes. We need to make sure that the cache doesn't get too out of date, otherwise we'll be serving stale data. We'll add an alert that fires if SUCCESS or NOT_CONFIGURED hasn't happened recently. --- .../registry/batch/SyncRemoteCacheAction.java | 27 +++++++++++++++++++ .../batch/SyncRemoteCacheActionTest.java | 22 +++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/core/src/main/java/google/registry/batch/SyncRemoteCacheAction.java b/core/src/main/java/google/registry/batch/SyncRemoteCacheAction.java index 6ba502b24..8f3c61073 100644 --- a/core/src/main/java/google/registry/batch/SyncRemoteCacheAction.java +++ b/core/src/main/java/google/registry/batch/SyncRemoteCacheAction.java @@ -28,6 +28,9 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.flogger.FluentLogger; import com.google.common.net.MediaType; +import com.google.monitoring.metrics.IncrementableMetric; +import com.google.monitoring.metrics.LabelDescriptor; +import com.google.monitoring.metrics.MetricRegistryImpl; import google.registry.cache.SimplifiedJedisClient; import google.registry.model.EppResource; import google.registry.model.common.Cursor; @@ -39,6 +42,7 @@ import google.registry.request.Action; import google.registry.request.Response; import google.registry.request.auth.Auth; import google.registry.request.lock.LockHandler; +import google.registry.util.NonFinalForTesting; import jakarta.inject.Inject; import java.time.Duration; import java.time.Instant; @@ -61,6 +65,25 @@ public class SyncRemoteCacheAction implements Runnable { private static final String LOCK_NAME = "syncRemoteCacheAction"; private static final int BATCH_SIZE = 10000; + public enum SyncStatus { + SUCCESS, + FAILURE, + NOT_CONFIGURED + } + + private static final ImmutableSet LABEL_DESCRIPTORS = + ImmutableSet.of( + LabelDescriptor.create("status", "Whether SyncRemoteCacheAction succeeded or failed.")); + + @NonFinalForTesting + static final IncrementableMetric SYNC_CACHE_RUNS_METRIC = + MetricRegistryImpl.getDefault() + .newIncrementableMetric( + "/batch/sync_remote_cache/runs", + "Count of SyncRemoteCacheAction executions", + "count", + LABEL_DESCRIPTORS); + private final LockHandler lockHandler; private final Response response; private final Optional jedisClient; @@ -79,14 +102,17 @@ public class SyncRemoteCacheAction implements Runnable { if (jedisClient.isEmpty()) { response.setStatus(SC_NO_CONTENT); response.setPayload("No Jedis/Valkey configuration found"); + SYNC_CACHE_RUNS_METRIC.increment(SyncStatus.NOT_CONFIGURED.name()); return; } Callable runner = () -> { try { runLocked(); + SYNC_CACHE_RUNS_METRIC.increment(SyncStatus.SUCCESS.name()); response.setStatus(SC_OK); } catch (Exception e) { + SYNC_CACHE_RUNS_METRIC.increment(SyncStatus.FAILURE.name()); logger.atSevere().withCause(e).log("Errored out during execution."); response.setStatus(SC_INTERNAL_SERVER_ERROR); response.setPayload(String.format("Errored out with cause: %s", e)); @@ -95,6 +121,7 @@ public class SyncRemoteCacheAction implements Runnable { }; if (!lockHandler.executeWithLocks(runner, null, Duration.ofHours(1), LOCK_NAME)) { + SYNC_CACHE_RUNS_METRIC.increment(SyncStatus.FAILURE.name()); // Send a 200-series status code to prevent this conflicting action from retrying. response.setStatus(SC_NO_CONTENT); response.setPayload("Could not acquire lock; already running?"); diff --git a/core/src/test/java/google/registry/batch/SyncRemoteCacheActionTest.java b/core/src/test/java/google/registry/batch/SyncRemoteCacheActionTest.java index b7558e78e..78f22e680 100644 --- a/core/src/test/java/google/registry/batch/SyncRemoteCacheActionTest.java +++ b/core/src/test/java/google/registry/batch/SyncRemoteCacheActionTest.java @@ -15,6 +15,10 @@ package google.registry.batch; import static com.google.common.truth.Truth.assertThat; +import static com.google.monitoring.metrics.contrib.LongMetricSubject.assertThat; +import static google.registry.batch.SyncRemoteCacheAction.SyncStatus.FAILURE; +import static google.registry.batch.SyncRemoteCacheAction.SyncStatus.NOT_CONFIGURED; +import static google.registry.batch.SyncRemoteCacheAction.SyncStatus.SUCCESS; import static google.registry.model.common.Cursor.CursorType.REMOTE_CACHE_DOMAIN_SYNC; import static google.registry.model.common.Cursor.CursorType.REMOTE_CACHE_HOST_SYNC; import static google.registry.testing.DatabaseHelper.createTld; @@ -73,15 +77,24 @@ class SyncRemoteCacheActionTest { @BeforeEach void beforeEach() { createTld("tld"); + SyncRemoteCacheAction.SYNC_CACHE_RUNS_METRIC.reset(); action = new SyncRemoteCacheAction(lockHandler, response, Optional.of(jedisClient)); } + private static void verifyMetrics(SyncRemoteCacheAction.SyncStatus status) { + assertThat(SyncRemoteCacheAction.SYNC_CACHE_RUNS_METRIC) + .hasValueForLabels(1, status.name()) + .and() + .hasNoOtherValues(); + } + @Test void test_noJedisConfig() { action = new SyncRemoteCacheAction(lockHandler, response, Optional.empty()); action.run(); assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT); assertThat(response.getPayload()).contains("No Jedis/Valkey configuration found"); + verifyMetrics(NOT_CONFIGURED); } @Test @@ -91,6 +104,7 @@ class SyncRemoteCacheActionTest { action.run(); assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT); assertThat(response.getPayload()).contains("Could not acquire lock"); + verifyMetrics(FAILURE); } @Test @@ -100,6 +114,7 @@ class SyncRemoteCacheActionTest { action.run(); assertThat(response.getStatus()).isEqualTo(SC_INTERNAL_SERVER_ERROR); assertThat(response.getPayload()).contains("Errored out with cause"); + verifyMetrics(FAILURE); } @Test @@ -109,6 +124,7 @@ class SyncRemoteCacheActionTest { verifyNoInteractions(jedisClient); assertThat(DatabaseHelper.loadByKeyIfPresent(Cursor.createGlobalVKey(REMOTE_CACHE_DOMAIN_SYNC))) .isEmpty(); + verifyMetrics(SUCCESS); } @Test @@ -131,6 +147,7 @@ class SyncRemoteCacheActionTest { .getCursorTime() .toString()) .isEqualTo("2025-01-01T00:00:00.001Z"); + verifyMetrics(SUCCESS); } @Test @@ -146,6 +163,7 @@ class SyncRemoteCacheActionTest { ImmutableList.of( new SimplifiedJedisClient.JedisResource<>("active.tld", activeDomain))); verify(jedisClient).deleteAll(Domain.class, ImmutableList.of("deleted.tld")); + verifyMetrics(SUCCESS); } @Test @@ -166,6 +184,7 @@ class SyncRemoteCacheActionTest { verify(jedisClient) .setAll( ImmutableList.of(new SimplifiedJedisClient.JedisResource<>("example2.tld", domain2))); + verifyMetrics(SUCCESS); } @Test @@ -175,6 +194,7 @@ class SyncRemoteCacheActionTest { verifyNoInteractions(jedisClient); assertThat(DatabaseHelper.loadByKeyIfPresent(Cursor.createGlobalVKey(REMOTE_CACHE_HOST_SYNC))) .isEmpty(); + verifyMetrics(SUCCESS); } @Test @@ -197,6 +217,7 @@ class SyncRemoteCacheActionTest { .getCursorTime() .toString()) .isEqualTo("2025-01-01T00:00:00.001Z"); + verifyMetrics(SUCCESS); } @Test @@ -212,5 +233,6 @@ class SyncRemoteCacheActionTest { ImmutableList.of( new SimplifiedJedisClient.JedisResource<>(active.getRepoId(), active))); verify(jedisClient).deleteAll(Host.class, ImmutableList.of(deleted.getRepoId())); + verifyMetrics(SUCCESS); } }