From a6f9409a39e00384b832da1b4f7dec5b2dba35ac Mon Sep 17 00:00:00 2001 From: Chris Eager Date: Wed, 27 Jul 2022 13:59:46 -0500 Subject: [PATCH] Remove dynamic configuration feature flags; add `DynamicMessagePersisterConfiguration` --- .../dynamic/DynamicConfiguration.java | 20 +++--- .../DynamicMessagePersisterConfiguration.java | 18 ++++++ .../storage/MessagePersister.java | 9 ++- .../dynamic/DynamicConfigurationTest.java | 62 ++++++++++++------- .../DynamicConfigurationManagerTest.java | 11 ++-- 5 files changed, 78 insertions(+), 42 deletions(-) create mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicMessagePersisterConfiguration.java diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicConfiguration.java b/service/src/main/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicConfiguration.java index d23e36732..ef7396404 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicConfiguration.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicConfiguration.java @@ -5,7 +5,6 @@ import com.google.common.annotations.VisibleForTesting; import java.util.Collections; import java.util.Map; import java.util.Optional; -import java.util.Set; import javax.validation.Valid; public class DynamicConfiguration { @@ -30,9 +29,6 @@ public class DynamicConfiguration { @Valid private DynamicPaymentsConfiguration payments = new DynamicPaymentsConfiguration(); - @JsonProperty - private Set featureFlags = Collections.emptySet(); - @JsonProperty @Valid private DynamicTwilioConfiguration twilio = new DynamicTwilioConfiguration(); @@ -64,6 +60,10 @@ public class DynamicConfiguration { @Valid DynamicAbusiveHostRulesConfiguration abusiveHostRules = new DynamicAbusiveHostRulesConfiguration(); + @JsonProperty + @Valid + DynamicMessagePersisterConfiguration messagePersister = new DynamicMessagePersisterConfiguration(); + public Optional getExperimentEnrollmentConfiguration( final String experimentName) { return Optional.ofNullable(experiments.get(experimentName)); @@ -86,10 +86,6 @@ public class DynamicConfiguration { return payments; } - public Set getActiveFeatureFlags() { - return featureFlags; - } - public DynamicTwilioConfiguration getTwilioConfiguration() { return twilio; } @@ -115,7 +111,9 @@ public class DynamicConfiguration { return pushLatency; } - public DynamicUakMigrationConfiguration getUakMigrationConfiguration() { return uakMigrationConfiguration; } + public DynamicUakMigrationConfiguration getUakMigrationConfiguration() { + return uakMigrationConfiguration; + } public DynamicTurnConfiguration getTurnConfiguration() { return turn; @@ -124,4 +122,8 @@ public class DynamicConfiguration { public DynamicAbusiveHostRulesConfiguration getAbusiveHostRules() { return abusiveHostRules; } + + public DynamicMessagePersisterConfiguration getMessagePersisterConfiguration() { + return messagePersister; + } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicMessagePersisterConfiguration.java b/service/src/main/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicMessagePersisterConfiguration.java new file mode 100644 index 000000000..d74cac20d --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicMessagePersisterConfiguration.java @@ -0,0 +1,18 @@ +/* + * Copyright 2022 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.whispersystems.textsecuregcm.configuration.dynamic; + +import com.fasterxml.jackson.annotation.JsonProperty; + +public class DynamicMessagePersisterConfiguration { + + @JsonProperty + private boolean persistenceEnabled = true; + + public boolean isPersistenceEnabled() { + return persistenceEnabled; + } +} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagePersister.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagePersister.java index 642f395c1..baa532f30 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagePersister.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagePersister.java @@ -50,7 +50,6 @@ public class MessagePersister implements Managed { private static final long EXCEPTION_PAUSE_MILLIS = Duration.ofSeconds(3).toMillis(); - private static final String DISABLE_PERSISTER_FEATURE_FLAG = "DISABLE_MESSAGE_PERSISTER"; private static final int WORKER_THREAD_COUNT = 4; private static final int CONSECUTIVE_EMPTY_CACHE_REMOVAL_LIMIT = 3; @@ -69,10 +68,8 @@ public class MessagePersister implements Managed { for (int i = 0; i < workerThreads.length; i++) { workerThreads[i] = new Thread(() -> { while (running) { - if (dynamicConfigurationManager.getConfiguration().getActiveFeatureFlags() - .contains(DISABLE_PERSISTER_FEATURE_FLAG)) { - Util.sleep(1000); - } else { + if (dynamicConfigurationManager.getConfiguration().getMessagePersisterConfiguration() + .isPersistenceEnabled()) { try { final int queuesPersisted = persistNextQueues(Instant.now()); queueCountHistogram.update(queuesPersisted); @@ -84,6 +81,8 @@ public class MessagePersister implements Managed { logger.warn("Failed to persist queues", t); Util.sleep(EXCEPTION_PAUSE_MILLIS); } + } else { + Util.sleep(1000); } } }, "MessagePersisterWorker-" + i); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicConfigurationTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicConfigurationTest.java index b3776d735..c0a01f198 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicConfigurationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicConfigurationTest.java @@ -201,29 +201,6 @@ class DynamicConfigurationTest { } } - @Test - void testParseFeatureFlags() throws JsonProcessingException { - { - final String emptyConfigYaml = REQUIRED_CONFIG.concat("test: true"); - final DynamicConfiguration emptyConfig = - DynamicConfigurationManager.parseConfiguration(emptyConfigYaml, DynamicConfiguration.class).orElseThrow(); - - assertTrue(emptyConfig.getActiveFeatureFlags().isEmpty()); - } - - { - final String featureFlagYaml = REQUIRED_CONFIG.concat(""" - featureFlags: - - testFlag - """); - - final DynamicConfiguration emptyConfig = - DynamicConfigurationManager.parseConfiguration(featureFlagYaml, DynamicConfiguration.class).orElseThrow(); - - assertTrue(emptyConfig.getActiveFeatureFlags().contains("testFlag")); - } - } - @Test void testParseTwilioConfiguration() throws JsonProcessingException { { @@ -435,6 +412,43 @@ class DynamicConfigurationTest { assertThat(turnConfiguration.getUriConfigs().get(1).getEnrolledNumbers()).containsExactly("+15555555555"); } - } + + @Test + void testMessagePersister() throws JsonProcessingException { + { + final String emptyConfigYaml = REQUIRED_CONFIG.concat("test: true"); + final DynamicConfiguration emptyConfig = + DynamicConfigurationManager.parseConfiguration(emptyConfigYaml, DynamicConfiguration.class).orElseThrow(); + + assertTrue(emptyConfig.getMessagePersisterConfiguration().isPersistenceEnabled()); + } + + { + final String messagePersisterEnabledYaml = REQUIRED_CONFIG.concat(""" + messagePersister: + persistenceEnabled: true + """); + + final DynamicConfiguration config = + DynamicConfigurationManager.parseConfiguration(messagePersisterEnabledYaml, DynamicConfiguration.class) + .orElseThrow(); + + assertTrue(config.getMessagePersisterConfiguration().isPersistenceEnabled()); + } + + { + final String messagePersisterDisabledYaml = REQUIRED_CONFIG.concat(""" + messagePersister: + persistenceEnabled: false + """); + + final DynamicConfiguration config = + DynamicConfigurationManager.parseConfiguration(messagePersisterDisabledYaml, DynamicConfiguration.class) + .orElseThrow(); + + assertFalse(config.getMessagePersisterConfiguration().isPersistenceEnabled()); + } + } + } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/DynamicConfigurationManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/DynamicConfigurationManagerTest.java index 9ddf6c0d4..d0b35743d 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/DynamicConfigurationManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/DynamicConfigurationManagerTest.java @@ -120,8 +120,9 @@ class DynamicConfigurationManagerTest { configurationToken("1").build())) .thenReturn(GetLatestConfigurationResponse.builder() .configuration(SdkBytes.fromUtf8String(""" - featureFlags: - - testFlag + experiments: + test: + enrollmentPercentage: 50 captcha: scoreFloor: 1.0 """)) @@ -139,10 +140,12 @@ class DynamicConfigurationManagerTest { assertTimeoutPreemptively(Duration.ofSeconds(5), () -> { // we should eventually get the updated config (or the test will timeout) dynamicConfigurationManager.start(); - while (dynamicConfigurationManager.getConfiguration().getActiveFeatureFlags().isEmpty()) { + while (dynamicConfigurationManager.getConfiguration().getExperimentEnrollmentConfiguration("test").isEmpty()) { Thread.sleep(100); } - assertThat(dynamicConfigurationManager.getConfiguration().getActiveFeatureFlags()).containsExactly("testFlag"); + assertThat( + dynamicConfigurationManager.getConfiguration().getExperimentEnrollmentConfiguration("test").get() + .getEnrollmentPercentage()).isEqualTo(50); }); }