From 07bbe7dfb2108112af517579173b02b6208f47bc Mon Sep 17 00:00:00 2001 From: Jon Chambers Date: Mon, 24 Aug 2020 21:33:35 -0400 Subject: [PATCH] Return to an async model for push notification latency. --- .../metrics/PushLatencyManager.java | 34 +++++++------------ .../metrics/PushLatencyManagerTest.java | 11 +++--- 2 files changed, 19 insertions(+), 26 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/metrics/PushLatencyManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/metrics/PushLatencyManager.java index 557e8bb8e..1f6aae3dc 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/metrics/PushLatencyManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/metrics/PushLatencyManager.java @@ -3,15 +3,13 @@ package org.whispersystems.textsecuregcm.metrics; import com.codahale.metrics.MetricRegistry; import com.google.common.annotations.VisibleForTesting; import io.lettuce.core.SetArgs; -import io.lettuce.core.cluster.api.sync.RedisAdvancedClusterCommands; +import io.lettuce.core.cluster.api.async.RedisAdvancedClusterAsyncCommands; import io.micrometer.core.instrument.Metrics; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.whispersystems.textsecuregcm.redis.FaultTolerantRedisCluster; import java.time.Duration; -import java.util.Optional; import java.util.UUID; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; /** @@ -29,8 +27,6 @@ public class PushLatencyManager { private final FaultTolerantRedisCluster redisCluster; - private static final Logger log = LoggerFactory.getLogger(PushLatencyManager.class); - public PushLatencyManager(final FaultTolerantRedisCluster redisCluster) { this.redisCluster = redisCluster; } @@ -41,33 +37,29 @@ public class PushLatencyManager { @VisibleForTesting void recordPushSent(final UUID accountUuid, final long deviceId, final long currentTime) { - try { - redisCluster.useCluster(connection -> - connection.sync().set(getFirstUnacknowledgedPushKey(accountUuid, deviceId), String.valueOf(currentTime), SetArgs.Builder.nx().ex(TTL))); - } catch (final Exception e) { - log.warn("Failed to record \"push notification sent\" timestamp", e); - } + redisCluster.useCluster(connection -> + connection.async().set(getFirstUnacknowledgedPushKey(accountUuid, deviceId), String.valueOf(currentTime), SetArgs.Builder.nx().ex(TTL))); } public void recordQueueRead(final UUID accountUuid, final long deviceId, final String userAgent) { - final Optional maybeLatency = getLatencyAndClearTimestamp(accountUuid, deviceId, System.currentTimeMillis()); - - if (maybeLatency.isPresent()) { - Metrics.timer(TIMER_NAME, UserAgentTagUtil.getUserAgentTags(userAgent)).record(maybeLatency.get(), TimeUnit.MILLISECONDS); - } + getLatencyAndClearTimestamp(accountUuid, deviceId, System.currentTimeMillis()).thenAccept(latency -> { + if (latency != null) { + Metrics.timer(TIMER_NAME, UserAgentTagUtil.getUserAgentTags(userAgent)).record(latency, TimeUnit.MILLISECONDS); + } + }); } @VisibleForTesting - Optional getLatencyAndClearTimestamp(final UUID accountUuid, final long deviceId, final long currentTimeMillis) { + CompletableFuture getLatencyAndClearTimestamp(final UUID accountUuid, final long deviceId, final long currentTimeMillis) { final String key = getFirstUnacknowledgedPushKey(accountUuid, deviceId); return redisCluster.withCluster(connection -> { - final RedisAdvancedClusterCommands commands = connection.sync(); + final RedisAdvancedClusterAsyncCommands commands = connection.async(); - final String timestampString = commands.get(key); + final CompletableFuture getFuture = commands.get(key).toCompletableFuture(); commands.del(key); - return timestampString != null ? Optional.of(currentTimeMillis - Long.parseLong(timestampString, 10)) : Optional.empty(); + return getFuture.thenApply(timestampString -> timestampString != null ? currentTimeMillis - Long.parseLong(timestampString, 10) : null); }); } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/metrics/PushLatencyManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/metrics/PushLatencyManagerTest.java index 7024a1676..47bb05ed9 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/metrics/PushLatencyManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/metrics/PushLatencyManagerTest.java @@ -3,15 +3,16 @@ package org.whispersystems.textsecuregcm.metrics; import org.junit.Test; import org.whispersystems.textsecuregcm.redis.AbstractRedisClusterTest; -import java.util.Optional; import java.util.UUID; +import java.util.concurrent.ExecutionException; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; public class PushLatencyManagerTest extends AbstractRedisClusterTest { @Test - public void testGetLatency() { + public void testGetLatency() throws ExecutionException, InterruptedException { final PushLatencyManager pushLatencyManager = new PushLatencyManager(getRedisCluster()); final UUID accountUuid = UUID.randomUUID(); final long deviceId = 1; @@ -19,13 +20,13 @@ public class PushLatencyManagerTest extends AbstractRedisClusterTest { final long pushSentTimestamp = System.currentTimeMillis(); final long clearQueueTimestamp = pushSentTimestamp + expectedLatency; - assertEquals(Optional.empty(), pushLatencyManager.getLatencyAndClearTimestamp(accountUuid, deviceId, System.currentTimeMillis())); + assertNull(pushLatencyManager.getLatencyAndClearTimestamp(accountUuid, deviceId, System.currentTimeMillis()).get()); { pushLatencyManager.recordPushSent(accountUuid, deviceId, pushSentTimestamp); - assertEquals(Optional.of(expectedLatency), pushLatencyManager.getLatencyAndClearTimestamp(accountUuid, deviceId, clearQueueTimestamp)); - assertEquals(Optional.empty(), pushLatencyManager.getLatencyAndClearTimestamp(accountUuid, deviceId, System.currentTimeMillis())); + assertEquals(expectedLatency, (long)pushLatencyManager.getLatencyAndClearTimestamp(accountUuid, deviceId, clearQueueTimestamp).get()); + assertNull(pushLatencyManager.getLatencyAndClearTimestamp(accountUuid, deviceId, System.currentTimeMillis()).get()); } } }