From b81a0e99d4229a71561137198cd5f7e9471ff1df Mon Sep 17 00:00:00 2001 From: Chris Eager Date: Fri, 23 Jun 2023 12:34:46 -0500 Subject: [PATCH] Always have 0 `ApnPushNotificationScheduler` worker threads in front-end service --- .../textsecuregcm/WhisperServerService.java | 2 +- .../dynamic/DynamicConfiguration.java | 9 ----- ...edApnNotificationSendingConfiguration.java | 11 ------- .../push/ApnPushNotificationScheduler.java | 33 +++---------------- ...nPushNotificationSenderServiceCommand.java | 14 +------- .../ApnPushNotificationSchedulerTest.java | 32 ++++-------------- 6 files changed, 13 insertions(+), 88 deletions(-) delete mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicScheduledApnNotificationSendingConfiguration.java diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index 7a1da2c97..794606d57 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -490,7 +490,7 @@ public class WhisperServerService extends Application dynamicConfigurationManager; - - private static final int DEFAULT_WORKER_THREAD_COUNT = 4; @VisibleForTesting static final Duration BACKGROUND_NOTIFICATION_PERIOD = Duration.ofMinutes(20); @@ -105,18 +99,6 @@ public class ApnPushNotificationScheduler implements Managed { } private long processNextSlot() { - if (dedicatedProcess) { - if (!dynamicConfigurationManager.getConfiguration().getScheduledApnNotificationSendingConfiguration() - .enabledForDedicatedProcess()) { - return 0; - } - } else { - if (!dynamicConfigurationManager.getConfiguration().getScheduledApnNotificationSendingConfiguration() - .enabledForServer()) { - return 0; - } - } - final int slot = (int) (pushSchedulingCluster.withCluster(connection -> connection.sync().incr(NEXT_SLOT_TO_PROCESS_KEY)) % SlotHash.SLOT_COUNT); @@ -181,11 +163,10 @@ public class ApnPushNotificationScheduler implements Managed { } public ApnPushNotificationScheduler(FaultTolerantRedisCluster pushSchedulingCluster, - APNSender apnSender, AccountsManager accountsManager, final Optional dedicatedProcessWorkerThreadCount, - DynamicConfigurationManager dynamicConfigurationManager) throws IOException { + APNSender apnSender, AccountsManager accountsManager, final int dedicatedProcessWorkerThreadCount) + throws IOException { - this(pushSchedulingCluster, apnSender, accountsManager, Clock.systemUTC(), dedicatedProcessWorkerThreadCount, - dynamicConfigurationManager); + this(pushSchedulingCluster, apnSender, accountsManager, Clock.systemUTC(), dedicatedProcessWorkerThreadCount); } @VisibleForTesting @@ -193,8 +174,7 @@ public class ApnPushNotificationScheduler implements Managed { APNSender apnSender, AccountsManager accountsManager, Clock clock, - Optional dedicatedProcessThreadCount, - DynamicConfigurationManager dynamicConfigurationManager) throws IOException { + int dedicatedProcessThreadCount) throws IOException { this.apnSender = apnSender; this.accountsManager = accountsManager; @@ -211,14 +191,11 @@ public class ApnPushNotificationScheduler implements Managed { this.scheduleBackgroundNotificationScript = ClusterLuaScript.fromResource(pushSchedulingCluster, "lua/apn/schedule_background_notification.lua", ScriptOutputType.VALUE); - this.workerThreads = dedicatedProcessThreadCount.map(Thread[]::new) - .orElseGet(() -> new Thread[DEFAULT_WORKER_THREAD_COUNT]); + this.workerThreads = new Thread[dedicatedProcessThreadCount]; for (int i = 0; i < this.workerThreads.length; i++) { this.workerThreads[i] = new Thread(new NotificationWorker(), "ApnFallbackManagerWorker-" + i); } - this.dedicatedProcess = dedicatedProcessThreadCount.isPresent(); - this.dynamicConfigurationManager = dynamicConfigurationManager; } /** diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/workers/ScheduledApnPushNotificationSenderServiceCommand.java b/service/src/main/java/org/whispersystems/textsecuregcm/workers/ScheduledApnPushNotificationSenderServiceCommand.java index b2a95ed02..6a5e98b90 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/workers/ScheduledApnPushNotificationSenderServiceCommand.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/workers/ScheduledApnPushNotificationSenderServiceCommand.java @@ -10,17 +10,14 @@ import static com.codahale.metrics.MetricRegistry.name; import io.dropwizard.Application; import io.dropwizard.cli.ServerCommand; import io.dropwizard.setup.Environment; -import java.util.Optional; import java.util.concurrent.ExecutorService; import net.sourceforge.argparse4j.inf.Namespace; import net.sourceforge.argparse4j.inf.Subparser; import org.whispersystems.textsecuregcm.WhisperServerConfiguration; -import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration; import org.whispersystems.textsecuregcm.metrics.MetricsUtil; import org.whispersystems.textsecuregcm.push.APNSender; import org.whispersystems.textsecuregcm.push.ApnPushNotificationScheduler; import org.whispersystems.textsecuregcm.redis.FaultTolerantRedisCluster; -import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager; import org.whispersystems.textsecuregcm.util.logging.UncaughtExceptionHandler; public class ScheduledApnPushNotificationSenderServiceCommand extends ServerCommand { @@ -63,18 +60,9 @@ public class ScheduledApnPushNotificationSenderServiceCommand extends ServerComm final ExecutorService apnSenderExecutor = environment.lifecycle().executorService(name(getClass(), "apnSender-%d")) .maxThreads(1).minThreads(1).build(); - final DynamicConfigurationManager dynamicConfigurationManager = new DynamicConfigurationManager<>( - configuration.getAppConfig().getApplication(), - configuration.getAppConfig().getEnvironment(), - configuration.getAppConfig().getConfigurationName(), - DynamicConfiguration.class); - - dynamicConfigurationManager.start(); - final APNSender apnSender = new APNSender(apnSenderExecutor, configuration.getApnConfiguration()); final ApnPushNotificationScheduler apnPushNotificationScheduler = new ApnPushNotificationScheduler( - pushSchedulerCluster, apnSender, deps.accountsManager(), Optional.of(namespace.getInt(WORKER_COUNT)), - dynamicConfigurationManager); + pushSchedulerCluster, apnSender, deps.accountsManager(), namespace.getInt(WORKER_COUNT)); environment.lifecycle().manage(apnSender); environment.lifecycle().manage(apnPushNotificationScheduler); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/push/ApnPushNotificationSchedulerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/push/ApnPushNotificationSchedulerTest.java index 31511a4d6..714e43625 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/push/ApnPushNotificationSchedulerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/push/ApnPushNotificationSchedulerTest.java @@ -31,13 +31,11 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; import org.mockito.ArgumentCaptor; import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration; -import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicScheduledApnNotificationSendingConfiguration; import org.whispersystems.textsecuregcm.redis.FaultTolerantRedisCluster; import org.whispersystems.textsecuregcm.redis.RedisClusterExtension; import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.AccountsManager; import org.whispersystems.textsecuregcm.storage.Device; -import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager; import org.whispersystems.textsecuregcm.util.Pair; import org.whispersystems.textsecuregcm.util.TestClock; @@ -81,17 +79,8 @@ class ApnPushNotificationSchedulerTest { apnSender = mock(APNSender.class); clock = TestClock.now(); - final DynamicConfigurationManager dynamicConfigurationManager = mock( - DynamicConfigurationManager.class); - final DynamicConfiguration dynamicConfiguration = mock(DynamicConfiguration.class); - final DynamicScheduledApnNotificationSendingConfiguration scheduledApnNotificationSendingConfiguration = new DynamicScheduledApnNotificationSendingConfiguration( - true, true); - when(dynamicConfiguration.getScheduledApnNotificationSendingConfiguration()).thenReturn( - scheduledApnNotificationSendingConfiguration); - when(dynamicConfigurationManager.getConfiguration()).thenReturn(dynamicConfiguration); - apnPushNotificationScheduler = new ApnPushNotificationScheduler(REDIS_CLUSTER_EXTENSION.getRedisCluster(), - apnSender, accountsManager, clock, Optional.empty(), dynamicConfigurationManager); + apnSender, accountsManager, clock, 1); } @Test @@ -236,30 +225,21 @@ class ApnPushNotificationSchedulerTest { @ParameterizedTest @CsvSource({ - "true, false, true, true", - "true, true, false, false", - "false, true, false, true", - "false, false, true, false" + "1, true", + "0, false", }) - void testDedicatedProcessDynamicConfiguration(final boolean dedicatedProcess, final boolean enabledForServer, - final boolean enabledForDedicatedProcess, final boolean expectActivity) throws Exception { + void testDedicatedProcessDynamicConfiguration(final int dedicatedThreadCount, final boolean expectActivity) + throws Exception { final FaultTolerantRedisCluster redisCluster = mock(FaultTolerantRedisCluster.class); when(redisCluster.withCluster(any())).thenReturn(0L); final AccountsManager accountsManager = mock(AccountsManager.class); - final DynamicConfigurationManager dynamicConfigurationManager = mock( - DynamicConfigurationManager.class); final DynamicConfiguration dynamicConfiguration = mock(DynamicConfiguration.class); - when(dynamicConfigurationManager.getConfiguration()).thenReturn(dynamicConfiguration); - final DynamicScheduledApnNotificationSendingConfiguration scheduledApnNotificationSendingConfiguration = new DynamicScheduledApnNotificationSendingConfiguration( - enabledForServer, enabledForDedicatedProcess); - when(dynamicConfiguration.getScheduledApnNotificationSendingConfiguration()).thenReturn( - scheduledApnNotificationSendingConfiguration); apnPushNotificationScheduler = new ApnPushNotificationScheduler(redisCluster, apnSender, - accountsManager, dedicatedProcess ? Optional.of(4) : Optional.empty(), dynamicConfigurationManager); + accountsManager, dedicatedThreadCount); apnPushNotificationScheduler.start(); apnPushNotificationScheduler.stop();