From 9e3d1c0651da4af470863fd89d6e3281cf1e98ba Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Tue, 7 Nov 2023 13:48:14 +0100 Subject: [PATCH 01/15] feat: Obey HTTP Status codes from upstream. Previously we have treated any status code as equal. This PR changes that to allow 429 and 50x status codes to incrementally increase the interval between each time we call. And then gradually decrease once it starts succeeding. I'm a bit worried about the metrics here, because we drop the metric bucket on the floor if we don't get a 20x back from the server. --- .../metric/DefaultHttpMetricsSender.java | 14 +- .../io/getunleash/metric/MetricSender.java | 4 +- .../metric/OkHttpMetricsSender.java | 9 +- .../metric/UnleashMetricServiceImpl.java | 59 +- .../repository/FeatureRepository.java | 95 +++- .../repository/FeatureToggleResponse.java | 2 +- .../io/getunleash/variant/VariantUtil.java | 1 + .../metric/UnleashMetricServiceImplTest.java | 305 ++++++++-- .../repository/FeatureRepositoryTest.java | 534 ++++++++++++++---- .../getunleash/variant/VariantUtilTest.java | 3 + src/test/resources/logback-test.xml | 1 + 11 files changed, 825 insertions(+), 202 deletions(-) diff --git a/src/main/java/io/getunleash/metric/DefaultHttpMetricsSender.java b/src/main/java/io/getunleash/metric/DefaultHttpMetricsSender.java index 8f6ecfcb6..a69b5c389 100644 --- a/src/main/java/io/getunleash/metric/DefaultHttpMetricsSender.java +++ b/src/main/java/io/getunleash/metric/DefaultHttpMetricsSender.java @@ -37,26 +37,32 @@ public DefaultHttpMetricsSender(UnleashConfig unleashConfig) { .create(); } - public void registerClient(ClientRegistration registration) { + public int registerClient(ClientRegistration registration) { if (!unleashConfig.isDisableMetrics()) { try { - post(clientRegistrationURL, registration); + int statusCode = post(clientRegistrationURL, registration); eventDispatcher.dispatch(registration); + return statusCode; } catch (UnleashException ex) { eventDispatcher.dispatch(ex); + return -1; } } + return -1; } - public void sendMetrics(ClientMetrics metrics) { + public int sendMetrics(ClientMetrics metrics) { if (!unleashConfig.isDisableMetrics()) { try { - post(clientMetricsURL, metrics); + int statusCode = post(clientMetricsURL, metrics); eventDispatcher.dispatch(metrics); + return statusCode; } catch (UnleashException ex) { eventDispatcher.dispatch(ex); + return -1; } } + return -1; } private int post(URL url, Object o) throws UnleashException { diff --git a/src/main/java/io/getunleash/metric/MetricSender.java b/src/main/java/io/getunleash/metric/MetricSender.java index 221f2de85..90cf9a47e 100644 --- a/src/main/java/io/getunleash/metric/MetricSender.java +++ b/src/main/java/io/getunleash/metric/MetricSender.java @@ -1,7 +1,7 @@ package io.getunleash.metric; public interface MetricSender { - void registerClient(ClientRegistration registration); + int registerClient(ClientRegistration registration); - void sendMetrics(ClientMetrics metrics); + int sendMetrics(ClientMetrics metrics); } diff --git a/src/main/java/io/getunleash/metric/OkHttpMetricsSender.java b/src/main/java/io/getunleash/metric/OkHttpMetricsSender.java index b637ff2d4..ba4bd3399 100644 --- a/src/main/java/io/getunleash/metric/OkHttpMetricsSender.java +++ b/src/main/java/io/getunleash/metric/OkHttpMetricsSender.java @@ -60,19 +60,21 @@ public OkHttpMetricsSender(UnleashConfig config) { } @Override - public void registerClient(ClientRegistration registration) { + public int registerClient(ClientRegistration registration) { if (!config.isDisableMetrics()) { try { - post(clientRegistrationUrl, registration); + int statusCode = post(clientRegistrationUrl, registration); eventDispatcher.dispatch(registration); + return statusCode; } catch (UnleashException ex) { eventDispatcher.dispatch(ex); } } + return -1; } @Override - public void sendMetrics(ClientMetrics metrics) { + public int sendMetrics(ClientMetrics metrics) { if (!config.isDisableMetrics()) { try { post(clientMetricsUrl, metrics); @@ -81,6 +83,7 @@ public void sendMetrics(ClientMetrics metrics) { eventDispatcher.dispatch(ex); } } + return -1; } private int post(HttpUrl url, Object o) { diff --git a/src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java b/src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java index 83d4ae230..c8e2d5f16 100644 --- a/src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java +++ b/src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java @@ -2,11 +2,17 @@ import io.getunleash.util.UnleashConfig; import io.getunleash.util.UnleashScheduledExecutor; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.net.HttpURLConnection; import java.time.LocalDateTime; import java.time.ZoneId; import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; public class UnleashMetricServiceImpl implements UnleashMetricService { + private static final Logger LOGGER = LoggerFactory.getLogger(UnleashMetricServiceImpl.class); private final LocalDateTime started; private final UnleashConfig unleashConfig; @@ -15,6 +21,10 @@ public class UnleashMetricServiceImpl implements UnleashMetricService { // mutable private volatile MetricsBucket currentMetricsBucket; + private final int maxInterval; + private AtomicInteger failures = new AtomicInteger(); + private AtomicInteger interval = new AtomicInteger(); + public UnleashMetricServiceImpl( UnleashConfig unleashConfig, UnleashScheduledExecutor executor) { this(unleashConfig, unleashConfig.getMetricSenderFactory().apply(unleashConfig), executor); @@ -28,6 +38,7 @@ public UnleashMetricServiceImpl( this.started = LocalDateTime.now(ZoneId.of("UTC")); this.unleashConfig = unleashConfig; this.metricSender = metricSender; + this.maxInterval = Integer.max(20, 300 / Integer.max(Long.valueOf(unleashConfig.getSendMetricsInterval()).intValue(), 1)); long metricsInterval = unleashConfig.getSendMetricsInterval(); executor.setInterval(sendMetrics(), metricsInterval, metricsInterval); } @@ -51,11 +62,49 @@ public void countVariant(String toggleName, String variantName) { private Runnable sendMetrics() { return () -> { - MetricsBucket metricsBucket = this.currentMetricsBucket; - this.currentMetricsBucket = new MetricsBucket(); - metricsBucket.end(); - ClientMetrics metrics = new ClientMetrics(unleashConfig, metricsBucket); - metricSender.sendMetrics(metrics); + if (interval.get() == 0) { + MetricsBucket metricsBucket = this.currentMetricsBucket; + this.currentMetricsBucket = new MetricsBucket(); + metricsBucket.end(); + ClientMetrics metrics = new ClientMetrics(unleashConfig, metricsBucket); + int statusCode = metricSender.sendMetrics(metrics); + if (statusCode >= 200 && statusCode < 400) { + if (failures.get() > 0) { + interval.set(Integer.max(failures.decrementAndGet(), 0)); + } + } + if (statusCode >= 400) { + handleHttpErrorCodes(statusCode); + } + } else { + interval.decrementAndGet(); + } }; } + + private void handleHttpErrorCodes(int responseCode) { + if (responseCode == 404) { + interval.set(maxInterval); + failures.incrementAndGet(); + LOGGER.error("Server said that the Metrics receiving endpoint at {} does not exist. Backing off to {} times our poll interval to avoid overloading server", unleashConfig.getUnleashURLs().getClientMetricsURL(), maxInterval); + } else if (responseCode == 429) { + interval.set(Math.min(failures.incrementAndGet(), maxInterval)); + LOGGER.info("Client Metrics was RATE LIMITED for the {}. time. Further backing off. Current backoff at {} times our metrics post interval", failures.get(), interval.get()); + } else if (responseCode == HttpURLConnection.HTTP_UNAUTHORIZED || responseCode == HttpURLConnection.HTTP_FORBIDDEN) { + failures.incrementAndGet(); + interval.set(maxInterval); + LOGGER.error("Client was not authorized to post metrics to the Unleash API at {}. Backing off to {} times our poll interval to avoid overloading server", unleashConfig.getUnleashURLs().getClientMetricsURL(), maxInterval); + } else if (responseCode >= 500) { + interval.set(Math.min(failures.incrementAndGet(), maxInterval)); + LOGGER.info("Server failed with a {} status code. Backing off. Current backoff at {} times our poll interval", responseCode, interval.get()); + } + } + + protected int getInterval() { + return this.interval.get(); + } + + protected int getFailures() { + return this.failures.get(); + } } diff --git a/src/main/java/io/getunleash/repository/FeatureRepository.java b/src/main/java/io/getunleash/repository/FeatureRepository.java index e826b948e..58da32b91 100644 --- a/src/main/java/io/getunleash/repository/FeatureRepository.java +++ b/src/main/java/io/getunleash/repository/FeatureRepository.java @@ -8,13 +8,19 @@ import io.getunleash.lang.Nullable; import io.getunleash.util.UnleashConfig; import io.getunleash.util.UnleashScheduledExecutor; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.net.HttpURLConnection; import java.util.Collections; import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; import java.util.stream.Collectors; public class FeatureRepository implements IFeatureRepository { - + private final Integer maxInterval; // Set to 20 times our polling interval, so with our default of 15 seconds, the longest interval will be 5 minutes. + private static final Logger LOGGER = LoggerFactory.getLogger(FeatureRepository.class); private final UnleashConfig unleashConfig; private final BackupHandler featureBackupHandler; private final FeatureBootstrapHandler featureBootstrapHandler; @@ -24,48 +30,54 @@ public class FeatureRepository implements IFeatureRepository { private FeatureCollection featureCollection; private boolean ready; + private AtomicInteger failures = new AtomicInteger(0); + private AtomicInteger interval = new AtomicInteger(0); + public FeatureRepository(UnleashConfig unleashConfig) { this(unleashConfig, new FeatureBackupHandlerFile(unleashConfig)); } public FeatureRepository( - UnleashConfig unleashConfig, - final BackupHandler featureBackupHandler) { + UnleashConfig unleashConfig, + final BackupHandler featureBackupHandler) { this.unleashConfig = unleashConfig; this.featureBackupHandler = featureBackupHandler; this.featureFetcher = unleashConfig.getUnleashFeatureFetcherFactory().apply(unleashConfig); this.featureBootstrapHandler = new FeatureBootstrapHandler(unleashConfig); this.eventDispatcher = new EventDispatcher(unleashConfig); + this.maxInterval = Integer.max(20, 300 / Integer.max(Long.valueOf(unleashConfig.getFetchTogglesInterval()).intValue(), 1)); this.initCollections(unleashConfig.getScheduledExecutor()); } protected FeatureRepository( - UnleashConfig unleashConfig, - BackupHandler featureBackupHandler, - EventDispatcher eventDispatcher, - FeatureFetcher featureFetcher, - FeatureBootstrapHandler featureBootstrapHandler) { + UnleashConfig unleashConfig, + BackupHandler featureBackupHandler, + EventDispatcher eventDispatcher, + FeatureFetcher featureFetcher, + FeatureBootstrapHandler featureBootstrapHandler) { this.unleashConfig = unleashConfig; this.featureBackupHandler = featureBackupHandler; this.featureFetcher = featureFetcher; this.featureBootstrapHandler = featureBootstrapHandler; this.eventDispatcher = eventDispatcher; + this.maxInterval = Integer.max(20, 300 / Integer.max(Long.valueOf(unleashConfig.getFetchTogglesInterval()).intValue(), 1)); this.initCollections(unleashConfig.getScheduledExecutor()); } protected FeatureRepository( - UnleashConfig unleashConfig, - FeatureBackupHandlerFile featureBackupHandler, - UnleashScheduledExecutor executor, - FeatureFetcher featureFetcher, - FeatureBootstrapHandler featureBootstrapHandler) { + UnleashConfig unleashConfig, + FeatureBackupHandlerFile featureBackupHandler, + UnleashScheduledExecutor executor, + FeatureFetcher featureFetcher, + FeatureBootstrapHandler featureBootstrapHandler) { this.unleashConfig = unleashConfig; this.featureBackupHandler = featureBackupHandler; this.featureFetcher = featureFetcher; this.featureBootstrapHandler = featureBootstrapHandler; this.eventDispatcher = new EventDispatcher(unleashConfig); + this.maxInterval = Integer.max(20, 300 / Integer.max(Long.valueOf(unleashConfig.getFetchTogglesInterval()).intValue(), 1)); this.initCollections(executor); } @@ -90,35 +102,66 @@ private void initCollections(UnleashScheduledExecutor executor) { } } + // private Runnable updateFeatures(@Nullable final Consumer handler) { - return () -> { + return () -> updateFeaturesInternal(handler); + } + + private void updateFeaturesInternal(@Nullable final Consumer handler) { + LOGGER.info("Interval: {}. Failures: {}", interval.get(), failures.get()); + if (interval.get() <= 0L) { try { ClientFeaturesResponse response = featureFetcher.fetchFeatures(); eventDispatcher.dispatch(response); if (response.getStatus() == ClientFeaturesResponse.Status.CHANGED) { SegmentCollection segmentCollection = response.getSegmentCollection(); featureCollection = - new FeatureCollection( - response.getToggleCollection(), - segmentCollection != null - ? segmentCollection - : new SegmentCollection(Collections.emptyList())); + new FeatureCollection( + response.getToggleCollection(), + segmentCollection != null + ? segmentCollection + : new SegmentCollection(Collections.emptyList())); featureBackupHandler.write(featureCollection); + } else if (response.getStatus() == ClientFeaturesResponse.Status.UNAVAILABLE) { + handleHttpErrorCodes(response.getHttpStatusCode()); + return; } + interval.set(Math.max(failures.decrementAndGet(),0)); if (!ready) { eventDispatcher.dispatch(new UnleashReady()); ready = true; } } catch (UnleashException e) { + interval.set(Math.min(failures.incrementAndGet(), maxInterval)); if (handler != null) { handler.accept(e); } else { throw e; } } - }; + } else { + interval.decrementAndGet(); + } + } + + private void handleHttpErrorCodes(int responseCode) { + if (responseCode == 404) { + interval.set(maxInterval); + failures.incrementAndGet(); + LOGGER.error("Server said that the API at {} does not exist. Backing off to {} times our poll interval to avoid overloading server", unleashConfig.getUnleashAPI(), maxInterval); + } else if (responseCode == 429) { + interval.set(Math.min(failures.incrementAndGet(), maxInterval)); + LOGGER.info("Client was RATE LIMITED for the {} time. Further backing off. Current backoff at {} times our poll interval", failures.get(), interval.get()); + } else if (responseCode == HttpURLConnection.HTTP_UNAUTHORIZED || responseCode == HttpURLConnection.HTTP_FORBIDDEN) { + failures.incrementAndGet(); + interval.set(maxInterval); + LOGGER.error("Client failed to authenticate to the Unleash API at {}. Backing off to {} times our poll interval to avoid overloading server", unleashConfig.getUnleashAPI(), maxInterval); + } else if (responseCode >= 500) { + interval.set(Math.min(failures.incrementAndGet(), maxInterval)); + LOGGER.info("Server failed with a {} status code. Backing off. Current backoff at {} times our poll interval", responseCode, interval.get()); + } } @Override @@ -129,12 +172,20 @@ private Runnable updateFeatures(@Nullable final Consumer handl @Override public List getFeatureNames() { return featureCollection.getToggleCollection().getFeatures().stream() - .map(FeatureToggle::getName) - .collect(Collectors.toList()); + .map(FeatureToggle::getName) + .collect(Collectors.toList()); } @Override public Segment getSegment(Integer id) { return featureCollection.getSegmentCollection().getSegment(id); } + + public Integer getFailures() { + return failures.get(); + } + + public Integer getInterval() { + return interval.get(); + } } diff --git a/src/main/java/io/getunleash/repository/FeatureToggleResponse.java b/src/main/java/io/getunleash/repository/FeatureToggleResponse.java index 73bbc9b83..b1c255653 100644 --- a/src/main/java/io/getunleash/repository/FeatureToggleResponse.java +++ b/src/main/java/io/getunleash/repository/FeatureToggleResponse.java @@ -13,7 +13,7 @@ public class FeatureToggleResponse implements UnleashEvent { public enum Status { NOT_CHANGED, CHANGED, - UNAVAILABLE + UNAVAILABLE, } private final Status status; diff --git a/src/main/java/io/getunleash/variant/VariantUtil.java b/src/main/java/io/getunleash/variant/VariantUtil.java index 0a0fd5655..f2c71493b 100644 --- a/src/main/java/io/getunleash/variant/VariantUtil.java +++ b/src/main/java/io/getunleash/variant/VariantUtil.java @@ -133,4 +133,5 @@ public static Variant selectVariant( } return null; } + } diff --git a/src/test/java/io/getunleash/metric/UnleashMetricServiceImplTest.java b/src/test/java/io/getunleash/metric/UnleashMetricServiceImplTest.java index 07939f454..a82940321 100644 --- a/src/test/java/io/getunleash/metric/UnleashMetricServiceImplTest.java +++ b/src/test/java/io/getunleash/metric/UnleashMetricServiceImplTest.java @@ -3,10 +3,14 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.*; +import io.getunleash.repository.ClientFeaturesResponse; +import io.getunleash.repository.FeatureToggleResponse; import io.getunleash.util.UnleashConfig; import io.getunleash.util.UnleashScheduledExecutor; + import java.util.HashSet; import java.util.Set; + import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; @@ -16,11 +20,11 @@ public class UnleashMetricServiceImplTest { public void should_register_future_for_sending_interval_regualry() { long interval = 10; UnleashConfig config = - UnleashConfig.builder() - .appName("test") - .sendMetricsInterval(interval) - .unleashAPI("http://unleash.com") - .build(); + UnleashConfig.builder() + .appName("test") + .sendMetricsInterval(interval) + .unleashAPI("http://unleash.com") + .build(); UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); UnleashMetricService unleashMetricService = new UnleashMetricServiceImpl(config, executor); @@ -31,24 +35,24 @@ public void should_register_future_for_sending_interval_regualry() { public void should_register_client() { long interval = 10; UnleashConfig config = - UnleashConfig.builder() - .appName("test") - .sendMetricsInterval(interval) - .unleashAPI("http://unleash.com") - .build(); + UnleashConfig.builder() + .appName("test") + .sendMetricsInterval(interval) + .unleashAPI("http://unleash.com") + .build(); UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); DefaultHttpMetricsSender sender = mock(DefaultHttpMetricsSender.class); UnleashMetricService unleashMetricService = - new UnleashMetricServiceImpl(config, sender, executor); + new UnleashMetricServiceImpl(config, sender, executor); Set strategies = new HashSet<>(); strategies.add("default"); strategies.add("custom"); unleashMetricService.register(strategies); ArgumentCaptor argument = - ArgumentCaptor.forClass(ClientRegistration.class); + ArgumentCaptor.forClass(ClientRegistration.class); verify(sender).registerClient(argument.capture()); assertThat(argument.getValue().getAppName()).isEqualTo(config.getAppName()); @@ -62,25 +66,25 @@ public void should_register_client() { public void should_register_client_with_env() { long interval = 10; UnleashConfig config = - UnleashConfig.builder() - .appName("test") - .environment("dev") - .sendMetricsInterval(interval) - .unleashAPI("http://unleash.com") - .build(); + UnleashConfig.builder() + .appName("test") + .environment("dev") + .sendMetricsInterval(interval) + .unleashAPI("http://unleash.com") + .build(); UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); DefaultHttpMetricsSender sender = mock(DefaultHttpMetricsSender.class); UnleashMetricService unleashMetricService = - new UnleashMetricServiceImpl(config, sender, executor); + new UnleashMetricServiceImpl(config, sender, executor); Set strategies = new HashSet<>(); strategies.add("default"); strategies.add("custom"); unleashMetricService.register(strategies); ArgumentCaptor argument = - ArgumentCaptor.forClass(ClientRegistration.class); + ArgumentCaptor.forClass(ClientRegistration.class); verify(sender).registerClient(argument.capture()); assertThat(argument.getValue().getEnvironment()).isEqualTo(config.getEnvironment()); @@ -89,17 +93,17 @@ public void should_register_client_with_env() { @Test public void should_send_metrics() { UnleashConfig config = - UnleashConfig.builder() - .appName("test") - .sendMetricsInterval(10) - .unleashAPI("http://unleash.com") - .build(); + UnleashConfig.builder() + .appName("test") + .sendMetricsInterval(10) + .unleashAPI("http://unleash.com") + .build(); UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); DefaultHttpMetricsSender sender = mock(DefaultHttpMetricsSender.class); UnleashMetricService unleashMetricService = - new UnleashMetricServiceImpl(config, sender, executor); + new UnleashMetricServiceImpl(config, sender, executor); ArgumentCaptor sendMetricsCallback = ArgumentCaptor.forClass(Runnable.class); verify(executor).setInterval(sendMetricsCallback.capture(), anyLong(), anyLong()); @@ -111,18 +115,18 @@ public void should_send_metrics() { @Test public void should_record_and_send_metrics() { UnleashConfig config = - UnleashConfig.builder() - .appName("test") - .environment("prod") - .sendMetricsInterval(10) - .unleashAPI("http://unleash.com") - .build(); + UnleashConfig.builder() + .appName("test") + .environment("prod") + .sendMetricsInterval(10) + .unleashAPI("http://unleash.com") + .build(); UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); DefaultHttpMetricsSender sender = mock(DefaultHttpMetricsSender.class); UnleashMetricService unleashMetricService = - new UnleashMetricServiceImpl(config, sender, executor); + new UnleashMetricServiceImpl(config, sender, executor); unleashMetricService.count("someToggle", true); unleashMetricService.count("someToggle", false); unleashMetricService.count("someToggle", true); @@ -134,7 +138,7 @@ public void should_record_and_send_metrics() { sendMetricsCallback.getValue().run(); ArgumentCaptor clientMetricsArgumentCaptor = - ArgumentCaptor.forClass(ClientMetrics.class); + ArgumentCaptor.forClass(ClientMetrics.class); verify(sender).sendMetrics(clientMetricsArgumentCaptor.capture()); ClientMetrics clientMetrics = clientMetricsArgumentCaptor.getValue(); @@ -153,17 +157,17 @@ public void should_record_and_send_metrics() { @Test public void should_record_and_send_variant_metrics() { UnleashConfig config = - UnleashConfig.builder() - .appName("test") - .sendMetricsInterval(10) - .unleashAPI("http://unleash.com") - .build(); + UnleashConfig.builder() + .appName("test") + .sendMetricsInterval(10) + .unleashAPI("http://unleash.com") + .build(); UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); DefaultHttpMetricsSender sender = mock(DefaultHttpMetricsSender.class); UnleashMetricService unleashMetricService = - new UnleashMetricServiceImpl(config, sender, executor); + new UnleashMetricServiceImpl(config, sender, executor); unleashMetricService.countVariant("someToggle", "v1"); unleashMetricService.countVariant("someToggle", "v1"); unleashMetricService.countVariant("someToggle", "v1"); @@ -176,7 +180,7 @@ public void should_record_and_send_variant_metrics() { sendMetricsCallback.getValue().run(); ArgumentCaptor clientMetricsArgumentCaptor = - ArgumentCaptor.forClass(ClientMetrics.class); + ArgumentCaptor.forClass(ClientMetrics.class); verify(sender).sendMetrics(clientMetricsArgumentCaptor.capture()); ClientMetrics clientMetrics = clientMetricsArgumentCaptor.getValue(); @@ -188,12 +192,225 @@ public void should_record_and_send_variant_metrics() { assertThat(bucket.getStop()).isNotNull(); assertThat(bucket.getToggles()).hasSize(1); assertThat(bucket.getToggles().get("someToggle").getVariants().get("v1").longValue()) - .isEqualTo(3l); + .isEqualTo(3l); assertThat(bucket.getToggles().get("someToggle").getVariants().get("v2").longValue()) - .isEqualTo(1l); + .isEqualTo(1l); assertThat(bucket.getToggles().get("someToggle").getVariants().get("disabled").longValue()) - .isEqualTo(1l); + .isEqualTo(1l); assertThat(bucket.getToggles().get("someToggle").getYes()).isEqualTo(0l); assertThat(bucket.getToggles().get("someToggle").getNo()).isEqualTo(0l); } + + @Test + public void should_backoff_when_told_to_by_429_code() { + UnleashConfig config = + UnleashConfig.builder() + .appName("test") + .sendMetricsInterval(10) + .unleashAPI("http://unleash.com") + .build(); + + UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); + DefaultHttpMetricsSender sender = mock(DefaultHttpMetricsSender.class); + + UnleashMetricServiceImpl unleashMetricService = + new UnleashMetricServiceImpl(config, sender, executor); + unleashMetricService.countVariant("someToggle", "v1"); + unleashMetricService.countVariant("someToggle", "v1"); + unleashMetricService.countVariant("someToggle", "v1"); + unleashMetricService.countVariant("someToggle", "v2"); + unleashMetricService.countVariant("someToggle", "disabled"); + + // Call the sendMetricsCallback + ArgumentCaptor sendMetricsCallback = ArgumentCaptor.forClass(Runnable.class); + verify(executor).setInterval(sendMetricsCallback.capture(), anyLong(), anyLong()); + + when(sender.sendMetrics(any(ClientMetrics.class))).thenReturn(429).thenReturn(429).thenReturn(429).thenReturn(200).thenReturn(200).thenReturn(200).thenReturn(200); + + sendMetricsCallback.getValue().run(); + assertThat(unleashMetricService.getInterval()).isEqualTo(1); + assertThat(unleashMetricService.getFailures()).isEqualTo(1); + sendMetricsCallback.getValue().run(); + assertThat(unleashMetricService.getInterval()).isEqualTo(0); + assertThat(unleashMetricService.getFailures()).isEqualTo(1); + sendMetricsCallback.getValue().run(); + assertThat(unleashMetricService.getInterval()).isEqualTo(2); + assertThat(unleashMetricService.getFailures()).isEqualTo(2); + sendMetricsCallback.getValue().run(); // NO-OP because interval > 0 + sendMetricsCallback.getValue().run(); // NO-OP because interval > 0 + assertThat(unleashMetricService.getInterval()).isEqualTo(0); + assertThat(unleashMetricService.getFailures()).isEqualTo(2); + sendMetricsCallback.getValue().run(); + assertThat(unleashMetricService.getInterval()).isEqualTo(3); + assertThat(unleashMetricService.getFailures()).isEqualTo(3); + sendMetricsCallback.getValue().run(); + sendMetricsCallback.getValue().run(); + sendMetricsCallback.getValue().run(); + assertThat(unleashMetricService.getInterval()).isEqualTo(0); + assertThat(unleashMetricService.getFailures()).isEqualTo(3); + sendMetricsCallback.getValue().run(); + assertThat(unleashMetricService.getInterval()).isEqualTo(2); + assertThat(unleashMetricService.getFailures()).isEqualTo(2); + sendMetricsCallback.getValue().run(); + sendMetricsCallback.getValue().run(); + assertThat(unleashMetricService.getInterval()).isEqualTo(0); + assertThat(unleashMetricService.getFailures()).isEqualTo(2); + sendMetricsCallback.getValue().run(); + assertThat(unleashMetricService.getInterval()).isEqualTo(1); + assertThat(unleashMetricService.getFailures()).isEqualTo(1); + sendMetricsCallback.getValue().run(); + assertThat(unleashMetricService.getInterval()).isEqualTo(0); + assertThat(unleashMetricService.getFailures()).isEqualTo(1); + sendMetricsCallback.getValue().run(); + assertThat(unleashMetricService.getInterval()).isEqualTo(0); + assertThat(unleashMetricService.getFailures()).isEqualTo(0); + } + + @Test + public void server_errors_should_also_incrementally_backoff() { + UnleashConfig config = + UnleashConfig.builder() + .appName("test") + .sendMetricsInterval(10) + .unleashAPI("http://unleash.com") + .build(); + + UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); + DefaultHttpMetricsSender sender = mock(DefaultHttpMetricsSender.class); + + UnleashMetricServiceImpl unleashMetricService = + new UnleashMetricServiceImpl(config, sender, executor); + unleashMetricService.countVariant("someToggle", "v1"); + unleashMetricService.countVariant("someToggle", "v1"); + unleashMetricService.countVariant("someToggle", "v1"); + unleashMetricService.countVariant("someToggle", "v2"); + unleashMetricService.countVariant("someToggle", "disabled"); + + // Call the sendMetricsCallback + ArgumentCaptor sendMetricsCallback = ArgumentCaptor.forClass(Runnable.class); + verify(executor).setInterval(sendMetricsCallback.capture(), anyLong(), anyLong()); + when(sender.sendMetrics(any(ClientMetrics.class))) + .thenReturn(500) + .thenReturn(502) + .thenReturn(503) + .thenReturn(304) + .thenReturn(304) + .thenReturn(304) + .thenReturn(304); + sendMetricsCallback.getValue().run(); + assertThat(unleashMetricService.getInterval()).isEqualTo(1); + assertThat(unleashMetricService.getFailures()).isEqualTo(1); + sendMetricsCallback.getValue().run(); + assertThat(unleashMetricService.getInterval()).isEqualTo(0); + assertThat(unleashMetricService.getFailures()).isEqualTo(1); + sendMetricsCallback.getValue().run(); + assertThat(unleashMetricService.getInterval()).isEqualTo(2); + assertThat(unleashMetricService.getFailures()).isEqualTo(2); + sendMetricsCallback.getValue().run(); // NO-OP because interval > 0 + sendMetricsCallback.getValue().run(); // NO-OP because interval > 0 + assertThat(unleashMetricService.getInterval()).isEqualTo(0); + assertThat(unleashMetricService.getFailures()).isEqualTo(2); + sendMetricsCallback.getValue().run(); + assertThat(unleashMetricService.getInterval()).isEqualTo(3); + assertThat(unleashMetricService.getFailures()).isEqualTo(3); + sendMetricsCallback.getValue().run(); + sendMetricsCallback.getValue().run(); + sendMetricsCallback.getValue().run(); + assertThat(unleashMetricService.getInterval()).isEqualTo(0); + assertThat(unleashMetricService.getFailures()).isEqualTo(3); + sendMetricsCallback.getValue().run(); + assertThat(unleashMetricService.getInterval()).isEqualTo(2); + assertThat(unleashMetricService.getFailures()).isEqualTo(2); + sendMetricsCallback.getValue().run(); + sendMetricsCallback.getValue().run(); + assertThat(unleashMetricService.getInterval()).isEqualTo(0); + assertThat(unleashMetricService.getFailures()).isEqualTo(2); + sendMetricsCallback.getValue().run(); + assertThat(unleashMetricService.getInterval()).isEqualTo(1); + assertThat(unleashMetricService.getFailures()).isEqualTo(1); + sendMetricsCallback.getValue().run(); + assertThat(unleashMetricService.getInterval()).isEqualTo(0); + assertThat(unleashMetricService.getFailures()).isEqualTo(1); + sendMetricsCallback.getValue().run(); + assertThat(unleashMetricService.getInterval()).isEqualTo(0); + assertThat(unleashMetricService.getFailures()).isEqualTo(0); + } + + @Test + public void failure_to_authenticate_immediately_increases_interval_to_max() { + UnleashConfig config = + UnleashConfig.builder() + .appName("test") + .sendMetricsInterval(10) + .unleashAPI("http://unleash.com") + .build(); + + UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); + DefaultHttpMetricsSender sender = mock(DefaultHttpMetricsSender.class); + + UnleashMetricServiceImpl unleashMetricService = + new UnleashMetricServiceImpl(config, sender, executor); + unleashMetricService.countVariant("someToggle", "v1"); + unleashMetricService.countVariant("someToggle", "v1"); + unleashMetricService.countVariant("someToggle", "v1"); + unleashMetricService.countVariant("someToggle", "v2"); + unleashMetricService.countVariant("someToggle", "disabled"); + + // Call the sendMetricsCallback + ArgumentCaptor sendMetricsCallback = ArgumentCaptor.forClass(Runnable.class); + verify(executor).setInterval(sendMetricsCallback.capture(), anyLong(), anyLong()); + when(sender.sendMetrics(any(ClientMetrics.class))) + .thenReturn(403) + .thenReturn(200); + sendMetricsCallback.getValue().run(); + assertThat(unleashMetricService.getInterval()).isEqualTo(30); + assertThat(unleashMetricService.getFailures()).isEqualTo(1); + for (int i = 0; i < 30; i++) { + sendMetricsCallback.getValue().run(); + } + assertThat(unleashMetricService.getFailures()).isEqualTo(1); + assertThat(unleashMetricService.getInterval()).isEqualTo(0); + sendMetricsCallback.getValue().run(); + assertThat(unleashMetricService.getFailures()).isEqualTo(0); + assertThat(unleashMetricService.getInterval()).isEqualTo(0); + } + + @Test + public void url_not_found_immediately_increases_interval_to_max() { + UnleashConfig config = + UnleashConfig.builder() + .appName("test") + .sendMetricsInterval(10) + .unleashAPI("http://unleash.com") + .build(); + + UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); + DefaultHttpMetricsSender sender = mock(DefaultHttpMetricsSender.class); + + UnleashMetricServiceImpl unleashMetricService = + new UnleashMetricServiceImpl(config, sender, executor); + unleashMetricService.countVariant("someToggle", "v1"); + unleashMetricService.countVariant("someToggle", "v1"); + unleashMetricService.countVariant("someToggle", "v1"); + unleashMetricService.countVariant("someToggle", "v2"); + unleashMetricService.countVariant("someToggle", "disabled"); + + // Call the sendMetricsCallback + ArgumentCaptor sendMetricsCallback = ArgumentCaptor.forClass(Runnable.class); + verify(executor).setInterval(sendMetricsCallback.capture(), anyLong(), anyLong()); + when(sender.sendMetrics(any(ClientMetrics.class))) + .thenReturn(404) + .thenReturn(200); + sendMetricsCallback.getValue().run(); + assertThat(unleashMetricService.getInterval()).isEqualTo(30); + assertThat(unleashMetricService.getFailures()).isEqualTo(1); + for (int i = 0; i < 30; i++) { + sendMetricsCallback.getValue().run(); + } + assertThat(unleashMetricService.getFailures()).isEqualTo(1); + assertThat(unleashMetricService.getInterval()).isEqualTo(0); + sendMetricsCallback.getValue().run(); + assertThat(unleashMetricService.getFailures()).isEqualTo(0); + assertThat(unleashMetricService.getInterval()).isEqualTo(0); + } } diff --git a/src/test/java/io/getunleash/repository/FeatureRepositoryTest.java b/src/test/java/io/getunleash/repository/FeatureRepositoryTest.java index 818cfb86b..0eb5a4cc9 100644 --- a/src/test/java/io/getunleash/repository/FeatureRepositoryTest.java +++ b/src/test/java/io/getunleash/repository/FeatureRepositoryTest.java @@ -1,5 +1,7 @@ package io.getunleash.repository; +import static java.util.stream.Collectors.toCollection; +import static java.util.stream.Collectors.toList; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.*; @@ -9,21 +11,29 @@ import io.getunleash.lang.Nullable; import io.getunleash.util.UnleashConfig; import io.getunleash.util.UnleashScheduledExecutor; + import java.io.File; import java.io.IOException; import java.net.URISyntaxException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.time.Clock; +import java.time.Duration; +import java.time.Instant; +import java.time.temporal.ChronoUnit; +import java.time.temporal.TemporalUnit; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.concurrent.*; + +import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; public class FeatureRepositoryTest { - FeatureBackupHandlerFile backupHandler; FeatureBootstrapHandler bootstrapHandler; @@ -38,24 +48,24 @@ public void setUp() { fetcher = mock(HttpFeatureFetcher.class); defaultConfig = - new UnleashConfig.Builder() - .appName("test") - .unleashAPI("http://localhost:4242/api/") - .scheduledExecutor(mock(UnleashScheduledExecutor.class)) - .fetchTogglesInterval(200L) - .synchronousFetchOnInitialisation(false) - .build(); + new UnleashConfig.Builder() + .appName("test") + .unleashAPI("http://localhost:4242/api/") + .scheduledExecutor(mock(UnleashScheduledExecutor.class)) + .fetchTogglesInterval(200L) + .synchronousFetchOnInitialisation(false) + .build(); } @Test public void no_backup_file_and_no_repository_available_should_give_empty_repo() { UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); UnleashConfig config = - UnleashConfig.builder() - .appName("test") - .unleashAPI("http://localhost:4242/api/") - .scheduledExecutor(executor) - .build(); + UnleashConfig.builder() + .appName("test") + .unleashAPI("http://localhost:4242/api/") + .scheduledExecutor(executor) + .build(); when(backupHandler.read()).thenReturn(new FeatureCollection()); when(bootstrapHandler.read()).thenReturn(new FeatureCollection()); @@ -67,21 +77,21 @@ public void no_backup_file_and_no_repository_available_should_give_empty_repo() public void backup_toggles_should_be_loaded_at_startup() { UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); UnleashConfig config = - UnleashConfig.builder() - .appName("test") - .scheduledExecutor(executor) - .unleashAPI("http://localhost:4242/api/") - .fetchTogglesInterval(Long.MAX_VALUE) - .build(); + UnleashConfig.builder() + .appName("test") + .scheduledExecutor(executor) + .unleashAPI("http://localhost:4242/api/") + .fetchTogglesInterval(Long.MAX_VALUE) + .build(); when(backupHandler.read()) - .thenReturn( - new FeatureCollection( - new ToggleCollection(Collections.emptyList()), - new SegmentCollection(Collections.emptyList()))); + .thenReturn( + new FeatureCollection( + new ToggleCollection(Collections.emptyList()), + new SegmentCollection(Collections.emptyList()))); new FeatureRepository( - config, backupHandler, new EventDispatcher(config), fetcher, bootstrapHandler); + config, backupHandler, new EventDispatcher(config), fetcher, bootstrapHandler); verify(backupHandler, times(1)).read(); } @@ -92,37 +102,37 @@ public void feature_toggles_should_be_updated() { ArgumentCaptor runnableArgumentCaptor = ArgumentCaptor.forClass(Runnable.class); UnleashConfig config = - new UnleashConfig.Builder() - .appName("test") - .unleashAPI("http://localhost:4242/api/") - .scheduledExecutor(executor) - .fetchTogglesInterval(200L) - .synchronousFetchOnInitialisation(false) - .build(); + new UnleashConfig.Builder() + .appName("test") + .unleashAPI("http://localhost:4242/api/") + .scheduledExecutor(executor) + .fetchTogglesInterval(200L) + .synchronousFetchOnInitialisation(false) + .build(); FeatureCollection featureCollection = - populatedFeatureCollection( - null, - new FeatureToggle( - "toggleFetcherCalled", - false, - Arrays.asList(new ActivationStrategy("custom", null)))); + populatedFeatureCollection( + null, + new FeatureToggle( + "toggleFetcherCalled", + false, + Arrays.asList(new ActivationStrategy("custom", null)))); when(backupHandler.read()).thenReturn(featureCollection); featureCollection = - populatedFeatureCollection( - null, - new FeatureToggle( - "toggleFetcherCalled", - true, - Arrays.asList(new ActivationStrategy("custom", null)))); + populatedFeatureCollection( + null, + new FeatureToggle( + "toggleFetcherCalled", + true, + Arrays.asList(new ActivationStrategy("custom", null)))); ClientFeaturesResponse response = - new ClientFeaturesResponse( - ClientFeaturesResponse.Status.CHANGED, featureCollection); + new ClientFeaturesResponse( + ClientFeaturesResponse.Status.CHANGED, featureCollection); FeatureRepository featureRepository = - new FeatureRepository(config, backupHandler, executor, fetcher, bootstrapHandler); + new FeatureRepository(config, backupHandler, executor, fetcher, bootstrapHandler); // run the toggleName fetcher callback verify(executor).setInterval(runnableArgumentCaptor.capture(), anyLong(), anyLong()); verify(fetcher, times(0)).fetchFeatures(); @@ -138,25 +148,25 @@ public void feature_toggles_should_be_updated() { @Test public void get_feature_names_should_return_list_of_names() { FeatureCollection featureCollection = - populatedFeatureCollection( - null, - new FeatureToggle( - "toggleFeatureName1", - true, - Arrays.asList(new ActivationStrategy("custom", null))), - new FeatureToggle( - "toggleFeatureName2", - true, - Arrays.asList(new ActivationStrategy("custom", null)))); + populatedFeatureCollection( + null, + new FeatureToggle( + "toggleFeatureName1", + true, + Arrays.asList(new ActivationStrategy("custom", null))), + new FeatureToggle( + "toggleFeatureName2", + true, + Arrays.asList(new ActivationStrategy("custom", null)))); when(backupHandler.read()).thenReturn(featureCollection); FeatureRepository featureRepository = - new FeatureRepository( - defaultConfig, - backupHandler, - new EventDispatcher(defaultConfig), - fetcher, - bootstrapHandler); + new FeatureRepository( + defaultConfig, + backupHandler, + new EventDispatcher(defaultConfig), + fetcher, + bootstrapHandler); assertEquals(2, featureRepository.getFeatureNames().size()); assertEquals("toggleFeatureName2", featureRepository.getFeatureNames().get(1)); } @@ -165,23 +175,23 @@ public void get_feature_names_should_return_list_of_names() { public void should_perform_synchronous_fetch_on_initialisation() { UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); UnleashConfig config = - UnleashConfig.builder() - .synchronousFetchOnInitialisation(true) - .scheduledExecutor(executor) - .appName("test-sync-update") - .unleashAPI("http://localhost:8080") - .build(); + UnleashConfig.builder() + .synchronousFetchOnInitialisation(true) + .scheduledExecutor(executor) + .appName("test-sync-update") + .unleashAPI("http://localhost:8080") + .build(); when(backupHandler.read()).thenReturn(new FeatureCollection()); FeatureCollection featureCollection = populatedFeatureCollection(null); ClientFeaturesResponse response = - new ClientFeaturesResponse( - ClientFeaturesResponse.Status.CHANGED, featureCollection); + new ClientFeaturesResponse( + ClientFeaturesResponse.Status.CHANGED, featureCollection); when(fetcher.fetchFeatures()).thenReturn(response); new FeatureRepository( - config, backupHandler, new EventDispatcher(config), fetcher, bootstrapHandler); + config, backupHandler, new EventDispatcher(config), fetcher, bootstrapHandler); verify(fetcher, times(1)).fetchFeatures(); } @@ -189,19 +199,19 @@ public void should_perform_synchronous_fetch_on_initialisation() { public void should_not_perform_synchronous_fetch_on_initialisation() { UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); UnleashConfig config = - UnleashConfig.builder() - .synchronousFetchOnInitialisation(false) - .scheduledExecutor(executor) - .appName("test-sync-update") - .unleashAPI("http://localhost:8080") - .build(); + UnleashConfig.builder() + .synchronousFetchOnInitialisation(false) + .scheduledExecutor(executor) + .appName("test-sync-update") + .unleashAPI("http://localhost:8080") + .build(); when(backupHandler.read()).thenReturn(new FeatureCollection()); FeatureCollection featureCollection = populatedFeatureCollection(null); ClientFeaturesResponse response = - new ClientFeaturesResponse( - ClientFeaturesResponse.Status.CHANGED, featureCollection); + new ClientFeaturesResponse( + ClientFeaturesResponse.Status.CHANGED, featureCollection); when(fetcher.fetchFeatures()).thenReturn(response); @@ -211,7 +221,7 @@ public void should_not_perform_synchronous_fetch_on_initialisation() { } private FeatureCollection populatedFeatureCollection( - @Nullable List segments, FeatureToggle... featureToggles) { + @Nullable List segments, FeatureToggle... featureToggles) { List toggleList = new ArrayList<>(); toggleList.addAll(Arrays.asList(featureToggles)); @@ -219,27 +229,27 @@ private FeatureCollection populatedFeatureCollection( if (segments != null) segmentList.addAll(segments); return new FeatureCollection( - new ToggleCollection(toggleList), new SegmentCollection(segmentList)); + new ToggleCollection(toggleList), new SegmentCollection(segmentList)); } @Test public void should_read_from_bootstrap_location_if_backup_was_empty() - throws URISyntaxException, IOException { + throws URISyntaxException, IOException { File file = - new File(getClass().getClassLoader().getResource("unleash-repo-v2.json").toURI()); + new File(getClass().getClassLoader().getResource("unleash-repo-v2.json").toURI()); ToggleBootstrapProvider toggleBootstrapProvider = mock(ToggleBootstrapProvider.class); when(toggleBootstrapProvider.read()).thenReturn(fileToString(file)); UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); UnleashConfig config = - UnleashConfig.builder() - .synchronousFetchOnInitialisation(false) - .appName("test-sync-update") - .scheduledExecutor(executor) - .unleashAPI("http://localhost:8080") - .toggleBootstrapProvider(toggleBootstrapProvider) - .build(); + UnleashConfig.builder() + .synchronousFetchOnInitialisation(false) + .appName("test-sync-update") + .scheduledExecutor(executor) + .unleashAPI("http://localhost:8080") + .toggleBootstrapProvider(toggleBootstrapProvider) + .build(); when(backupHandler.read()).thenReturn(new FeatureCollection()); @@ -249,51 +259,333 @@ public void should_read_from_bootstrap_location_if_backup_was_empty() @Test public void should_not_read_bootstrap_if_backup_was_found() - throws IOException, URISyntaxException { + throws IOException, URISyntaxException { File file = - new File(getClass().getClassLoader().getResource("unleash-repo-v2.json").toURI()); + new File(getClass().getClassLoader().getResource("unleash-repo-v2.json").toURI()); ToggleBootstrapProvider toggleBootstrapProvider = mock(ToggleBootstrapProvider.class); UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); UnleashConfig config = - UnleashConfig.builder() - .synchronousFetchOnInitialisation(false) - .appName("test-sync-update") - .scheduledExecutor(executor) - .unleashAPI("http://localhost:8080") - .toggleBootstrapProvider(toggleBootstrapProvider) - .build(); + UnleashConfig.builder() + .synchronousFetchOnInitialisation(false) + .appName("test-sync-update") + .scheduledExecutor(executor) + .unleashAPI("http://localhost:8080") + .toggleBootstrapProvider(toggleBootstrapProvider) + .build(); when(toggleBootstrapProvider.read()).thenReturn(fileToString(file)); when(backupHandler.read()) - .thenReturn( - populatedFeatureCollection( - Arrays.asList( - new Segment( - 1, - "some-name", - Arrays.asList( - new Constraint( - "some-context", - Operator.IN, - "some-value")))), - new FeatureToggle( - "toggleFeatureName1", - true, - Collections.singletonList( - new ActivationStrategy("custom", null))), - new FeatureToggle( - "toggleFeatureName2", - true, - Collections.singletonList( - new ActivationStrategy("custom", null))))); + .thenReturn( + populatedFeatureCollection( + Arrays.asList( + new Segment( + 1, + "some-name", + Arrays.asList( + new Constraint( + "some-context", + Operator.IN, + "some-value")))), + new FeatureToggle( + "toggleFeatureName1", + true, + Collections.singletonList( + new ActivationStrategy("custom", null))), + new FeatureToggle( + "toggleFeatureName2", + true, + Collections.singletonList( + new ActivationStrategy("custom", null))))); new FeatureRepository( - config, backupHandler, new EventDispatcher(config), fetcher, bootstrapHandler); + config, backupHandler, new EventDispatcher(config), fetcher, bootstrapHandler); verify(toggleBootstrapProvider, times(0)).read(); } + @Test + public void should_increase_to_max_interval_when_denied() throws URISyntaxException, IOException { + UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); + ArgumentCaptor runnableArgumentCaptor = ArgumentCaptor.forClass(Runnable.class); + File file = + new File(getClass().getClassLoader().getResource("unleash-repo-v2.json").toURI()); + ToggleBootstrapProvider toggleBootstrapProvider = mock(ToggleBootstrapProvider.class); + when(toggleBootstrapProvider.read()).thenReturn(fileToString(file)); + UnleashConfig config = + UnleashConfig.builder() + .synchronousFetchOnInitialisation(false) + .appName("test-sync-update") + .scheduledExecutor(executor) + .unleashAPI("http://localhost:8080") + .build(); + when(backupHandler.read()) + .thenReturn( + populatedFeatureCollection( + Arrays.asList( + new Segment( + 1, + "some-name", + Arrays.asList( + new Constraint( + "some-context", + Operator.IN, + "some-value")))), + new FeatureToggle( + "toggleFeatureName1", + true, + Collections.singletonList( + new ActivationStrategy("custom", null))), + new FeatureToggle( + "toggleFeatureName2", + true, + Collections.singletonList( + new ActivationStrategy("custom", null))))); + when(fetcher.fetchFeatures()).thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.UNAVAILABLE, 403)) + .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)); + + FeatureRepository featureRepository = new FeatureRepository(config, backupHandler, new EventDispatcher(config), fetcher, bootstrapHandler); + verify(executor).setInterval(runnableArgumentCaptor.capture(), anyLong(), anyLong()); + runnableArgumentCaptor.getValue().run(); + assertThat(featureRepository.getFailures()).isEqualTo(1); + assertThat(featureRepository.getInterval()).isEqualTo(30); + for (int i = 0; i < 30; i++) { + runnableArgumentCaptor.getValue().run(); + } + assertThat(featureRepository.getFailures()).isEqualTo(1); + assertThat(featureRepository.getInterval()).isEqualTo(0); + runnableArgumentCaptor.getValue().run(); + assertThat(featureRepository.getFailures()).isEqualTo(0); + assertThat(featureRepository.getInterval()).isEqualTo(0); + } + + @Test + public void should_increase_to_max_interval_when_not_found() throws URISyntaxException, IOException { + UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); + ArgumentCaptor runnableArgumentCaptor = ArgumentCaptor.forClass(Runnable.class); + File file = + new File(getClass().getClassLoader().getResource("unleash-repo-v2.json").toURI()); + ToggleBootstrapProvider toggleBootstrapProvider = mock(ToggleBootstrapProvider.class); + when(toggleBootstrapProvider.read()).thenReturn(fileToString(file)); + UnleashConfig config = + UnleashConfig.builder() + .synchronousFetchOnInitialisation(false) + .appName("test-sync-update") + .scheduledExecutor(executor) + .unleashAPI("http://localhost:8080") + .build(); + when(backupHandler.read()) + .thenReturn( + populatedFeatureCollection( + Arrays.asList( + new Segment( + 1, + "some-name", + Arrays.asList( + new Constraint( + "some-context", + Operator.IN, + "some-value")))), + new FeatureToggle( + "toggleFeatureName1", + true, + Collections.singletonList( + new ActivationStrategy("custom", null))), + new FeatureToggle( + "toggleFeatureName2", + true, + Collections.singletonList( + new ActivationStrategy("custom", null))))); + when(fetcher.fetchFeatures()).thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.UNAVAILABLE, 404)) + .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)); + + FeatureRepository featureRepository = new FeatureRepository(config, backupHandler, new EventDispatcher(config), fetcher, bootstrapHandler); + verify(executor).setInterval(runnableArgumentCaptor.capture(), anyLong(), anyLong()); + runnableArgumentCaptor.getValue().run(); + assertThat(featureRepository.getFailures()).isEqualTo(1); + assertThat(featureRepository.getInterval()).isEqualTo(30); + for (int i = 0; i < 30; i++) { + runnableArgumentCaptor.getValue().run(); + } + assertThat(featureRepository.getFailures()).isEqualTo(1); + assertThat(featureRepository.getInterval()).isEqualTo(0); + runnableArgumentCaptor.getValue().run(); + assertThat(featureRepository.getFailures()).isEqualTo(0); + assertThat(featureRepository.getInterval()).isEqualTo(0); + } + @Test + public void should_incrementally_increase_interval_as_we_receive_too_many_requests() throws URISyntaxException, IOException { + UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); + ArgumentCaptor runnableArgumentCaptor = ArgumentCaptor.forClass(Runnable.class); + File file = + new File(getClass().getClassLoader().getResource("unleash-repo-v2.json").toURI()); + ToggleBootstrapProvider toggleBootstrapProvider = mock(ToggleBootstrapProvider.class); + when(toggleBootstrapProvider.read()).thenReturn(fileToString(file)); + UnleashConfig config = + UnleashConfig.builder() + .synchronousFetchOnInitialisation(false) + .appName("test-sync-update") + .scheduledExecutor(executor) + .unleashAPI("http://localhost:8080") + .build(); + when(backupHandler.read()) + .thenReturn( + populatedFeatureCollection( + Arrays.asList( + new Segment( + 1, + "some-name", + Arrays.asList( + new Constraint( + "some-context", + Operator.IN, + "some-value")))), + new FeatureToggle( + "toggleFeatureName1", + true, + Collections.singletonList( + new ActivationStrategy("custom", null))), + new FeatureToggle( + "toggleFeatureName2", + true, + Collections.singletonList( + new ActivationStrategy("custom", null))))); + FeatureRepository featureRepository = new FeatureRepository(config, backupHandler, new EventDispatcher(config), fetcher, bootstrapHandler); + verify(executor).setInterval(runnableArgumentCaptor.capture(), anyLong(), anyLong()); + when(fetcher.fetchFeatures()) + .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.UNAVAILABLE, 429)) + .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.UNAVAILABLE, 429)) + .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.UNAVAILABLE, 429)) + .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)) + .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)) + .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)) + .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)); + runnableArgumentCaptor.getValue().run(); + assertThat(featureRepository.getInterval()).isEqualTo(1); + assertThat(featureRepository.getFailures()).isEqualTo(1); + runnableArgumentCaptor.getValue().run(); + assertThat(featureRepository.getInterval()).isEqualTo(0); + assertThat(featureRepository.getFailures()).isEqualTo(1); + runnableArgumentCaptor.getValue().run(); + assertThat(featureRepository.getInterval()).isEqualTo(2); + assertThat(featureRepository.getFailures()).isEqualTo(2); + runnableArgumentCaptor.getValue().run(); // NO-OP because interval > 0 + runnableArgumentCaptor.getValue().run(); // NO-OP because interval > 0 + assertThat(featureRepository.getInterval()).isEqualTo(0); + assertThat(featureRepository.getFailures()).isEqualTo(2); + runnableArgumentCaptor.getValue().run(); + assertThat(featureRepository.getInterval()).isEqualTo(3); + assertThat(featureRepository.getFailures()).isEqualTo(3); + runnableArgumentCaptor.getValue().run(); + runnableArgumentCaptor.getValue().run(); + runnableArgumentCaptor.getValue().run(); + assertThat(featureRepository.getInterval()).isEqualTo(0); + assertThat(featureRepository.getFailures()).isEqualTo(3); + runnableArgumentCaptor.getValue().run(); + assertThat(featureRepository.getInterval()).isEqualTo(2); + assertThat(featureRepository.getFailures()).isEqualTo(2); + runnableArgumentCaptor.getValue().run(); + runnableArgumentCaptor.getValue().run(); + assertThat(featureRepository.getInterval()).isEqualTo(0); + assertThat(featureRepository.getFailures()).isEqualTo(2); + runnableArgumentCaptor.getValue().run(); + assertThat(featureRepository.getInterval()).isEqualTo(1); + assertThat(featureRepository.getFailures()).isEqualTo(1); + runnableArgumentCaptor.getValue().run(); + assertThat(featureRepository.getInterval()).isEqualTo(0); + assertThat(featureRepository.getFailures()).isEqualTo(1); + runnableArgumentCaptor.getValue().run(); + assertThat(featureRepository.getInterval()).isEqualTo(0); + assertThat(featureRepository.getFailures()).isEqualTo(0); + } + + @Test + public void server_errors_should_incrementally_increase_interval() throws URISyntaxException, IOException { + UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); + ArgumentCaptor runnableArgumentCaptor = ArgumentCaptor.forClass(Runnable.class); + File file = + new File(getClass().getClassLoader().getResource("unleash-repo-v2.json").toURI()); + ToggleBootstrapProvider toggleBootstrapProvider = mock(ToggleBootstrapProvider.class); + when(toggleBootstrapProvider.read()).thenReturn(fileToString(file)); + UnleashConfig config = + UnleashConfig.builder() + .synchronousFetchOnInitialisation(false) + .appName("test-sync-update") + .scheduledExecutor(executor) + .unleashAPI("http://localhost:8080") + .build(); + when(backupHandler.read()) + .thenReturn( + populatedFeatureCollection( + Arrays.asList( + new Segment( + 1, + "some-name", + Arrays.asList( + new Constraint( + "some-context", + Operator.IN, + "some-value")))), + new FeatureToggle( + "toggleFeatureName1", + true, + Collections.singletonList( + new ActivationStrategy("custom", null))), + new FeatureToggle( + "toggleFeatureName2", + true, + Collections.singletonList( + new ActivationStrategy("custom", null))))); + FeatureRepository featureRepository = new FeatureRepository(config, backupHandler, new EventDispatcher(config), fetcher, bootstrapHandler); + verify(executor).setInterval(runnableArgumentCaptor.capture(), anyLong(), anyLong()); + when(fetcher.fetchFeatures()) + .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.UNAVAILABLE, 500)) + .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.UNAVAILABLE, 502)) + .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.UNAVAILABLE, 503)) + .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)) + .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)) + .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)) + .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)); + runnableArgumentCaptor.getValue().run(); + assertThat(featureRepository.getInterval()).isEqualTo(1); + assertThat(featureRepository.getFailures()).isEqualTo(1); + runnableArgumentCaptor.getValue().run(); + assertThat(featureRepository.getInterval()).isEqualTo(0); + assertThat(featureRepository.getFailures()).isEqualTo(1); + runnableArgumentCaptor.getValue().run(); + assertThat(featureRepository.getInterval()).isEqualTo(2); + assertThat(featureRepository.getFailures()).isEqualTo(2); + runnableArgumentCaptor.getValue().run(); // NO-OP because interval > 0 + runnableArgumentCaptor.getValue().run(); // NO-OP because interval > 0 + assertThat(featureRepository.getInterval()).isEqualTo(0); + assertThat(featureRepository.getFailures()).isEqualTo(2); + runnableArgumentCaptor.getValue().run(); + assertThat(featureRepository.getInterval()).isEqualTo(3); + assertThat(featureRepository.getFailures()).isEqualTo(3); + runnableArgumentCaptor.getValue().run(); + runnableArgumentCaptor.getValue().run(); + runnableArgumentCaptor.getValue().run(); + assertThat(featureRepository.getInterval()).isEqualTo(0); + assertThat(featureRepository.getFailures()).isEqualTo(3); + runnableArgumentCaptor.getValue().run(); + assertThat(featureRepository.getInterval()).isEqualTo(2); + assertThat(featureRepository.getFailures()).isEqualTo(2); + runnableArgumentCaptor.getValue().run(); + runnableArgumentCaptor.getValue().run(); + assertThat(featureRepository.getInterval()).isEqualTo(0); + assertThat(featureRepository.getFailures()).isEqualTo(2); + runnableArgumentCaptor.getValue().run(); + assertThat(featureRepository.getInterval()).isEqualTo(1); + assertThat(featureRepository.getFailures()).isEqualTo(1); + runnableArgumentCaptor.getValue().run(); + assertThat(featureRepository.getInterval()).isEqualTo(0); + assertThat(featureRepository.getFailures()).isEqualTo(1); + runnableArgumentCaptor.getValue().run(); + assertThat(featureRepository.getInterval()).isEqualTo(0); + assertThat(featureRepository.getFailures()).isEqualTo(0); + } + private String fileToString(File f) throws IOException { return new String(Files.readAllBytes(f.toPath()), StandardCharsets.UTF_8); } + } diff --git a/src/test/java/io/getunleash/variant/VariantUtilTest.java b/src/test/java/io/getunleash/variant/VariantUtilTest.java index ee431edb6..ea4e5a4ab 100644 --- a/src/test/java/io/getunleash/variant/VariantUtilTest.java +++ b/src/test/java/io/getunleash/variant/VariantUtilTest.java @@ -5,6 +5,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; +import com.sangupta.murmur.Murmur3; import io.getunleash.ActivationStrategy; import io.getunleash.FeatureToggle; import io.getunleash.UnleashContext; @@ -410,4 +411,6 @@ public void feature_variants_variant_d_with_override_client_spec_tests() { toggle, UnleashContext.builder().userId("10").build(), DISABLED_VARIANT); assertThat(variantUser10.getName()).isEqualTo("variant2"); } + + } diff --git a/src/test/resources/logback-test.xml b/src/test/resources/logback-test.xml index a787811f5..a265cac70 100644 --- a/src/test/resources/logback-test.xml +++ b/src/test/resources/logback-test.xml @@ -11,5 +11,6 @@ + From a877b88aa562983b27ce2067fce9705c41edef31 Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Tue, 7 Nov 2023 13:53:24 +0100 Subject: [PATCH 02/15] fix: Remove debug line for interval/failures --- src/main/java/io/getunleash/repository/FeatureRepository.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/io/getunleash/repository/FeatureRepository.java b/src/main/java/io/getunleash/repository/FeatureRepository.java index 58da32b91..e1a85fdcb 100644 --- a/src/main/java/io/getunleash/repository/FeatureRepository.java +++ b/src/main/java/io/getunleash/repository/FeatureRepository.java @@ -108,7 +108,6 @@ private Runnable updateFeatures(@Nullable final Consumer handl } private void updateFeaturesInternal(@Nullable final Consumer handler) { - LOGGER.info("Interval: {}. Failures: {}", interval.get(), failures.get()); if (interval.get() <= 0L) { try { ClientFeaturesResponse response = featureFetcher.fetchFeatures(); From 65648c25c5484c2b30d3edeb12ba0524cbc00cf4 Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Tue, 7 Nov 2023 14:33:38 +0100 Subject: [PATCH 03/15] Updated subscriber test expectations --- .../metric/UnleashMetricServiceImpl.java | 37 +- .../repository/FeatureRepository.java | 99 ++- .../io/getunleash/variant/VariantUtil.java | 1 - .../io/getunleash/DefaultUnleashTest.java | 1 - .../io/getunleash/event/SubscriberTest.java | 21 +- .../metric/UnleashMetricServiceImplTest.java | 171 +++--- .../repository/FeatureRepositoryTest.java | 580 ++++++++++-------- .../getunleash/variant/VariantUtilTest.java | 3 - 8 files changed, 503 insertions(+), 410 deletions(-) diff --git a/src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java b/src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java index c8e2d5f16..d9972a087 100644 --- a/src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java +++ b/src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java @@ -2,14 +2,13 @@ import io.getunleash.util.UnleashConfig; import io.getunleash.util.UnleashScheduledExecutor; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import java.net.HttpURLConnection; import java.time.LocalDateTime; import java.time.ZoneId; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class UnleashMetricServiceImpl implements UnleashMetricService { private static final Logger LOGGER = LoggerFactory.getLogger(UnleashMetricServiceImpl.class); @@ -38,7 +37,14 @@ public UnleashMetricServiceImpl( this.started = LocalDateTime.now(ZoneId.of("UTC")); this.unleashConfig = unleashConfig; this.metricSender = metricSender; - this.maxInterval = Integer.max(20, 300 / Integer.max(Long.valueOf(unleashConfig.getSendMetricsInterval()).intValue(), 1)); + this.maxInterval = + Integer.max( + 20, + 300 + / Integer.max( + Long.valueOf(unleashConfig.getSendMetricsInterval()) + .intValue(), + 1)); long metricsInterval = unleashConfig.getSendMetricsInterval(); executor.setInterval(sendMetrics(), metricsInterval, metricsInterval); } @@ -86,17 +92,30 @@ private void handleHttpErrorCodes(int responseCode) { if (responseCode == 404) { interval.set(maxInterval); failures.incrementAndGet(); - LOGGER.error("Server said that the Metrics receiving endpoint at {} does not exist. Backing off to {} times our poll interval to avoid overloading server", unleashConfig.getUnleashURLs().getClientMetricsURL(), maxInterval); + LOGGER.error( + "Server said that the Metrics receiving endpoint at {} does not exist. Backing off to {} times our poll interval to avoid overloading server", + unleashConfig.getUnleashURLs().getClientMetricsURL(), + maxInterval); } else if (responseCode == 429) { interval.set(Math.min(failures.incrementAndGet(), maxInterval)); - LOGGER.info("Client Metrics was RATE LIMITED for the {}. time. Further backing off. Current backoff at {} times our metrics post interval", failures.get(), interval.get()); - } else if (responseCode == HttpURLConnection.HTTP_UNAUTHORIZED || responseCode == HttpURLConnection.HTTP_FORBIDDEN) { + LOGGER.info( + "Client Metrics was RATE LIMITED for the {}. time. Further backing off. Current backoff at {} times our metrics post interval", + failures.get(), + interval.get()); + } else if (responseCode == HttpURLConnection.HTTP_UNAUTHORIZED + || responseCode == HttpURLConnection.HTTP_FORBIDDEN) { failures.incrementAndGet(); interval.set(maxInterval); - LOGGER.error("Client was not authorized to post metrics to the Unleash API at {}. Backing off to {} times our poll interval to avoid overloading server", unleashConfig.getUnleashURLs().getClientMetricsURL(), maxInterval); + LOGGER.error( + "Client was not authorized to post metrics to the Unleash API at {}. Backing off to {} times our poll interval to avoid overloading server", + unleashConfig.getUnleashURLs().getClientMetricsURL(), + maxInterval); } else if (responseCode >= 500) { interval.set(Math.min(failures.incrementAndGet(), maxInterval)); - LOGGER.info("Server failed with a {} status code. Backing off. Current backoff at {} times our poll interval", responseCode, interval.get()); + LOGGER.info( + "Server failed with a {} status code. Backing off. Current backoff at {} times our poll interval", + responseCode, + interval.get()); } } diff --git a/src/main/java/io/getunleash/repository/FeatureRepository.java b/src/main/java/io/getunleash/repository/FeatureRepository.java index e1a85fdcb..b81338349 100644 --- a/src/main/java/io/getunleash/repository/FeatureRepository.java +++ b/src/main/java/io/getunleash/repository/FeatureRepository.java @@ -8,18 +8,16 @@ import io.getunleash.lang.Nullable; import io.getunleash.util.UnleashConfig; import io.getunleash.util.UnleashScheduledExecutor; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import java.net.HttpURLConnection; import java.util.Collections; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; import java.util.stream.Collectors; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class FeatureRepository implements IFeatureRepository { - private final Integer maxInterval; // Set to 20 times our polling interval, so with our default of 15 seconds, the longest interval will be 5 minutes. private static final Logger LOGGER = LoggerFactory.getLogger(FeatureRepository.class); private final UnleashConfig unleashConfig; private final BackupHandler featureBackupHandler; @@ -32,52 +30,74 @@ public class FeatureRepository implements IFeatureRepository { private AtomicInteger failures = new AtomicInteger(0); private AtomicInteger interval = new AtomicInteger(0); + private final Integer maxInterval; public FeatureRepository(UnleashConfig unleashConfig) { this(unleashConfig, new FeatureBackupHandlerFile(unleashConfig)); } public FeatureRepository( - UnleashConfig unleashConfig, - final BackupHandler featureBackupHandler) { + UnleashConfig unleashConfig, + final BackupHandler featureBackupHandler) { this.unleashConfig = unleashConfig; this.featureBackupHandler = featureBackupHandler; this.featureFetcher = unleashConfig.getUnleashFeatureFetcherFactory().apply(unleashConfig); this.featureBootstrapHandler = new FeatureBootstrapHandler(unleashConfig); this.eventDispatcher = new EventDispatcher(unleashConfig); - this.maxInterval = Integer.max(20, 300 / Integer.max(Long.valueOf(unleashConfig.getFetchTogglesInterval()).intValue(), 1)); + this.maxInterval = + Integer.max( + 20, + 300 + / Integer.max( + Long.valueOf(unleashConfig.getFetchTogglesInterval()) + .intValue(), + 1)); this.initCollections(unleashConfig.getScheduledExecutor()); } protected FeatureRepository( - UnleashConfig unleashConfig, - BackupHandler featureBackupHandler, - EventDispatcher eventDispatcher, - FeatureFetcher featureFetcher, - FeatureBootstrapHandler featureBootstrapHandler) { + UnleashConfig unleashConfig, + BackupHandler featureBackupHandler, + EventDispatcher eventDispatcher, + FeatureFetcher featureFetcher, + FeatureBootstrapHandler featureBootstrapHandler) { this.unleashConfig = unleashConfig; this.featureBackupHandler = featureBackupHandler; this.featureFetcher = featureFetcher; this.featureBootstrapHandler = featureBootstrapHandler; this.eventDispatcher = eventDispatcher; - this.maxInterval = Integer.max(20, 300 / Integer.max(Long.valueOf(unleashConfig.getFetchTogglesInterval()).intValue(), 1)); + this.maxInterval = + Integer.max( + 20, + 300 + / Integer.max( + Long.valueOf(unleashConfig.getFetchTogglesInterval()) + .intValue(), + 1)); this.initCollections(unleashConfig.getScheduledExecutor()); } protected FeatureRepository( - UnleashConfig unleashConfig, - FeatureBackupHandlerFile featureBackupHandler, - UnleashScheduledExecutor executor, - FeatureFetcher featureFetcher, - FeatureBootstrapHandler featureBootstrapHandler) { + UnleashConfig unleashConfig, + FeatureBackupHandlerFile featureBackupHandler, + UnleashScheduledExecutor executor, + FeatureFetcher featureFetcher, + FeatureBootstrapHandler featureBootstrapHandler) { this.unleashConfig = unleashConfig; this.featureBackupHandler = featureBackupHandler; this.featureFetcher = featureFetcher; this.featureBootstrapHandler = featureBootstrapHandler; this.eventDispatcher = new EventDispatcher(unleashConfig); - this.maxInterval = Integer.max(20, 300 / Integer.max(Long.valueOf(unleashConfig.getFetchTogglesInterval()).intValue(), 1)); + this.maxInterval = + Integer.max( + 20, + 300 + / Integer.max( + Long.valueOf(unleashConfig.getFetchTogglesInterval()) + .intValue(), + 1)); this.initCollections(executor); } @@ -115,25 +135,23 @@ private void updateFeaturesInternal(@Nullable final Consumer h if (response.getStatus() == ClientFeaturesResponse.Status.CHANGED) { SegmentCollection segmentCollection = response.getSegmentCollection(); featureCollection = - new FeatureCollection( - response.getToggleCollection(), - segmentCollection != null - ? segmentCollection - : new SegmentCollection(Collections.emptyList())); + new FeatureCollection( + response.getToggleCollection(), + segmentCollection != null + ? segmentCollection + : new SegmentCollection(Collections.emptyList())); featureBackupHandler.write(featureCollection); } else if (response.getStatus() == ClientFeaturesResponse.Status.UNAVAILABLE) { handleHttpErrorCodes(response.getHttpStatusCode()); return; } - - interval.set(Math.max(failures.decrementAndGet(),0)); + interval.set(Math.max(failures.decrementAndGet(), 0)); if (!ready) { eventDispatcher.dispatch(new UnleashReady()); ready = true; } } catch (UnleashException e) { - interval.set(Math.min(failures.incrementAndGet(), maxInterval)); if (handler != null) { handler.accept(e); } else { @@ -149,17 +167,30 @@ private void handleHttpErrorCodes(int responseCode) { if (responseCode == 404) { interval.set(maxInterval); failures.incrementAndGet(); - LOGGER.error("Server said that the API at {} does not exist. Backing off to {} times our poll interval to avoid overloading server", unleashConfig.getUnleashAPI(), maxInterval); + LOGGER.error( + "Server said that the API at {} does not exist. Backing off to {} times our poll interval to avoid overloading server", + unleashConfig.getUnleashAPI(), + maxInterval); } else if (responseCode == 429) { interval.set(Math.min(failures.incrementAndGet(), maxInterval)); - LOGGER.info("Client was RATE LIMITED for the {} time. Further backing off. Current backoff at {} times our poll interval", failures.get(), interval.get()); - } else if (responseCode == HttpURLConnection.HTTP_UNAUTHORIZED || responseCode == HttpURLConnection.HTTP_FORBIDDEN) { + LOGGER.info( + "Client was RATE LIMITED for the {} time. Further backing off. Current backoff at {} times our poll interval", + failures.get(), + interval.get()); + } else if (responseCode == HttpURLConnection.HTTP_UNAUTHORIZED + || responseCode == HttpURLConnection.HTTP_FORBIDDEN) { failures.incrementAndGet(); interval.set(maxInterval); - LOGGER.error("Client failed to authenticate to the Unleash API at {}. Backing off to {} times our poll interval to avoid overloading server", unleashConfig.getUnleashAPI(), maxInterval); + LOGGER.error( + "Client failed to authenticate to the Unleash API at {}. Backing off to {} times our poll interval to avoid overloading server", + unleashConfig.getUnleashAPI(), + maxInterval); } else if (responseCode >= 500) { interval.set(Math.min(failures.incrementAndGet(), maxInterval)); - LOGGER.info("Server failed with a {} status code. Backing off. Current backoff at {} times our poll interval", responseCode, interval.get()); + LOGGER.info( + "Server failed with a {} status code. Backing off. Current backoff at {} times our poll interval", + responseCode, + interval.get()); } } @@ -171,8 +202,8 @@ private void handleHttpErrorCodes(int responseCode) { @Override public List getFeatureNames() { return featureCollection.getToggleCollection().getFeatures().stream() - .map(FeatureToggle::getName) - .collect(Collectors.toList()); + .map(FeatureToggle::getName) + .collect(Collectors.toList()); } @Override diff --git a/src/main/java/io/getunleash/variant/VariantUtil.java b/src/main/java/io/getunleash/variant/VariantUtil.java index f2c71493b..0a0fd5655 100644 --- a/src/main/java/io/getunleash/variant/VariantUtil.java +++ b/src/main/java/io/getunleash/variant/VariantUtil.java @@ -133,5 +133,4 @@ public static Variant selectVariant( } return null; } - } diff --git a/src/test/java/io/getunleash/DefaultUnleashTest.java b/src/test/java/io/getunleash/DefaultUnleashTest.java index 2f620735e..44f329272 100644 --- a/src/test/java/io/getunleash/DefaultUnleashTest.java +++ b/src/test/java/io/getunleash/DefaultUnleashTest.java @@ -243,7 +243,6 @@ public void asynchronous_fetch_on_initialisation_fails_silently_and_retries() when(fetcher.fetchFeatures()) .thenThrow(UnleashException.class) .thenReturn(new ClientFeaturesResponse(expectedStatus, expectedResponse)); - UnleashConfig config = UnleashConfig.builder() .unleashAPI("http://wrong:4242") diff --git a/src/test/java/io/getunleash/event/SubscriberTest.java b/src/test/java/io/getunleash/event/SubscriberTest.java index de76ead65..b190ac364 100644 --- a/src/test/java/io/getunleash/event/SubscriberTest.java +++ b/src/test/java/io/getunleash/event/SubscriberTest.java @@ -1,7 +1,8 @@ package io.getunleash.event; +import static com.github.tomakehurst.wiremock.client.WireMock.*; import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; -import static io.getunleash.repository.FeatureToggleResponse.Status.UNAVAILABLE; +import static io.getunleash.repository.FeatureToggleResponse.Status.CHANGED; import static org.assertj.core.api.Assertions.assertThat; import com.github.tomakehurst.wiremock.junit5.WireMockExtension; @@ -44,19 +45,29 @@ void setup() { } @Test - void subscriberAreNotified() { + void subscribersAreNotified() { + serverMock.stubFor(post("/client/register").willReturn(ok())); + serverMock.stubFor(post("/client/metrics").willReturn(ok())); + serverMock.stubFor( + get("/client/features") + .willReturn( + ok().withHeader("Content-Type", "application/json") + .withBody("{\"features\": [], \"version\": 2 }"))); Unleash unleash = new DefaultUnleash(unleashConfig); unleash.isEnabled("myFeature"); unleash.isEnabled("myFeature"); unleash.isEnabled("myFeature"); - assertThat(testSubscriber.togglesFetchedCounter).isEqualTo(2); // one forced, one scheduled - assertThat(testSubscriber.status).isEqualTo(UNAVAILABLE); + assertThat(testSubscriber.togglesFetchedCounter) + .isEqualTo( + 2); // Server successfully returns, we call synchronous fetch and schedule + // once, so 2 calls. + assertThat(testSubscriber.status).isEqualTo(CHANGED); assertThat(testSubscriber.toggleEvaluatedCounter).isEqualTo(3); assertThat(testSubscriber.toggleName).isEqualTo("myFeature"); assertThat(testSubscriber.toggleEnabled).isFalse(); - assertThat(testSubscriber.errors).hasSize(2); + assertThat(testSubscriber.errors).isEmpty(); // assertThat(testSubscriber.events).filteredOn(e -> e instanceof // ToggleBootstrapHandler.ToggleBootstrapRead).hasSize(1); diff --git a/src/test/java/io/getunleash/metric/UnleashMetricServiceImplTest.java b/src/test/java/io/getunleash/metric/UnleashMetricServiceImplTest.java index a82940321..975a015a4 100644 --- a/src/test/java/io/getunleash/metric/UnleashMetricServiceImplTest.java +++ b/src/test/java/io/getunleash/metric/UnleashMetricServiceImplTest.java @@ -3,14 +3,10 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.*; -import io.getunleash.repository.ClientFeaturesResponse; -import io.getunleash.repository.FeatureToggleResponse; import io.getunleash.util.UnleashConfig; import io.getunleash.util.UnleashScheduledExecutor; - import java.util.HashSet; import java.util.Set; - import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; @@ -20,11 +16,11 @@ public class UnleashMetricServiceImplTest { public void should_register_future_for_sending_interval_regualry() { long interval = 10; UnleashConfig config = - UnleashConfig.builder() - .appName("test") - .sendMetricsInterval(interval) - .unleashAPI("http://unleash.com") - .build(); + UnleashConfig.builder() + .appName("test") + .sendMetricsInterval(interval) + .unleashAPI("http://unleash.com") + .build(); UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); UnleashMetricService unleashMetricService = new UnleashMetricServiceImpl(config, executor); @@ -35,24 +31,24 @@ public void should_register_future_for_sending_interval_regualry() { public void should_register_client() { long interval = 10; UnleashConfig config = - UnleashConfig.builder() - .appName("test") - .sendMetricsInterval(interval) - .unleashAPI("http://unleash.com") - .build(); + UnleashConfig.builder() + .appName("test") + .sendMetricsInterval(interval) + .unleashAPI("http://unleash.com") + .build(); UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); DefaultHttpMetricsSender sender = mock(DefaultHttpMetricsSender.class); UnleashMetricService unleashMetricService = - new UnleashMetricServiceImpl(config, sender, executor); + new UnleashMetricServiceImpl(config, sender, executor); Set strategies = new HashSet<>(); strategies.add("default"); strategies.add("custom"); unleashMetricService.register(strategies); ArgumentCaptor argument = - ArgumentCaptor.forClass(ClientRegistration.class); + ArgumentCaptor.forClass(ClientRegistration.class); verify(sender).registerClient(argument.capture()); assertThat(argument.getValue().getAppName()).isEqualTo(config.getAppName()); @@ -66,25 +62,25 @@ public void should_register_client() { public void should_register_client_with_env() { long interval = 10; UnleashConfig config = - UnleashConfig.builder() - .appName("test") - .environment("dev") - .sendMetricsInterval(interval) - .unleashAPI("http://unleash.com") - .build(); + UnleashConfig.builder() + .appName("test") + .environment("dev") + .sendMetricsInterval(interval) + .unleashAPI("http://unleash.com") + .build(); UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); DefaultHttpMetricsSender sender = mock(DefaultHttpMetricsSender.class); UnleashMetricService unleashMetricService = - new UnleashMetricServiceImpl(config, sender, executor); + new UnleashMetricServiceImpl(config, sender, executor); Set strategies = new HashSet<>(); strategies.add("default"); strategies.add("custom"); unleashMetricService.register(strategies); ArgumentCaptor argument = - ArgumentCaptor.forClass(ClientRegistration.class); + ArgumentCaptor.forClass(ClientRegistration.class); verify(sender).registerClient(argument.capture()); assertThat(argument.getValue().getEnvironment()).isEqualTo(config.getEnvironment()); @@ -93,17 +89,17 @@ public void should_register_client_with_env() { @Test public void should_send_metrics() { UnleashConfig config = - UnleashConfig.builder() - .appName("test") - .sendMetricsInterval(10) - .unleashAPI("http://unleash.com") - .build(); + UnleashConfig.builder() + .appName("test") + .sendMetricsInterval(10) + .unleashAPI("http://unleash.com") + .build(); UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); DefaultHttpMetricsSender sender = mock(DefaultHttpMetricsSender.class); UnleashMetricService unleashMetricService = - new UnleashMetricServiceImpl(config, sender, executor); + new UnleashMetricServiceImpl(config, sender, executor); ArgumentCaptor sendMetricsCallback = ArgumentCaptor.forClass(Runnable.class); verify(executor).setInterval(sendMetricsCallback.capture(), anyLong(), anyLong()); @@ -115,18 +111,18 @@ public void should_send_metrics() { @Test public void should_record_and_send_metrics() { UnleashConfig config = - UnleashConfig.builder() - .appName("test") - .environment("prod") - .sendMetricsInterval(10) - .unleashAPI("http://unleash.com") - .build(); + UnleashConfig.builder() + .appName("test") + .environment("prod") + .sendMetricsInterval(10) + .unleashAPI("http://unleash.com") + .build(); UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); DefaultHttpMetricsSender sender = mock(DefaultHttpMetricsSender.class); UnleashMetricService unleashMetricService = - new UnleashMetricServiceImpl(config, sender, executor); + new UnleashMetricServiceImpl(config, sender, executor); unleashMetricService.count("someToggle", true); unleashMetricService.count("someToggle", false); unleashMetricService.count("someToggle", true); @@ -138,7 +134,7 @@ public void should_record_and_send_metrics() { sendMetricsCallback.getValue().run(); ArgumentCaptor clientMetricsArgumentCaptor = - ArgumentCaptor.forClass(ClientMetrics.class); + ArgumentCaptor.forClass(ClientMetrics.class); verify(sender).sendMetrics(clientMetricsArgumentCaptor.capture()); ClientMetrics clientMetrics = clientMetricsArgumentCaptor.getValue(); @@ -157,17 +153,17 @@ public void should_record_and_send_metrics() { @Test public void should_record_and_send_variant_metrics() { UnleashConfig config = - UnleashConfig.builder() - .appName("test") - .sendMetricsInterval(10) - .unleashAPI("http://unleash.com") - .build(); + UnleashConfig.builder() + .appName("test") + .sendMetricsInterval(10) + .unleashAPI("http://unleash.com") + .build(); UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); DefaultHttpMetricsSender sender = mock(DefaultHttpMetricsSender.class); UnleashMetricService unleashMetricService = - new UnleashMetricServiceImpl(config, sender, executor); + new UnleashMetricServiceImpl(config, sender, executor); unleashMetricService.countVariant("someToggle", "v1"); unleashMetricService.countVariant("someToggle", "v1"); unleashMetricService.countVariant("someToggle", "v1"); @@ -180,7 +176,7 @@ public void should_record_and_send_variant_metrics() { sendMetricsCallback.getValue().run(); ArgumentCaptor clientMetricsArgumentCaptor = - ArgumentCaptor.forClass(ClientMetrics.class); + ArgumentCaptor.forClass(ClientMetrics.class); verify(sender).sendMetrics(clientMetricsArgumentCaptor.capture()); ClientMetrics clientMetrics = clientMetricsArgumentCaptor.getValue(); @@ -192,11 +188,11 @@ public void should_record_and_send_variant_metrics() { assertThat(bucket.getStop()).isNotNull(); assertThat(bucket.getToggles()).hasSize(1); assertThat(bucket.getToggles().get("someToggle").getVariants().get("v1").longValue()) - .isEqualTo(3l); + .isEqualTo(3l); assertThat(bucket.getToggles().get("someToggle").getVariants().get("v2").longValue()) - .isEqualTo(1l); + .isEqualTo(1l); assertThat(bucket.getToggles().get("someToggle").getVariants().get("disabled").longValue()) - .isEqualTo(1l); + .isEqualTo(1l); assertThat(bucket.getToggles().get("someToggle").getYes()).isEqualTo(0l); assertThat(bucket.getToggles().get("someToggle").getNo()).isEqualTo(0l); } @@ -204,17 +200,17 @@ public void should_record_and_send_variant_metrics() { @Test public void should_backoff_when_told_to_by_429_code() { UnleashConfig config = - UnleashConfig.builder() - .appName("test") - .sendMetricsInterval(10) - .unleashAPI("http://unleash.com") - .build(); + UnleashConfig.builder() + .appName("test") + .sendMetricsInterval(10) + .unleashAPI("http://unleash.com") + .build(); UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); DefaultHttpMetricsSender sender = mock(DefaultHttpMetricsSender.class); UnleashMetricServiceImpl unleashMetricService = - new UnleashMetricServiceImpl(config, sender, executor); + new UnleashMetricServiceImpl(config, sender, executor); unleashMetricService.countVariant("someToggle", "v1"); unleashMetricService.countVariant("someToggle", "v1"); unleashMetricService.countVariant("someToggle", "v1"); @@ -225,7 +221,14 @@ public void should_backoff_when_told_to_by_429_code() { ArgumentCaptor sendMetricsCallback = ArgumentCaptor.forClass(Runnable.class); verify(executor).setInterval(sendMetricsCallback.capture(), anyLong(), anyLong()); - when(sender.sendMetrics(any(ClientMetrics.class))).thenReturn(429).thenReturn(429).thenReturn(429).thenReturn(200).thenReturn(200).thenReturn(200).thenReturn(200); + when(sender.sendMetrics(any(ClientMetrics.class))) + .thenReturn(429) + .thenReturn(429) + .thenReturn(429) + .thenReturn(200) + .thenReturn(200) + .thenReturn(200) + .thenReturn(200); sendMetricsCallback.getValue().run(); assertThat(unleashMetricService.getInterval()).isEqualTo(1); @@ -269,17 +272,17 @@ public void should_backoff_when_told_to_by_429_code() { @Test public void server_errors_should_also_incrementally_backoff() { UnleashConfig config = - UnleashConfig.builder() - .appName("test") - .sendMetricsInterval(10) - .unleashAPI("http://unleash.com") - .build(); + UnleashConfig.builder() + .appName("test") + .sendMetricsInterval(10) + .unleashAPI("http://unleash.com") + .build(); UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); DefaultHttpMetricsSender sender = mock(DefaultHttpMetricsSender.class); UnleashMetricServiceImpl unleashMetricService = - new UnleashMetricServiceImpl(config, sender, executor); + new UnleashMetricServiceImpl(config, sender, executor); unleashMetricService.countVariant("someToggle", "v1"); unleashMetricService.countVariant("someToggle", "v1"); unleashMetricService.countVariant("someToggle", "v1"); @@ -290,13 +293,13 @@ public void server_errors_should_also_incrementally_backoff() { ArgumentCaptor sendMetricsCallback = ArgumentCaptor.forClass(Runnable.class); verify(executor).setInterval(sendMetricsCallback.capture(), anyLong(), anyLong()); when(sender.sendMetrics(any(ClientMetrics.class))) - .thenReturn(500) - .thenReturn(502) - .thenReturn(503) - .thenReturn(304) - .thenReturn(304) - .thenReturn(304) - .thenReturn(304); + .thenReturn(500) + .thenReturn(502) + .thenReturn(503) + .thenReturn(304) + .thenReturn(304) + .thenReturn(304) + .thenReturn(304); sendMetricsCallback.getValue().run(); assertThat(unleashMetricService.getInterval()).isEqualTo(1); assertThat(unleashMetricService.getFailures()).isEqualTo(1); @@ -339,17 +342,17 @@ public void server_errors_should_also_incrementally_backoff() { @Test public void failure_to_authenticate_immediately_increases_interval_to_max() { UnleashConfig config = - UnleashConfig.builder() - .appName("test") - .sendMetricsInterval(10) - .unleashAPI("http://unleash.com") - .build(); + UnleashConfig.builder() + .appName("test") + .sendMetricsInterval(10) + .unleashAPI("http://unleash.com") + .build(); UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); DefaultHttpMetricsSender sender = mock(DefaultHttpMetricsSender.class); UnleashMetricServiceImpl unleashMetricService = - new UnleashMetricServiceImpl(config, sender, executor); + new UnleashMetricServiceImpl(config, sender, executor); unleashMetricService.countVariant("someToggle", "v1"); unleashMetricService.countVariant("someToggle", "v1"); unleashMetricService.countVariant("someToggle", "v1"); @@ -359,9 +362,7 @@ public void failure_to_authenticate_immediately_increases_interval_to_max() { // Call the sendMetricsCallback ArgumentCaptor sendMetricsCallback = ArgumentCaptor.forClass(Runnable.class); verify(executor).setInterval(sendMetricsCallback.capture(), anyLong(), anyLong()); - when(sender.sendMetrics(any(ClientMetrics.class))) - .thenReturn(403) - .thenReturn(200); + when(sender.sendMetrics(any(ClientMetrics.class))).thenReturn(403).thenReturn(200); sendMetricsCallback.getValue().run(); assertThat(unleashMetricService.getInterval()).isEqualTo(30); assertThat(unleashMetricService.getFailures()).isEqualTo(1); @@ -378,17 +379,17 @@ public void failure_to_authenticate_immediately_increases_interval_to_max() { @Test public void url_not_found_immediately_increases_interval_to_max() { UnleashConfig config = - UnleashConfig.builder() - .appName("test") - .sendMetricsInterval(10) - .unleashAPI("http://unleash.com") - .build(); + UnleashConfig.builder() + .appName("test") + .sendMetricsInterval(10) + .unleashAPI("http://unleash.com") + .build(); UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); DefaultHttpMetricsSender sender = mock(DefaultHttpMetricsSender.class); UnleashMetricServiceImpl unleashMetricService = - new UnleashMetricServiceImpl(config, sender, executor); + new UnleashMetricServiceImpl(config, sender, executor); unleashMetricService.countVariant("someToggle", "v1"); unleashMetricService.countVariant("someToggle", "v1"); unleashMetricService.countVariant("someToggle", "v1"); @@ -398,9 +399,7 @@ public void url_not_found_immediately_increases_interval_to_max() { // Call the sendMetricsCallback ArgumentCaptor sendMetricsCallback = ArgumentCaptor.forClass(Runnable.class); verify(executor).setInterval(sendMetricsCallback.capture(), anyLong(), anyLong()); - when(sender.sendMetrics(any(ClientMetrics.class))) - .thenReturn(404) - .thenReturn(200); + when(sender.sendMetrics(any(ClientMetrics.class))).thenReturn(404).thenReturn(200); sendMetricsCallback.getValue().run(); assertThat(unleashMetricService.getInterval()).isEqualTo(30); assertThat(unleashMetricService.getFailures()).isEqualTo(1); diff --git a/src/test/java/io/getunleash/repository/FeatureRepositoryTest.java b/src/test/java/io/getunleash/repository/FeatureRepositoryTest.java index 0eb5a4cc9..3a0afd6ff 100644 --- a/src/test/java/io/getunleash/repository/FeatureRepositoryTest.java +++ b/src/test/java/io/getunleash/repository/FeatureRepositoryTest.java @@ -1,7 +1,5 @@ package io.getunleash.repository; -import static java.util.stream.Collectors.toCollection; -import static java.util.stream.Collectors.toList; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.*; @@ -11,24 +9,16 @@ import io.getunleash.lang.Nullable; import io.getunleash.util.UnleashConfig; import io.getunleash.util.UnleashScheduledExecutor; - import java.io.File; import java.io.IOException; import java.net.URISyntaxException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; -import java.time.Clock; -import java.time.Duration; -import java.time.Instant; -import java.time.temporal.ChronoUnit; -import java.time.temporal.TemporalUnit; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.concurrent.*; - -import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; @@ -48,24 +38,24 @@ public void setUp() { fetcher = mock(HttpFeatureFetcher.class); defaultConfig = - new UnleashConfig.Builder() - .appName("test") - .unleashAPI("http://localhost:4242/api/") - .scheduledExecutor(mock(UnleashScheduledExecutor.class)) - .fetchTogglesInterval(200L) - .synchronousFetchOnInitialisation(false) - .build(); + new UnleashConfig.Builder() + .appName("test") + .unleashAPI("http://localhost:4242/api/") + .scheduledExecutor(mock(UnleashScheduledExecutor.class)) + .fetchTogglesInterval(200L) + .synchronousFetchOnInitialisation(false) + .build(); } @Test public void no_backup_file_and_no_repository_available_should_give_empty_repo() { UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); UnleashConfig config = - UnleashConfig.builder() - .appName("test") - .unleashAPI("http://localhost:4242/api/") - .scheduledExecutor(executor) - .build(); + UnleashConfig.builder() + .appName("test") + .unleashAPI("http://localhost:4242/api/") + .scheduledExecutor(executor) + .build(); when(backupHandler.read()).thenReturn(new FeatureCollection()); when(bootstrapHandler.read()).thenReturn(new FeatureCollection()); @@ -77,21 +67,21 @@ public void no_backup_file_and_no_repository_available_should_give_empty_repo() public void backup_toggles_should_be_loaded_at_startup() { UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); UnleashConfig config = - UnleashConfig.builder() - .appName("test") - .scheduledExecutor(executor) - .unleashAPI("http://localhost:4242/api/") - .fetchTogglesInterval(Long.MAX_VALUE) - .build(); + UnleashConfig.builder() + .appName("test") + .scheduledExecutor(executor) + .unleashAPI("http://localhost:4242/api/") + .fetchTogglesInterval(Long.MAX_VALUE) + .build(); when(backupHandler.read()) - .thenReturn( - new FeatureCollection( - new ToggleCollection(Collections.emptyList()), - new SegmentCollection(Collections.emptyList()))); + .thenReturn( + new FeatureCollection( + new ToggleCollection(Collections.emptyList()), + new SegmentCollection(Collections.emptyList()))); new FeatureRepository( - config, backupHandler, new EventDispatcher(config), fetcher, bootstrapHandler); + config, backupHandler, new EventDispatcher(config), fetcher, bootstrapHandler); verify(backupHandler, times(1)).read(); } @@ -102,37 +92,37 @@ public void feature_toggles_should_be_updated() { ArgumentCaptor runnableArgumentCaptor = ArgumentCaptor.forClass(Runnable.class); UnleashConfig config = - new UnleashConfig.Builder() - .appName("test") - .unleashAPI("http://localhost:4242/api/") - .scheduledExecutor(executor) - .fetchTogglesInterval(200L) - .synchronousFetchOnInitialisation(false) - .build(); + new UnleashConfig.Builder() + .appName("test") + .unleashAPI("http://localhost:4242/api/") + .scheduledExecutor(executor) + .fetchTogglesInterval(200L) + .synchronousFetchOnInitialisation(false) + .build(); FeatureCollection featureCollection = - populatedFeatureCollection( - null, - new FeatureToggle( - "toggleFetcherCalled", - false, - Arrays.asList(new ActivationStrategy("custom", null)))); + populatedFeatureCollection( + null, + new FeatureToggle( + "toggleFetcherCalled", + false, + Arrays.asList(new ActivationStrategy("custom", null)))); when(backupHandler.read()).thenReturn(featureCollection); featureCollection = - populatedFeatureCollection( - null, - new FeatureToggle( - "toggleFetcherCalled", - true, - Arrays.asList(new ActivationStrategy("custom", null)))); + populatedFeatureCollection( + null, + new FeatureToggle( + "toggleFetcherCalled", + true, + Arrays.asList(new ActivationStrategy("custom", null)))); ClientFeaturesResponse response = - new ClientFeaturesResponse( - ClientFeaturesResponse.Status.CHANGED, featureCollection); + new ClientFeaturesResponse( + ClientFeaturesResponse.Status.CHANGED, featureCollection); FeatureRepository featureRepository = - new FeatureRepository(config, backupHandler, executor, fetcher, bootstrapHandler); + new FeatureRepository(config, backupHandler, executor, fetcher, bootstrapHandler); // run the toggleName fetcher callback verify(executor).setInterval(runnableArgumentCaptor.capture(), anyLong(), anyLong()); verify(fetcher, times(0)).fetchFeatures(); @@ -148,25 +138,25 @@ public void feature_toggles_should_be_updated() { @Test public void get_feature_names_should_return_list_of_names() { FeatureCollection featureCollection = - populatedFeatureCollection( - null, - new FeatureToggle( - "toggleFeatureName1", - true, - Arrays.asList(new ActivationStrategy("custom", null))), - new FeatureToggle( - "toggleFeatureName2", - true, - Arrays.asList(new ActivationStrategy("custom", null)))); + populatedFeatureCollection( + null, + new FeatureToggle( + "toggleFeatureName1", + true, + Arrays.asList(new ActivationStrategy("custom", null))), + new FeatureToggle( + "toggleFeatureName2", + true, + Arrays.asList(new ActivationStrategy("custom", null)))); when(backupHandler.read()).thenReturn(featureCollection); FeatureRepository featureRepository = - new FeatureRepository( - defaultConfig, - backupHandler, - new EventDispatcher(defaultConfig), - fetcher, - bootstrapHandler); + new FeatureRepository( + defaultConfig, + backupHandler, + new EventDispatcher(defaultConfig), + fetcher, + bootstrapHandler); assertEquals(2, featureRepository.getFeatureNames().size()); assertEquals("toggleFeatureName2", featureRepository.getFeatureNames().get(1)); } @@ -175,23 +165,23 @@ public void get_feature_names_should_return_list_of_names() { public void should_perform_synchronous_fetch_on_initialisation() { UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); UnleashConfig config = - UnleashConfig.builder() - .synchronousFetchOnInitialisation(true) - .scheduledExecutor(executor) - .appName("test-sync-update") - .unleashAPI("http://localhost:8080") - .build(); + UnleashConfig.builder() + .synchronousFetchOnInitialisation(true) + .scheduledExecutor(executor) + .appName("test-sync-update") + .unleashAPI("http://localhost:8080") + .build(); when(backupHandler.read()).thenReturn(new FeatureCollection()); FeatureCollection featureCollection = populatedFeatureCollection(null); ClientFeaturesResponse response = - new ClientFeaturesResponse( - ClientFeaturesResponse.Status.CHANGED, featureCollection); + new ClientFeaturesResponse( + ClientFeaturesResponse.Status.CHANGED, featureCollection); when(fetcher.fetchFeatures()).thenReturn(response); new FeatureRepository( - config, backupHandler, new EventDispatcher(config), fetcher, bootstrapHandler); + config, backupHandler, new EventDispatcher(config), fetcher, bootstrapHandler); verify(fetcher, times(1)).fetchFeatures(); } @@ -199,19 +189,19 @@ public void should_perform_synchronous_fetch_on_initialisation() { public void should_not_perform_synchronous_fetch_on_initialisation() { UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); UnleashConfig config = - UnleashConfig.builder() - .synchronousFetchOnInitialisation(false) - .scheduledExecutor(executor) - .appName("test-sync-update") - .unleashAPI("http://localhost:8080") - .build(); + UnleashConfig.builder() + .synchronousFetchOnInitialisation(false) + .scheduledExecutor(executor) + .appName("test-sync-update") + .unleashAPI("http://localhost:8080") + .build(); when(backupHandler.read()).thenReturn(new FeatureCollection()); FeatureCollection featureCollection = populatedFeatureCollection(null); ClientFeaturesResponse response = - new ClientFeaturesResponse( - ClientFeaturesResponse.Status.CHANGED, featureCollection); + new ClientFeaturesResponse( + ClientFeaturesResponse.Status.CHANGED, featureCollection); when(fetcher.fetchFeatures()).thenReturn(response); @@ -221,7 +211,7 @@ public void should_not_perform_synchronous_fetch_on_initialisation() { } private FeatureCollection populatedFeatureCollection( - @Nullable List segments, FeatureToggle... featureToggles) { + @Nullable List segments, FeatureToggle... featureToggles) { List toggleList = new ArrayList<>(); toggleList.addAll(Arrays.asList(featureToggles)); @@ -229,27 +219,27 @@ private FeatureCollection populatedFeatureCollection( if (segments != null) segmentList.addAll(segments); return new FeatureCollection( - new ToggleCollection(toggleList), new SegmentCollection(segmentList)); + new ToggleCollection(toggleList), new SegmentCollection(segmentList)); } @Test public void should_read_from_bootstrap_location_if_backup_was_empty() - throws URISyntaxException, IOException { + throws URISyntaxException, IOException { File file = - new File(getClass().getClassLoader().getResource("unleash-repo-v2.json").toURI()); + new File(getClass().getClassLoader().getResource("unleash-repo-v2.json").toURI()); ToggleBootstrapProvider toggleBootstrapProvider = mock(ToggleBootstrapProvider.class); when(toggleBootstrapProvider.read()).thenReturn(fileToString(file)); UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); UnleashConfig config = - UnleashConfig.builder() - .synchronousFetchOnInitialisation(false) - .appName("test-sync-update") - .scheduledExecutor(executor) - .unleashAPI("http://localhost:8080") - .toggleBootstrapProvider(toggleBootstrapProvider) - .build(); + UnleashConfig.builder() + .synchronousFetchOnInitialisation(false) + .appName("test-sync-update") + .scheduledExecutor(executor) + .unleashAPI("http://localhost:8080") + .toggleBootstrapProvider(toggleBootstrapProvider) + .build(); when(backupHandler.read()).thenReturn(new FeatureCollection()); @@ -259,91 +249,101 @@ public void should_read_from_bootstrap_location_if_backup_was_empty() @Test public void should_not_read_bootstrap_if_backup_was_found() - throws IOException, URISyntaxException { + throws IOException, URISyntaxException { File file = - new File(getClass().getClassLoader().getResource("unleash-repo-v2.json").toURI()); + new File(getClass().getClassLoader().getResource("unleash-repo-v2.json").toURI()); ToggleBootstrapProvider toggleBootstrapProvider = mock(ToggleBootstrapProvider.class); UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); UnleashConfig config = - UnleashConfig.builder() - .synchronousFetchOnInitialisation(false) - .appName("test-sync-update") - .scheduledExecutor(executor) - .unleashAPI("http://localhost:8080") - .toggleBootstrapProvider(toggleBootstrapProvider) - .build(); + UnleashConfig.builder() + .synchronousFetchOnInitialisation(false) + .appName("test-sync-update") + .scheduledExecutor(executor) + .unleashAPI("http://localhost:8080") + .toggleBootstrapProvider(toggleBootstrapProvider) + .build(); when(toggleBootstrapProvider.read()).thenReturn(fileToString(file)); when(backupHandler.read()) - .thenReturn( - populatedFeatureCollection( - Arrays.asList( - new Segment( - 1, - "some-name", - Arrays.asList( - new Constraint( - "some-context", - Operator.IN, - "some-value")))), - new FeatureToggle( - "toggleFeatureName1", - true, - Collections.singletonList( - new ActivationStrategy("custom", null))), - new FeatureToggle( - "toggleFeatureName2", - true, - Collections.singletonList( - new ActivationStrategy("custom", null))))); + .thenReturn( + populatedFeatureCollection( + Arrays.asList( + new Segment( + 1, + "some-name", + Arrays.asList( + new Constraint( + "some-context", + Operator.IN, + "some-value")))), + new FeatureToggle( + "toggleFeatureName1", + true, + Collections.singletonList( + new ActivationStrategy("custom", null))), + new FeatureToggle( + "toggleFeatureName2", + true, + Collections.singletonList( + new ActivationStrategy("custom", null))))); new FeatureRepository( - config, backupHandler, new EventDispatcher(config), fetcher, bootstrapHandler); + config, backupHandler, new EventDispatcher(config), fetcher, bootstrapHandler); verify(toggleBootstrapProvider, times(0)).read(); } @Test - public void should_increase_to_max_interval_when_denied() throws URISyntaxException, IOException { + public void should_increase_to_max_interval_when_denied() + throws URISyntaxException, IOException { UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); ArgumentCaptor runnableArgumentCaptor = ArgumentCaptor.forClass(Runnable.class); File file = - new File(getClass().getClassLoader().getResource("unleash-repo-v2.json").toURI()); + new File(getClass().getClassLoader().getResource("unleash-repo-v2.json").toURI()); ToggleBootstrapProvider toggleBootstrapProvider = mock(ToggleBootstrapProvider.class); when(toggleBootstrapProvider.read()).thenReturn(fileToString(file)); UnleashConfig config = - UnleashConfig.builder() - .synchronousFetchOnInitialisation(false) - .appName("test-sync-update") - .scheduledExecutor(executor) - .unleashAPI("http://localhost:8080") - .build(); + UnleashConfig.builder() + .synchronousFetchOnInitialisation(false) + .appName("test-sync-update") + .scheduledExecutor(executor) + .unleashAPI("http://localhost:8080") + .build(); when(backupHandler.read()) - .thenReturn( - populatedFeatureCollection( - Arrays.asList( - new Segment( - 1, - "some-name", - Arrays.asList( - new Constraint( - "some-context", - Operator.IN, - "some-value")))), - new FeatureToggle( - "toggleFeatureName1", - true, - Collections.singletonList( - new ActivationStrategy("custom", null))), - new FeatureToggle( - "toggleFeatureName2", - true, - Collections.singletonList( - new ActivationStrategy("custom", null))))); - when(fetcher.fetchFeatures()).thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.UNAVAILABLE, 403)) - .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)); - - FeatureRepository featureRepository = new FeatureRepository(config, backupHandler, new EventDispatcher(config), fetcher, bootstrapHandler); + .thenReturn( + populatedFeatureCollection( + Arrays.asList( + new Segment( + 1, + "some-name", + Arrays.asList( + new Constraint( + "some-context", + Operator.IN, + "some-value")))), + new FeatureToggle( + "toggleFeatureName1", + true, + Collections.singletonList( + new ActivationStrategy("custom", null))), + new FeatureToggle( + "toggleFeatureName2", + true, + Collections.singletonList( + new ActivationStrategy("custom", null))))); + when(fetcher.fetchFeatures()) + .thenReturn( + new ClientFeaturesResponse(FeatureToggleResponse.Status.UNAVAILABLE, 403)) + .thenReturn( + new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)); + + FeatureRepository featureRepository = + new FeatureRepository( + config, + backupHandler, + new EventDispatcher(config), + fetcher, + bootstrapHandler); verify(executor).setInterval(runnableArgumentCaptor.capture(), anyLong(), anyLong()); runnableArgumentCaptor.getValue().run(); assertThat(featureRepository.getFailures()).isEqualTo(1); @@ -359,46 +359,56 @@ public void should_increase_to_max_interval_when_denied() throws URISyntaxExcept } @Test - public void should_increase_to_max_interval_when_not_found() throws URISyntaxException, IOException { + public void should_increase_to_max_interval_when_not_found() + throws URISyntaxException, IOException { UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); ArgumentCaptor runnableArgumentCaptor = ArgumentCaptor.forClass(Runnable.class); File file = - new File(getClass().getClassLoader().getResource("unleash-repo-v2.json").toURI()); + new File(getClass().getClassLoader().getResource("unleash-repo-v2.json").toURI()); ToggleBootstrapProvider toggleBootstrapProvider = mock(ToggleBootstrapProvider.class); when(toggleBootstrapProvider.read()).thenReturn(fileToString(file)); UnleashConfig config = - UnleashConfig.builder() - .synchronousFetchOnInitialisation(false) - .appName("test-sync-update") - .scheduledExecutor(executor) - .unleashAPI("http://localhost:8080") - .build(); + UnleashConfig.builder() + .synchronousFetchOnInitialisation(false) + .appName("test-sync-update") + .scheduledExecutor(executor) + .unleashAPI("http://localhost:8080") + .build(); when(backupHandler.read()) - .thenReturn( - populatedFeatureCollection( - Arrays.asList( - new Segment( - 1, - "some-name", - Arrays.asList( - new Constraint( - "some-context", - Operator.IN, - "some-value")))), - new FeatureToggle( - "toggleFeatureName1", - true, - Collections.singletonList( - new ActivationStrategy("custom", null))), - new FeatureToggle( - "toggleFeatureName2", - true, - Collections.singletonList( - new ActivationStrategy("custom", null))))); - when(fetcher.fetchFeatures()).thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.UNAVAILABLE, 404)) - .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)); - - FeatureRepository featureRepository = new FeatureRepository(config, backupHandler, new EventDispatcher(config), fetcher, bootstrapHandler); + .thenReturn( + populatedFeatureCollection( + Arrays.asList( + new Segment( + 1, + "some-name", + Arrays.asList( + new Constraint( + "some-context", + Operator.IN, + "some-value")))), + new FeatureToggle( + "toggleFeatureName1", + true, + Collections.singletonList( + new ActivationStrategy("custom", null))), + new FeatureToggle( + "toggleFeatureName2", + true, + Collections.singletonList( + new ActivationStrategy("custom", null))))); + when(fetcher.fetchFeatures()) + .thenReturn( + new ClientFeaturesResponse(FeatureToggleResponse.Status.UNAVAILABLE, 404)) + .thenReturn( + new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)); + + FeatureRepository featureRepository = + new FeatureRepository( + config, + backupHandler, + new EventDispatcher(config), + fetcher, + bootstrapHandler); verify(executor).setInterval(runnableArgumentCaptor.capture(), anyLong(), anyLong()); runnableArgumentCaptor.getValue().run(); assertThat(featureRepository.getFailures()).isEqualTo(1); @@ -412,53 +422,68 @@ public void should_increase_to_max_interval_when_not_found() throws URISyntaxExc assertThat(featureRepository.getFailures()).isEqualTo(0); assertThat(featureRepository.getInterval()).isEqualTo(0); } + @Test - public void should_incrementally_increase_interval_as_we_receive_too_many_requests() throws URISyntaxException, IOException { + public void should_incrementally_increase_interval_as_we_receive_too_many_requests() + throws URISyntaxException, IOException { UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); ArgumentCaptor runnableArgumentCaptor = ArgumentCaptor.forClass(Runnable.class); File file = - new File(getClass().getClassLoader().getResource("unleash-repo-v2.json").toURI()); + new File(getClass().getClassLoader().getResource("unleash-repo-v2.json").toURI()); ToggleBootstrapProvider toggleBootstrapProvider = mock(ToggleBootstrapProvider.class); when(toggleBootstrapProvider.read()).thenReturn(fileToString(file)); UnleashConfig config = - UnleashConfig.builder() - .synchronousFetchOnInitialisation(false) - .appName("test-sync-update") - .scheduledExecutor(executor) - .unleashAPI("http://localhost:8080") - .build(); + UnleashConfig.builder() + .synchronousFetchOnInitialisation(false) + .appName("test-sync-update") + .scheduledExecutor(executor) + .unleashAPI("http://localhost:8080") + .build(); when(backupHandler.read()) - .thenReturn( - populatedFeatureCollection( - Arrays.asList( - new Segment( - 1, - "some-name", - Arrays.asList( - new Constraint( - "some-context", - Operator.IN, - "some-value")))), - new FeatureToggle( - "toggleFeatureName1", - true, - Collections.singletonList( - new ActivationStrategy("custom", null))), - new FeatureToggle( - "toggleFeatureName2", - true, - Collections.singletonList( - new ActivationStrategy("custom", null))))); - FeatureRepository featureRepository = new FeatureRepository(config, backupHandler, new EventDispatcher(config), fetcher, bootstrapHandler); + .thenReturn( + populatedFeatureCollection( + Arrays.asList( + new Segment( + 1, + "some-name", + Arrays.asList( + new Constraint( + "some-context", + Operator.IN, + "some-value")))), + new FeatureToggle( + "toggleFeatureName1", + true, + Collections.singletonList( + new ActivationStrategy("custom", null))), + new FeatureToggle( + "toggleFeatureName2", + true, + Collections.singletonList( + new ActivationStrategy("custom", null))))); + FeatureRepository featureRepository = + new FeatureRepository( + config, + backupHandler, + new EventDispatcher(config), + fetcher, + bootstrapHandler); verify(executor).setInterval(runnableArgumentCaptor.capture(), anyLong(), anyLong()); when(fetcher.fetchFeatures()) - .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.UNAVAILABLE, 429)) - .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.UNAVAILABLE, 429)) - .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.UNAVAILABLE, 429)) - .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)) - .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)) - .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)) - .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)); + .thenReturn( + new ClientFeaturesResponse(FeatureToggleResponse.Status.UNAVAILABLE, 429)) + .thenReturn( + new ClientFeaturesResponse(FeatureToggleResponse.Status.UNAVAILABLE, 429)) + .thenReturn( + new ClientFeaturesResponse(FeatureToggleResponse.Status.UNAVAILABLE, 429)) + .thenReturn( + new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)) + .thenReturn( + new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)) + .thenReturn( + new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)) + .thenReturn( + new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)); runnableArgumentCaptor.getValue().run(); assertThat(featureRepository.getInterval()).isEqualTo(1); assertThat(featureRepository.getFailures()).isEqualTo(1); @@ -499,52 +524,66 @@ public void should_incrementally_increase_interval_as_we_receive_too_many_reques } @Test - public void server_errors_should_incrementally_increase_interval() throws URISyntaxException, IOException { + public void server_errors_should_incrementally_increase_interval() + throws URISyntaxException, IOException { UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class); ArgumentCaptor runnableArgumentCaptor = ArgumentCaptor.forClass(Runnable.class); File file = - new File(getClass().getClassLoader().getResource("unleash-repo-v2.json").toURI()); + new File(getClass().getClassLoader().getResource("unleash-repo-v2.json").toURI()); ToggleBootstrapProvider toggleBootstrapProvider = mock(ToggleBootstrapProvider.class); when(toggleBootstrapProvider.read()).thenReturn(fileToString(file)); UnleashConfig config = - UnleashConfig.builder() - .synchronousFetchOnInitialisation(false) - .appName("test-sync-update") - .scheduledExecutor(executor) - .unleashAPI("http://localhost:8080") - .build(); + UnleashConfig.builder() + .synchronousFetchOnInitialisation(false) + .appName("test-sync-update") + .scheduledExecutor(executor) + .unleashAPI("http://localhost:8080") + .build(); when(backupHandler.read()) - .thenReturn( - populatedFeatureCollection( - Arrays.asList( - new Segment( - 1, - "some-name", - Arrays.asList( - new Constraint( - "some-context", - Operator.IN, - "some-value")))), - new FeatureToggle( - "toggleFeatureName1", - true, - Collections.singletonList( - new ActivationStrategy("custom", null))), - new FeatureToggle( - "toggleFeatureName2", - true, - Collections.singletonList( - new ActivationStrategy("custom", null))))); - FeatureRepository featureRepository = new FeatureRepository(config, backupHandler, new EventDispatcher(config), fetcher, bootstrapHandler); + .thenReturn( + populatedFeatureCollection( + Arrays.asList( + new Segment( + 1, + "some-name", + Arrays.asList( + new Constraint( + "some-context", + Operator.IN, + "some-value")))), + new FeatureToggle( + "toggleFeatureName1", + true, + Collections.singletonList( + new ActivationStrategy("custom", null))), + new FeatureToggle( + "toggleFeatureName2", + true, + Collections.singletonList( + new ActivationStrategy("custom", null))))); + FeatureRepository featureRepository = + new FeatureRepository( + config, + backupHandler, + new EventDispatcher(config), + fetcher, + bootstrapHandler); verify(executor).setInterval(runnableArgumentCaptor.capture(), anyLong(), anyLong()); when(fetcher.fetchFeatures()) - .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.UNAVAILABLE, 500)) - .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.UNAVAILABLE, 502)) - .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.UNAVAILABLE, 503)) - .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)) - .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)) - .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)) - .thenReturn(new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)); + .thenReturn( + new ClientFeaturesResponse(FeatureToggleResponse.Status.UNAVAILABLE, 500)) + .thenReturn( + new ClientFeaturesResponse(FeatureToggleResponse.Status.UNAVAILABLE, 502)) + .thenReturn( + new ClientFeaturesResponse(FeatureToggleResponse.Status.UNAVAILABLE, 503)) + .thenReturn( + new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)) + .thenReturn( + new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)) + .thenReturn( + new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)) + .thenReturn( + new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)); runnableArgumentCaptor.getValue().run(); assertThat(featureRepository.getInterval()).isEqualTo(1); assertThat(featureRepository.getFailures()).isEqualTo(1); @@ -587,5 +626,4 @@ public void server_errors_should_incrementally_increase_interval() throws URISyn private String fileToString(File f) throws IOException { return new String(Files.readAllBytes(f.toPath()), StandardCharsets.UTF_8); } - } diff --git a/src/test/java/io/getunleash/variant/VariantUtilTest.java b/src/test/java/io/getunleash/variant/VariantUtilTest.java index ea4e5a4ab..ee431edb6 100644 --- a/src/test/java/io/getunleash/variant/VariantUtilTest.java +++ b/src/test/java/io/getunleash/variant/VariantUtilTest.java @@ -5,7 +5,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; -import com.sangupta.murmur.Murmur3; import io.getunleash.ActivationStrategy; import io.getunleash.FeatureToggle; import io.getunleash.UnleashContext; @@ -411,6 +410,4 @@ public void feature_variants_variant_d_with_override_client_spec_tests() { toggle, UnleashContext.builder().userId("10").build(), DISABLED_VARIANT); assertThat(variantUser10.getName()).isEqualTo("variant2"); } - - } From f50261e2d68df6d2ccdd52c8776b2395fb9a41f3 Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Tue, 7 Nov 2023 14:53:43 +0100 Subject: [PATCH 04/15] spotless tweaks --- src/main/java/io/getunleash/util/ConstraintMerger.java | 3 ++- src/test/java/io/getunleash/util/IpAddressMatcherTest.java | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/getunleash/util/ConstraintMerger.java b/src/main/java/io/getunleash/util/ConstraintMerger.java index 28db759e2..7440decb7 100644 --- a/src/main/java/io/getunleash/util/ConstraintMerger.java +++ b/src/main/java/io/getunleash/util/ConstraintMerger.java @@ -17,7 +17,8 @@ public static List mergeConstraints( Optional.ofNullable(strategy.getConstraints()) .orElseGet(Collections::emptyList), Optional.ofNullable(strategy.getSegments()) - .orElseGet(Collections::emptyList).stream() + .orElseGet(Collections::emptyList) + .stream() .map(repository::getSegment) .map(s -> s == null ? DENY_SEGMENT : s) .map(Segment::getConstraints) diff --git a/src/test/java/io/getunleash/util/IpAddressMatcherTest.java b/src/test/java/io/getunleash/util/IpAddressMatcherTest.java index f9d48f37c..8b2bf5cba 100644 --- a/src/test/java/io/getunleash/util/IpAddressMatcherTest.java +++ b/src/test/java/io/getunleash/util/IpAddressMatcherTest.java @@ -19,7 +19,9 @@ import org.junit.jupiter.api.Test; -/** @author Luke Taylor */ +/** + * @author Luke Taylor + */ class IpAddressMatcherTest { private final IpAddressMatcher v6matcher = new IpAddressMatcher("fe80::21f:5bff:fe33:bd68"); private final IpAddressMatcher v4matcher = new IpAddressMatcher("192.168.1.104"); From f0420f15e3a787104b626a335ffff00464bcf05c Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Tue, 7 Nov 2023 14:56:19 +0100 Subject: [PATCH 05/15] checkout to v4 --- .github/workflows/pull_requests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pull_requests.yml b/.github/workflows/pull_requests.yml index 8dda5dc49..8713d09b8 100644 --- a/.github/workflows/pull_requests.yml +++ b/.github/workflows/pull_requests.yml @@ -10,7 +10,7 @@ jobs: version: [8,11,17] steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Setup Java uses: actions/setup-java@v3 with: From 4ec1b9a06694f6a171c3044d3176e7d258f341d0 Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Tue, 7 Nov 2023 15:19:21 +0100 Subject: [PATCH 06/15] Make tests less verbose again --- src/test/resources/logback-test.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/resources/logback-test.xml b/src/test/resources/logback-test.xml index a265cac70..5b57a78a4 100644 --- a/src/test/resources/logback-test.xml +++ b/src/test/resources/logback-test.xml @@ -11,6 +11,6 @@ - + From 441a06291e9d14ef948317ec79bb9edcdc6026cf Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Wed, 8 Nov 2023 09:04:02 +0100 Subject: [PATCH 07/15] Update src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Gastón Fournier --- .../java/io/getunleash/metric/UnleashMetricServiceImpl.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java b/src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java index d9972a087..79869829f 100644 --- a/src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java +++ b/src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java @@ -21,8 +21,8 @@ public class UnleashMetricServiceImpl implements UnleashMetricService { private volatile MetricsBucket currentMetricsBucket; private final int maxInterval; - private AtomicInteger failures = new AtomicInteger(); - private AtomicInteger interval = new AtomicInteger(); + private final AtomicInteger failures = new AtomicInteger(); + private final AtomicInteger interval = new AtomicInteger(); public UnleashMetricServiceImpl( UnleashConfig unleashConfig, UnleashScheduledExecutor executor) { From ee082783c5c202d5213dd68ececed543db2c2155 Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Wed, 8 Nov 2023 09:04:13 +0100 Subject: [PATCH 08/15] Update src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Gastón Fournier --- .../java/io/getunleash/metric/UnleashMetricServiceImpl.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java b/src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java index 79869829f..e73d797dd 100644 --- a/src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java +++ b/src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java @@ -42,8 +42,7 @@ public UnleashMetricServiceImpl( 20, 300 / Integer.max( - Long.valueOf(unleashConfig.getSendMetricsInterval()) - .intValue(), + (int) unleashConfig.getSendMetricsInterval(), 1)); long metricsInterval = unleashConfig.getSendMetricsInterval(); executor.setInterval(sendMetrics(), metricsInterval, metricsInterval); From 8e4790d9ea4ac4230bad4c43bfa1875e313c4c3f Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Wed, 8 Nov 2023 09:04:20 +0100 Subject: [PATCH 09/15] Update src/main/java/io/getunleash/repository/FeatureRepository.java MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Gastón Fournier --- src/main/java/io/getunleash/repository/FeatureRepository.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/io/getunleash/repository/FeatureRepository.java b/src/main/java/io/getunleash/repository/FeatureRepository.java index b81338349..28b82778e 100644 --- a/src/main/java/io/getunleash/repository/FeatureRepository.java +++ b/src/main/java/io/getunleash/repository/FeatureRepository.java @@ -122,7 +122,6 @@ private void initCollections(UnleashScheduledExecutor executor) { } } - // private Runnable updateFeatures(@Nullable final Consumer handler) { return () -> updateFeaturesInternal(handler); } From dd6d4a69c612927b5c619675438b0a8c87882e34 Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Wed, 8 Nov 2023 09:43:08 +0100 Subject: [PATCH 10/15] fix: make backoff logic easier to understand --- .../metric/UnleashMetricServiceImpl.java | 71 +++++++++----- .../repository/FeatureRepository.java | 94 ++++++++++--------- .../metric/UnleashMetricServiceImplTest.java | 56 +++++------ .../repository/FeatureRepositoryTest.java | 57 ++++++----- 4 files changed, 152 insertions(+), 126 deletions(-) diff --git a/src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java b/src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java index e73d797dd..ecdca673f 100644 --- a/src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java +++ b/src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java @@ -20,9 +20,9 @@ public class UnleashMetricServiceImpl implements UnleashMetricService { // mutable private volatile MetricsBucket currentMetricsBucket; - private final int maxInterval; + private final int maxSkips; private final AtomicInteger failures = new AtomicInteger(); - private final AtomicInteger interval = new AtomicInteger(); + private final AtomicInteger skips = new AtomicInteger(); public UnleashMetricServiceImpl( UnleashConfig unleashConfig, UnleashScheduledExecutor executor) { @@ -37,13 +37,8 @@ public UnleashMetricServiceImpl( this.started = LocalDateTime.now(ZoneId.of("UTC")); this.unleashConfig = unleashConfig; this.metricSender = metricSender; - this.maxInterval = - Integer.max( - 20, - 300 - / Integer.max( - (int) unleashConfig.getSendMetricsInterval(), - 1)); + this.maxSkips = + Integer.max(20, 300 / Integer.max((int) unleashConfig.getSendMetricsInterval(), 1)); long metricsInterval = unleashConfig.getSendMetricsInterval(); executor.setInterval(sendMetrics(), metricsInterval, metricsInterval); } @@ -67,59 +62,83 @@ public void countVariant(String toggleName, String variantName) { private Runnable sendMetrics() { return () -> { - if (interval.get() == 0) { + if (skips.get() == 0) { MetricsBucket metricsBucket = this.currentMetricsBucket; this.currentMetricsBucket = new MetricsBucket(); metricsBucket.end(); ClientMetrics metrics = new ClientMetrics(unleashConfig, metricsBucket); int statusCode = metricSender.sendMetrics(metrics); if (statusCode >= 200 && statusCode < 400) { - if (failures.get() > 0) { - interval.set(Integer.max(failures.decrementAndGet(), 0)); - } + decrementFailureCountAndResetSkips(); } if (statusCode >= 400) { handleHttpErrorCodes(statusCode); } } else { - interval.decrementAndGet(); + skips.decrementAndGet(); } }; } + /** + * We've had one successful call, so if we had 10 failures in a row, this will reduce the skips + * down to 9, so that we gradually start polling more often, instead of doing max load + * immediately after a sequence of errors. + */ + private void decrementFailureCountAndResetSkips() { + skips.set(Math.max(failures.decrementAndGet(), 0)); + } + + /** + * We've gotten the message to back off (usually a 429 or a 50x). If we have successive + * failures, failure count here will be incremented higher and higher which will handle + * increasing our backoff, since we set the skip count to the failure count after every reset + */ + private void increaseSkipCount() { + skips.set(Math.min(failures.incrementAndGet(), maxSkips)); + } + + /** + * We've received an error code that we don't expect to change, which means we've already logged + * an ERROR. To avoid hammering the server that just told us we did something wrong and to avoid + * flooding the logs, we'll increase our skip count to maximum + */ + private void maximizeSkips() { + skips.set(maxSkips); + failures.incrementAndGet(); + } + private void handleHttpErrorCodes(int responseCode) { if (responseCode == 404) { - interval.set(maxInterval); - failures.incrementAndGet(); + maximizeSkips(); LOGGER.error( "Server said that the Metrics receiving endpoint at {} does not exist. Backing off to {} times our poll interval to avoid overloading server", unleashConfig.getUnleashURLs().getClientMetricsURL(), - maxInterval); + maxSkips); } else if (responseCode == 429) { - interval.set(Math.min(failures.incrementAndGet(), maxInterval)); + increaseSkipCount(); LOGGER.info( "Client Metrics was RATE LIMITED for the {}. time. Further backing off. Current backoff at {} times our metrics post interval", failures.get(), - interval.get()); + skips.get()); } else if (responseCode == HttpURLConnection.HTTP_UNAUTHORIZED || responseCode == HttpURLConnection.HTTP_FORBIDDEN) { - failures.incrementAndGet(); - interval.set(maxInterval); + maximizeSkips(); LOGGER.error( "Client was not authorized to post metrics to the Unleash API at {}. Backing off to {} times our poll interval to avoid overloading server", unleashConfig.getUnleashURLs().getClientMetricsURL(), - maxInterval); + maxSkips); } else if (responseCode >= 500) { - interval.set(Math.min(failures.incrementAndGet(), maxInterval)); + increaseSkipCount(); LOGGER.info( "Server failed with a {} status code. Backing off. Current backoff at {} times our poll interval", responseCode, - interval.get()); + skips.get()); } } - protected int getInterval() { - return this.interval.get(); + protected int getSkips() { + return this.skips.get(); } protected int getFailures() { diff --git a/src/main/java/io/getunleash/repository/FeatureRepository.java b/src/main/java/io/getunleash/repository/FeatureRepository.java index 28b82778e..9efaabdcd 100644 --- a/src/main/java/io/getunleash/repository/FeatureRepository.java +++ b/src/main/java/io/getunleash/repository/FeatureRepository.java @@ -29,8 +29,8 @@ public class FeatureRepository implements IFeatureRepository { private boolean ready; private AtomicInteger failures = new AtomicInteger(0); - private AtomicInteger interval = new AtomicInteger(0); - private final Integer maxInterval; + private AtomicInteger skips = new AtomicInteger(0); + private final Integer maxSkips; public FeatureRepository(UnleashConfig unleashConfig) { this(unleashConfig, new FeatureBackupHandlerFile(unleashConfig)); @@ -44,15 +44,7 @@ public FeatureRepository( this.featureFetcher = unleashConfig.getUnleashFeatureFetcherFactory().apply(unleashConfig); this.featureBootstrapHandler = new FeatureBootstrapHandler(unleashConfig); this.eventDispatcher = new EventDispatcher(unleashConfig); - this.maxInterval = - Integer.max( - 20, - 300 - / Integer.max( - Long.valueOf(unleashConfig.getFetchTogglesInterval()) - .intValue(), - 1)); - + this.maxSkips = this.calculateMaxSkips((int) unleashConfig.getFetchTogglesInterval()); this.initCollections(unleashConfig.getScheduledExecutor()); } @@ -68,14 +60,7 @@ protected FeatureRepository( this.featureFetcher = featureFetcher; this.featureBootstrapHandler = featureBootstrapHandler; this.eventDispatcher = eventDispatcher; - this.maxInterval = - Integer.max( - 20, - 300 - / Integer.max( - Long.valueOf(unleashConfig.getFetchTogglesInterval()) - .intValue(), - 1)); + this.maxSkips = this.calculateMaxSkips((int) unleashConfig.getFetchTogglesInterval()); this.initCollections(unleashConfig.getScheduledExecutor()); } @@ -90,14 +75,7 @@ protected FeatureRepository( this.featureFetcher = featureFetcher; this.featureBootstrapHandler = featureBootstrapHandler; this.eventDispatcher = new EventDispatcher(unleashConfig); - this.maxInterval = - Integer.max( - 20, - 300 - / Integer.max( - Long.valueOf(unleashConfig.getFetchTogglesInterval()) - .intValue(), - 1)); + this.maxSkips = this.calculateMaxSkips((int) unleashConfig.getFetchTogglesInterval()); this.initCollections(executor); } @@ -122,12 +100,16 @@ private void initCollections(UnleashScheduledExecutor executor) { } } + private Integer calculateMaxSkips(int fetchTogglesInterval) { + return Integer.max(20, 300 / Integer.max(fetchTogglesInterval, 1)); + } + private Runnable updateFeatures(@Nullable final Consumer handler) { return () -> updateFeaturesInternal(handler); } private void updateFeaturesInternal(@Nullable final Consumer handler) { - if (interval.get() <= 0L) { + if (skips.get() <= 0L) { try { ClientFeaturesResponse response = featureFetcher.fetchFeatures(); eventDispatcher.dispatch(response); @@ -145,7 +127,7 @@ private void updateFeaturesInternal(@Nullable final Consumer h handleHttpErrorCodes(response.getHttpStatusCode()); return; } - interval.set(Math.max(failures.decrementAndGet(), 0)); + decrementFailureCountAndResetSkips(); if (!ready) { eventDispatcher.dispatch(new UnleashReady()); ready = true; @@ -158,38 +140,64 @@ private void updateFeaturesInternal(@Nullable final Consumer h } } } else { - interval.decrementAndGet(); + skips.decrementAndGet(); // We didn't do anything this iteration, just reduce the count } } + /** + * We've had one successful call, so if we had 10 failures in a row, this will reduce the skips + * down to 9, so that we gradually start polling more often, instead of doing max load + * immediately after a sequence of errors. + */ + private void decrementFailureCountAndResetSkips() { + skips.set(Math.max(failures.decrementAndGet(), 0)); + } + + /** + * We've gotten the message to back off (usually a 429 or a 50x). If we have successive + * failures, failure count here will be incremented higher and higher which will handle + * increasing our backoff, since we set the skip count to the failure count after every reset + */ + private void increaseSkipCount() { + skips.set(Math.min(failures.incrementAndGet(), maxSkips)); + } + + /** + * We've received an error code that we don't expect to change, which means we've already logged + * an ERROR. To avoid hammering the server that just told us we did something wrong and to avoid + * flooding the logs, we'll increase our skip count to maximum + */ + private void maximizeSkips() { + skips.set(maxSkips); + failures.incrementAndGet(); + } + private void handleHttpErrorCodes(int responseCode) { if (responseCode == 404) { - interval.set(maxInterval); - failures.incrementAndGet(); + maximizeSkips(); LOGGER.error( "Server said that the API at {} does not exist. Backing off to {} times our poll interval to avoid overloading server", unleashConfig.getUnleashAPI(), - maxInterval); + maxSkips); } else if (responseCode == 429) { - interval.set(Math.min(failures.incrementAndGet(), maxInterval)); + increaseSkipCount(); LOGGER.info( - "Client was RATE LIMITED for the {} time. Further backing off. Current backoff at {} times our poll interval", + "Client was RATE LIMITED for the {}. time. Further backing off. Current backoff at {} times our poll interval", failures.get(), - interval.get()); + skips.get()); } else if (responseCode == HttpURLConnection.HTTP_UNAUTHORIZED || responseCode == HttpURLConnection.HTTP_FORBIDDEN) { - failures.incrementAndGet(); - interval.set(maxInterval); + maximizeSkips(); LOGGER.error( "Client failed to authenticate to the Unleash API at {}. Backing off to {} times our poll interval to avoid overloading server", unleashConfig.getUnleashAPI(), - maxInterval); + maxSkips); } else if (responseCode >= 500) { - interval.set(Math.min(failures.incrementAndGet(), maxInterval)); + increaseSkipCount(); LOGGER.info( "Server failed with a {} status code. Backing off. Current backoff at {} times our poll interval", responseCode, - interval.get()); + skips.get()); } } @@ -214,7 +222,7 @@ public Integer getFailures() { return failures.get(); } - public Integer getInterval() { - return interval.get(); + public Integer getSkips() { + return skips.get(); } } diff --git a/src/test/java/io/getunleash/metric/UnleashMetricServiceImplTest.java b/src/test/java/io/getunleash/metric/UnleashMetricServiceImplTest.java index 975a015a4..1640ce03b 100644 --- a/src/test/java/io/getunleash/metric/UnleashMetricServiceImplTest.java +++ b/src/test/java/io/getunleash/metric/UnleashMetricServiceImplTest.java @@ -231,41 +231,41 @@ public void should_backoff_when_told_to_by_429_code() { .thenReturn(200); sendMetricsCallback.getValue().run(); - assertThat(unleashMetricService.getInterval()).isEqualTo(1); + assertThat(unleashMetricService.getSkips()).isEqualTo(1); assertThat(unleashMetricService.getFailures()).isEqualTo(1); sendMetricsCallback.getValue().run(); - assertThat(unleashMetricService.getInterval()).isEqualTo(0); + assertThat(unleashMetricService.getSkips()).isEqualTo(0); assertThat(unleashMetricService.getFailures()).isEqualTo(1); sendMetricsCallback.getValue().run(); - assertThat(unleashMetricService.getInterval()).isEqualTo(2); + assertThat(unleashMetricService.getSkips()).isEqualTo(2); assertThat(unleashMetricService.getFailures()).isEqualTo(2); sendMetricsCallback.getValue().run(); // NO-OP because interval > 0 sendMetricsCallback.getValue().run(); // NO-OP because interval > 0 - assertThat(unleashMetricService.getInterval()).isEqualTo(0); + assertThat(unleashMetricService.getSkips()).isEqualTo(0); assertThat(unleashMetricService.getFailures()).isEqualTo(2); sendMetricsCallback.getValue().run(); - assertThat(unleashMetricService.getInterval()).isEqualTo(3); + assertThat(unleashMetricService.getSkips()).isEqualTo(3); assertThat(unleashMetricService.getFailures()).isEqualTo(3); sendMetricsCallback.getValue().run(); sendMetricsCallback.getValue().run(); sendMetricsCallback.getValue().run(); - assertThat(unleashMetricService.getInterval()).isEqualTo(0); + assertThat(unleashMetricService.getSkips()).isEqualTo(0); assertThat(unleashMetricService.getFailures()).isEqualTo(3); sendMetricsCallback.getValue().run(); - assertThat(unleashMetricService.getInterval()).isEqualTo(2); + assertThat(unleashMetricService.getSkips()).isEqualTo(2); assertThat(unleashMetricService.getFailures()).isEqualTo(2); sendMetricsCallback.getValue().run(); sendMetricsCallback.getValue().run(); - assertThat(unleashMetricService.getInterval()).isEqualTo(0); + assertThat(unleashMetricService.getSkips()).isEqualTo(0); assertThat(unleashMetricService.getFailures()).isEqualTo(2); sendMetricsCallback.getValue().run(); - assertThat(unleashMetricService.getInterval()).isEqualTo(1); + assertThat(unleashMetricService.getSkips()).isEqualTo(1); assertThat(unleashMetricService.getFailures()).isEqualTo(1); sendMetricsCallback.getValue().run(); - assertThat(unleashMetricService.getInterval()).isEqualTo(0); + assertThat(unleashMetricService.getSkips()).isEqualTo(0); assertThat(unleashMetricService.getFailures()).isEqualTo(1); sendMetricsCallback.getValue().run(); - assertThat(unleashMetricService.getInterval()).isEqualTo(0); + assertThat(unleashMetricService.getSkips()).isEqualTo(0); assertThat(unleashMetricService.getFailures()).isEqualTo(0); } @@ -301,41 +301,41 @@ public void server_errors_should_also_incrementally_backoff() { .thenReturn(304) .thenReturn(304); sendMetricsCallback.getValue().run(); - assertThat(unleashMetricService.getInterval()).isEqualTo(1); + assertThat(unleashMetricService.getSkips()).isEqualTo(1); assertThat(unleashMetricService.getFailures()).isEqualTo(1); sendMetricsCallback.getValue().run(); - assertThat(unleashMetricService.getInterval()).isEqualTo(0); + assertThat(unleashMetricService.getSkips()).isEqualTo(0); assertThat(unleashMetricService.getFailures()).isEqualTo(1); sendMetricsCallback.getValue().run(); - assertThat(unleashMetricService.getInterval()).isEqualTo(2); + assertThat(unleashMetricService.getSkips()).isEqualTo(2); assertThat(unleashMetricService.getFailures()).isEqualTo(2); sendMetricsCallback.getValue().run(); // NO-OP because interval > 0 sendMetricsCallback.getValue().run(); // NO-OP because interval > 0 - assertThat(unleashMetricService.getInterval()).isEqualTo(0); + assertThat(unleashMetricService.getSkips()).isEqualTo(0); assertThat(unleashMetricService.getFailures()).isEqualTo(2); sendMetricsCallback.getValue().run(); - assertThat(unleashMetricService.getInterval()).isEqualTo(3); + assertThat(unleashMetricService.getSkips()).isEqualTo(3); assertThat(unleashMetricService.getFailures()).isEqualTo(3); sendMetricsCallback.getValue().run(); sendMetricsCallback.getValue().run(); sendMetricsCallback.getValue().run(); - assertThat(unleashMetricService.getInterval()).isEqualTo(0); + assertThat(unleashMetricService.getSkips()).isEqualTo(0); assertThat(unleashMetricService.getFailures()).isEqualTo(3); sendMetricsCallback.getValue().run(); - assertThat(unleashMetricService.getInterval()).isEqualTo(2); + assertThat(unleashMetricService.getSkips()).isEqualTo(2); assertThat(unleashMetricService.getFailures()).isEqualTo(2); sendMetricsCallback.getValue().run(); sendMetricsCallback.getValue().run(); - assertThat(unleashMetricService.getInterval()).isEqualTo(0); + assertThat(unleashMetricService.getSkips()).isEqualTo(0); assertThat(unleashMetricService.getFailures()).isEqualTo(2); sendMetricsCallback.getValue().run(); - assertThat(unleashMetricService.getInterval()).isEqualTo(1); + assertThat(unleashMetricService.getSkips()).isEqualTo(1); assertThat(unleashMetricService.getFailures()).isEqualTo(1); sendMetricsCallback.getValue().run(); - assertThat(unleashMetricService.getInterval()).isEqualTo(0); + assertThat(unleashMetricService.getSkips()).isEqualTo(0); assertThat(unleashMetricService.getFailures()).isEqualTo(1); sendMetricsCallback.getValue().run(); - assertThat(unleashMetricService.getInterval()).isEqualTo(0); + assertThat(unleashMetricService.getSkips()).isEqualTo(0); assertThat(unleashMetricService.getFailures()).isEqualTo(0); } @@ -364,16 +364,16 @@ public void failure_to_authenticate_immediately_increases_interval_to_max() { verify(executor).setInterval(sendMetricsCallback.capture(), anyLong(), anyLong()); when(sender.sendMetrics(any(ClientMetrics.class))).thenReturn(403).thenReturn(200); sendMetricsCallback.getValue().run(); - assertThat(unleashMetricService.getInterval()).isEqualTo(30); + assertThat(unleashMetricService.getSkips()).isEqualTo(30); assertThat(unleashMetricService.getFailures()).isEqualTo(1); for (int i = 0; i < 30; i++) { sendMetricsCallback.getValue().run(); } assertThat(unleashMetricService.getFailures()).isEqualTo(1); - assertThat(unleashMetricService.getInterval()).isEqualTo(0); + assertThat(unleashMetricService.getSkips()).isEqualTo(0); sendMetricsCallback.getValue().run(); assertThat(unleashMetricService.getFailures()).isEqualTo(0); - assertThat(unleashMetricService.getInterval()).isEqualTo(0); + assertThat(unleashMetricService.getSkips()).isEqualTo(0); } @Test @@ -401,15 +401,15 @@ public void url_not_found_immediately_increases_interval_to_max() { verify(executor).setInterval(sendMetricsCallback.capture(), anyLong(), anyLong()); when(sender.sendMetrics(any(ClientMetrics.class))).thenReturn(404).thenReturn(200); sendMetricsCallback.getValue().run(); - assertThat(unleashMetricService.getInterval()).isEqualTo(30); + assertThat(unleashMetricService.getSkips()).isEqualTo(30); assertThat(unleashMetricService.getFailures()).isEqualTo(1); for (int i = 0; i < 30; i++) { sendMetricsCallback.getValue().run(); } assertThat(unleashMetricService.getFailures()).isEqualTo(1); - assertThat(unleashMetricService.getInterval()).isEqualTo(0); + assertThat(unleashMetricService.getSkips()).isEqualTo(0); sendMetricsCallback.getValue().run(); assertThat(unleashMetricService.getFailures()).isEqualTo(0); - assertThat(unleashMetricService.getInterval()).isEqualTo(0); + assertThat(unleashMetricService.getSkips()).isEqualTo(0); } } diff --git a/src/test/java/io/getunleash/repository/FeatureRepositoryTest.java b/src/test/java/io/getunleash/repository/FeatureRepositoryTest.java index 3a0afd6ff..ae3615133 100644 --- a/src/test/java/io/getunleash/repository/FeatureRepositoryTest.java +++ b/src/test/java/io/getunleash/repository/FeatureRepositoryTest.java @@ -18,7 +18,6 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; -import java.util.concurrent.*; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; @@ -347,15 +346,15 @@ public void should_increase_to_max_interval_when_denied() verify(executor).setInterval(runnableArgumentCaptor.capture(), anyLong(), anyLong()); runnableArgumentCaptor.getValue().run(); assertThat(featureRepository.getFailures()).isEqualTo(1); - assertThat(featureRepository.getInterval()).isEqualTo(30); + assertThat(featureRepository.getSkips()).isEqualTo(30); for (int i = 0; i < 30; i++) { runnableArgumentCaptor.getValue().run(); } assertThat(featureRepository.getFailures()).isEqualTo(1); - assertThat(featureRepository.getInterval()).isEqualTo(0); + assertThat(featureRepository.getSkips()).isEqualTo(0); runnableArgumentCaptor.getValue().run(); assertThat(featureRepository.getFailures()).isEqualTo(0); - assertThat(featureRepository.getInterval()).isEqualTo(0); + assertThat(featureRepository.getSkips()).isEqualTo(0); } @Test @@ -412,15 +411,15 @@ public void should_increase_to_max_interval_when_not_found() verify(executor).setInterval(runnableArgumentCaptor.capture(), anyLong(), anyLong()); runnableArgumentCaptor.getValue().run(); assertThat(featureRepository.getFailures()).isEqualTo(1); - assertThat(featureRepository.getInterval()).isEqualTo(30); + assertThat(featureRepository.getSkips()).isEqualTo(30); for (int i = 0; i < 30; i++) { runnableArgumentCaptor.getValue().run(); } assertThat(featureRepository.getFailures()).isEqualTo(1); - assertThat(featureRepository.getInterval()).isEqualTo(0); + assertThat(featureRepository.getSkips()).isEqualTo(0); runnableArgumentCaptor.getValue().run(); assertThat(featureRepository.getFailures()).isEqualTo(0); - assertThat(featureRepository.getInterval()).isEqualTo(0); + assertThat(featureRepository.getSkips()).isEqualTo(0); } @Test @@ -485,41 +484,41 @@ public void should_incrementally_increase_interval_as_we_receive_too_many_reques .thenReturn( new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)); runnableArgumentCaptor.getValue().run(); - assertThat(featureRepository.getInterval()).isEqualTo(1); + assertThat(featureRepository.getSkips()).isEqualTo(1); assertThat(featureRepository.getFailures()).isEqualTo(1); runnableArgumentCaptor.getValue().run(); - assertThat(featureRepository.getInterval()).isEqualTo(0); + assertThat(featureRepository.getSkips()).isEqualTo(0); assertThat(featureRepository.getFailures()).isEqualTo(1); runnableArgumentCaptor.getValue().run(); - assertThat(featureRepository.getInterval()).isEqualTo(2); + assertThat(featureRepository.getSkips()).isEqualTo(2); assertThat(featureRepository.getFailures()).isEqualTo(2); runnableArgumentCaptor.getValue().run(); // NO-OP because interval > 0 runnableArgumentCaptor.getValue().run(); // NO-OP because interval > 0 - assertThat(featureRepository.getInterval()).isEqualTo(0); + assertThat(featureRepository.getSkips()).isEqualTo(0); assertThat(featureRepository.getFailures()).isEqualTo(2); runnableArgumentCaptor.getValue().run(); - assertThat(featureRepository.getInterval()).isEqualTo(3); + assertThat(featureRepository.getSkips()).isEqualTo(3); assertThat(featureRepository.getFailures()).isEqualTo(3); runnableArgumentCaptor.getValue().run(); runnableArgumentCaptor.getValue().run(); runnableArgumentCaptor.getValue().run(); - assertThat(featureRepository.getInterval()).isEqualTo(0); + assertThat(featureRepository.getSkips()).isEqualTo(0); assertThat(featureRepository.getFailures()).isEqualTo(3); runnableArgumentCaptor.getValue().run(); - assertThat(featureRepository.getInterval()).isEqualTo(2); + assertThat(featureRepository.getSkips()).isEqualTo(2); assertThat(featureRepository.getFailures()).isEqualTo(2); runnableArgumentCaptor.getValue().run(); runnableArgumentCaptor.getValue().run(); - assertThat(featureRepository.getInterval()).isEqualTo(0); + assertThat(featureRepository.getSkips()).isEqualTo(0); assertThat(featureRepository.getFailures()).isEqualTo(2); runnableArgumentCaptor.getValue().run(); - assertThat(featureRepository.getInterval()).isEqualTo(1); + assertThat(featureRepository.getSkips()).isEqualTo(1); assertThat(featureRepository.getFailures()).isEqualTo(1); runnableArgumentCaptor.getValue().run(); - assertThat(featureRepository.getInterval()).isEqualTo(0); + assertThat(featureRepository.getSkips()).isEqualTo(0); assertThat(featureRepository.getFailures()).isEqualTo(1); runnableArgumentCaptor.getValue().run(); - assertThat(featureRepository.getInterval()).isEqualTo(0); + assertThat(featureRepository.getSkips()).isEqualTo(0); assertThat(featureRepository.getFailures()).isEqualTo(0); } @@ -585,41 +584,41 @@ public void server_errors_should_incrementally_increase_interval() .thenReturn( new ClientFeaturesResponse(FeatureToggleResponse.Status.NOT_CHANGED, 304)); runnableArgumentCaptor.getValue().run(); - assertThat(featureRepository.getInterval()).isEqualTo(1); + assertThat(featureRepository.getSkips()).isEqualTo(1); assertThat(featureRepository.getFailures()).isEqualTo(1); runnableArgumentCaptor.getValue().run(); - assertThat(featureRepository.getInterval()).isEqualTo(0); + assertThat(featureRepository.getSkips()).isEqualTo(0); assertThat(featureRepository.getFailures()).isEqualTo(1); runnableArgumentCaptor.getValue().run(); - assertThat(featureRepository.getInterval()).isEqualTo(2); + assertThat(featureRepository.getSkips()).isEqualTo(2); assertThat(featureRepository.getFailures()).isEqualTo(2); runnableArgumentCaptor.getValue().run(); // NO-OP because interval > 0 runnableArgumentCaptor.getValue().run(); // NO-OP because interval > 0 - assertThat(featureRepository.getInterval()).isEqualTo(0); + assertThat(featureRepository.getSkips()).isEqualTo(0); assertThat(featureRepository.getFailures()).isEqualTo(2); runnableArgumentCaptor.getValue().run(); - assertThat(featureRepository.getInterval()).isEqualTo(3); + assertThat(featureRepository.getSkips()).isEqualTo(3); assertThat(featureRepository.getFailures()).isEqualTo(3); runnableArgumentCaptor.getValue().run(); runnableArgumentCaptor.getValue().run(); runnableArgumentCaptor.getValue().run(); - assertThat(featureRepository.getInterval()).isEqualTo(0); + assertThat(featureRepository.getSkips()).isEqualTo(0); assertThat(featureRepository.getFailures()).isEqualTo(3); runnableArgumentCaptor.getValue().run(); - assertThat(featureRepository.getInterval()).isEqualTo(2); + assertThat(featureRepository.getSkips()).isEqualTo(2); assertThat(featureRepository.getFailures()).isEqualTo(2); runnableArgumentCaptor.getValue().run(); runnableArgumentCaptor.getValue().run(); - assertThat(featureRepository.getInterval()).isEqualTo(0); + assertThat(featureRepository.getSkips()).isEqualTo(0); assertThat(featureRepository.getFailures()).isEqualTo(2); runnableArgumentCaptor.getValue().run(); - assertThat(featureRepository.getInterval()).isEqualTo(1); + assertThat(featureRepository.getSkips()).isEqualTo(1); assertThat(featureRepository.getFailures()).isEqualTo(1); runnableArgumentCaptor.getValue().run(); - assertThat(featureRepository.getInterval()).isEqualTo(0); + assertThat(featureRepository.getSkips()).isEqualTo(0); assertThat(featureRepository.getFailures()).isEqualTo(1); runnableArgumentCaptor.getValue().run(); - assertThat(featureRepository.getInterval()).isEqualTo(0); + assertThat(featureRepository.getSkips()).isEqualTo(0); assertThat(featureRepository.getFailures()).isEqualTo(0); } From f45e0fcb5c10361e3ede286c1e93c08597033c3c Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Wed, 8 Nov 2023 10:30:25 +0100 Subject: [PATCH 11/15] Move throttling logic into new class --- .../metric/UnleashMetricServiceImpl.java | 83 +++------------- .../repository/FeatureRepository.java | 97 +++++-------------- .../java/io/getunleash/util/Throttler.java | 96 ++++++++++++++++++ 3 files changed, 133 insertions(+), 143 deletions(-) create mode 100644 src/main/java/io/getunleash/util/Throttler.java diff --git a/src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java b/src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java index ecdca673f..fa200cd1b 100644 --- a/src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java +++ b/src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java @@ -1,12 +1,11 @@ package io.getunleash.metric; +import io.getunleash.util.Throttler; import io.getunleash.util.UnleashConfig; import io.getunleash.util.UnleashScheduledExecutor; -import java.net.HttpURLConnection; import java.time.LocalDateTime; import java.time.ZoneId; import java.util.Set; -import java.util.concurrent.atomic.AtomicInteger; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -20,9 +19,7 @@ public class UnleashMetricServiceImpl implements UnleashMetricService { // mutable private volatile MetricsBucket currentMetricsBucket; - private final int maxSkips; - private final AtomicInteger failures = new AtomicInteger(); - private final AtomicInteger skips = new AtomicInteger(); + private final Throttler throttler; public UnleashMetricServiceImpl( UnleashConfig unleashConfig, UnleashScheduledExecutor executor) { @@ -37,8 +34,11 @@ public UnleashMetricServiceImpl( this.started = LocalDateTime.now(ZoneId.of("UTC")); this.unleashConfig = unleashConfig; this.metricSender = metricSender; - this.maxSkips = - Integer.max(20, 300 / Integer.max((int) unleashConfig.getSendMetricsInterval(), 1)); + this.throttler = + new Throttler( + (int) unleashConfig.getSendMetricsInterval(), + 300, + unleashConfig.getUnleashURLs().getClientMetricsURL()); long metricsInterval = unleashConfig.getSendMetricsInterval(); executor.setInterval(sendMetrics(), metricsInterval, metricsInterval); } @@ -62,86 +62,29 @@ public void countVariant(String toggleName, String variantName) { private Runnable sendMetrics() { return () -> { - if (skips.get() == 0) { + if (throttler.performAction()) { MetricsBucket metricsBucket = this.currentMetricsBucket; this.currentMetricsBucket = new MetricsBucket(); metricsBucket.end(); ClientMetrics metrics = new ClientMetrics(unleashConfig, metricsBucket); int statusCode = metricSender.sendMetrics(metrics); if (statusCode >= 200 && statusCode < 400) { - decrementFailureCountAndResetSkips(); + throttler.decrementFailureCountAndResetSkips(); } if (statusCode >= 400) { - handleHttpErrorCodes(statusCode); + throttler.handleHttpErrorCodes(statusCode); } } else { - skips.decrementAndGet(); + throttler.skipped(); } }; } - /** - * We've had one successful call, so if we had 10 failures in a row, this will reduce the skips - * down to 9, so that we gradually start polling more often, instead of doing max load - * immediately after a sequence of errors. - */ - private void decrementFailureCountAndResetSkips() { - skips.set(Math.max(failures.decrementAndGet(), 0)); - } - - /** - * We've gotten the message to back off (usually a 429 or a 50x). If we have successive - * failures, failure count here will be incremented higher and higher which will handle - * increasing our backoff, since we set the skip count to the failure count after every reset - */ - private void increaseSkipCount() { - skips.set(Math.min(failures.incrementAndGet(), maxSkips)); - } - - /** - * We've received an error code that we don't expect to change, which means we've already logged - * an ERROR. To avoid hammering the server that just told us we did something wrong and to avoid - * flooding the logs, we'll increase our skip count to maximum - */ - private void maximizeSkips() { - skips.set(maxSkips); - failures.incrementAndGet(); - } - - private void handleHttpErrorCodes(int responseCode) { - if (responseCode == 404) { - maximizeSkips(); - LOGGER.error( - "Server said that the Metrics receiving endpoint at {} does not exist. Backing off to {} times our poll interval to avoid overloading server", - unleashConfig.getUnleashURLs().getClientMetricsURL(), - maxSkips); - } else if (responseCode == 429) { - increaseSkipCount(); - LOGGER.info( - "Client Metrics was RATE LIMITED for the {}. time. Further backing off. Current backoff at {} times our metrics post interval", - failures.get(), - skips.get()); - } else if (responseCode == HttpURLConnection.HTTP_UNAUTHORIZED - || responseCode == HttpURLConnection.HTTP_FORBIDDEN) { - maximizeSkips(); - LOGGER.error( - "Client was not authorized to post metrics to the Unleash API at {}. Backing off to {} times our poll interval to avoid overloading server", - unleashConfig.getUnleashURLs().getClientMetricsURL(), - maxSkips); - } else if (responseCode >= 500) { - increaseSkipCount(); - LOGGER.info( - "Server failed with a {} status code. Backing off. Current backoff at {} times our poll interval", - responseCode, - skips.get()); - } - } - protected int getSkips() { - return this.skips.get(); + return this.throttler.getSkips(); } protected int getFailures() { - return this.failures.get(); + return this.throttler.getFailures(); } } diff --git a/src/main/java/io/getunleash/repository/FeatureRepository.java b/src/main/java/io/getunleash/repository/FeatureRepository.java index 9efaabdcd..630f38953 100644 --- a/src/main/java/io/getunleash/repository/FeatureRepository.java +++ b/src/main/java/io/getunleash/repository/FeatureRepository.java @@ -6,12 +6,11 @@ import io.getunleash.event.EventDispatcher; import io.getunleash.event.UnleashReady; import io.getunleash.lang.Nullable; +import io.getunleash.util.Throttler; import io.getunleash.util.UnleashConfig; import io.getunleash.util.UnleashScheduledExecutor; -import java.net.HttpURLConnection; import java.util.Collections; import java.util.List; -import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; import java.util.stream.Collectors; import org.slf4j.Logger; @@ -25,13 +24,11 @@ public class FeatureRepository implements IFeatureRepository { private final FeatureFetcher featureFetcher; private final EventDispatcher eventDispatcher; + private final Throttler throttler; + private FeatureCollection featureCollection; private boolean ready; - private AtomicInteger failures = new AtomicInteger(0); - private AtomicInteger skips = new AtomicInteger(0); - private final Integer maxSkips; - public FeatureRepository(UnleashConfig unleashConfig) { this(unleashConfig, new FeatureBackupHandlerFile(unleashConfig)); } @@ -44,7 +41,11 @@ public FeatureRepository( this.featureFetcher = unleashConfig.getUnleashFeatureFetcherFactory().apply(unleashConfig); this.featureBootstrapHandler = new FeatureBootstrapHandler(unleashConfig); this.eventDispatcher = new EventDispatcher(unleashConfig); - this.maxSkips = this.calculateMaxSkips((int) unleashConfig.getFetchTogglesInterval()); + this.throttler = + new Throttler( + (int) unleashConfig.getFetchTogglesInterval(), + 300, + unleashConfig.getUnleashURLs().getFetchTogglesURL()); this.initCollections(unleashConfig.getScheduledExecutor()); } @@ -54,13 +55,16 @@ protected FeatureRepository( EventDispatcher eventDispatcher, FeatureFetcher featureFetcher, FeatureBootstrapHandler featureBootstrapHandler) { - this.unleashConfig = unleashConfig; this.featureBackupHandler = featureBackupHandler; this.featureFetcher = featureFetcher; this.featureBootstrapHandler = featureBootstrapHandler; this.eventDispatcher = eventDispatcher; - this.maxSkips = this.calculateMaxSkips((int) unleashConfig.getFetchTogglesInterval()); + this.throttler = + new Throttler( + (int) unleashConfig.getFetchTogglesInterval(), + 300, + unleashConfig.getUnleashURLs().getFetchTogglesURL()); this.initCollections(unleashConfig.getScheduledExecutor()); } @@ -75,7 +79,11 @@ protected FeatureRepository( this.featureFetcher = featureFetcher; this.featureBootstrapHandler = featureBootstrapHandler; this.eventDispatcher = new EventDispatcher(unleashConfig); - this.maxSkips = this.calculateMaxSkips((int) unleashConfig.getFetchTogglesInterval()); + this.throttler = + new Throttler( + (int) unleashConfig.getFetchTogglesInterval(), + 300, + unleashConfig.getUnleashURLs().getFetchTogglesURL()); this.initCollections(executor); } @@ -109,7 +117,7 @@ private Runnable updateFeatures(@Nullable final Consumer handl } private void updateFeaturesInternal(@Nullable final Consumer handler) { - if (skips.get() <= 0L) { + if (throttler.performAction()) { try { ClientFeaturesResponse response = featureFetcher.fetchFeatures(); eventDispatcher.dispatch(response); @@ -124,10 +132,10 @@ private void updateFeaturesInternal(@Nullable final Consumer h featureBackupHandler.write(featureCollection); } else if (response.getStatus() == ClientFeaturesResponse.Status.UNAVAILABLE) { - handleHttpErrorCodes(response.getHttpStatusCode()); + throttler.handleHttpErrorCodes(response.getHttpStatusCode()); return; } - decrementFailureCountAndResetSkips(); + throttler.decrementFailureCountAndResetSkips(); if (!ready) { eventDispatcher.dispatch(new UnleashReady()); ready = true; @@ -140,64 +148,7 @@ private void updateFeaturesInternal(@Nullable final Consumer h } } } else { - skips.decrementAndGet(); // We didn't do anything this iteration, just reduce the count - } - } - - /** - * We've had one successful call, so if we had 10 failures in a row, this will reduce the skips - * down to 9, so that we gradually start polling more often, instead of doing max load - * immediately after a sequence of errors. - */ - private void decrementFailureCountAndResetSkips() { - skips.set(Math.max(failures.decrementAndGet(), 0)); - } - - /** - * We've gotten the message to back off (usually a 429 or a 50x). If we have successive - * failures, failure count here will be incremented higher and higher which will handle - * increasing our backoff, since we set the skip count to the failure count after every reset - */ - private void increaseSkipCount() { - skips.set(Math.min(failures.incrementAndGet(), maxSkips)); - } - - /** - * We've received an error code that we don't expect to change, which means we've already logged - * an ERROR. To avoid hammering the server that just told us we did something wrong and to avoid - * flooding the logs, we'll increase our skip count to maximum - */ - private void maximizeSkips() { - skips.set(maxSkips); - failures.incrementAndGet(); - } - - private void handleHttpErrorCodes(int responseCode) { - if (responseCode == 404) { - maximizeSkips(); - LOGGER.error( - "Server said that the API at {} does not exist. Backing off to {} times our poll interval to avoid overloading server", - unleashConfig.getUnleashAPI(), - maxSkips); - } else if (responseCode == 429) { - increaseSkipCount(); - LOGGER.info( - "Client was RATE LIMITED for the {}. time. Further backing off. Current backoff at {} times our poll interval", - failures.get(), - skips.get()); - } else if (responseCode == HttpURLConnection.HTTP_UNAUTHORIZED - || responseCode == HttpURLConnection.HTTP_FORBIDDEN) { - maximizeSkips(); - LOGGER.error( - "Client failed to authenticate to the Unleash API at {}. Backing off to {} times our poll interval to avoid overloading server", - unleashConfig.getUnleashAPI(), - maxSkips); - } else if (responseCode >= 500) { - increaseSkipCount(); - LOGGER.info( - "Server failed with a {} status code. Backing off. Current backoff at {} times our poll interval", - responseCode, - skips.get()); + throttler.skipped(); // We didn't do anything this iteration, just reduce the count } } @@ -219,10 +170,10 @@ public Segment getSegment(Integer id) { } public Integer getFailures() { - return failures.get(); + return this.throttler.getFailures(); } public Integer getSkips() { - return skips.get(); + return this.throttler.getSkips(); } } diff --git a/src/main/java/io/getunleash/util/Throttler.java b/src/main/java/io/getunleash/util/Throttler.java new file mode 100644 index 000000000..1585e36c8 --- /dev/null +++ b/src/main/java/io/getunleash/util/Throttler.java @@ -0,0 +1,96 @@ +package io.getunleash.util; + +import static java.lang.Integer.max; + +import java.net.HttpURLConnection; +import java.net.URL; +import java.util.concurrent.atomic.AtomicInteger; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class Throttler { + private static final Logger LOGGER = LoggerFactory.getLogger(Throttler.class); + private final int maxSkips; + private final AtomicInteger skips = new AtomicInteger(0); + private final AtomicInteger failures = new AtomicInteger(0); + + private final URL target; + + public Throttler(int intervalLengthSeconds, int longestAcceptableIntervalSeconds, URL target) { + this.maxSkips = max(longestAcceptableIntervalSeconds / max(intervalLengthSeconds, 1), 1); + this.target = target; + } + + /** + * We've had one successful call, so if we had 10 failures in a row, this will reduce the skips + * down to 9, so that we gradually start polling more often, instead of doing max load + * immediately after a sequence of errors. + */ + public void decrementFailureCountAndResetSkips() { + skips.set(Math.max(failures.decrementAndGet(), 0)); + } + + /** + * We've gotten the message to back off (usually a 429 or a 50x). If we have successive + * failures, failure count here will be incremented higher and higher which will handle + * increasing our backoff, since we set the skip count to the failure count after every reset + */ + public void increaseSkipCount() { + skips.set(Math.min(failures.incrementAndGet(), maxSkips)); + } + + /** + * We've received an error code that we don't expect to change, which means we've already logged + * an ERROR. To avoid hammering the server that just told us we did something wrong and to avoid + * flooding the logs, we'll increase our skip count to maximum + */ + public void maximizeSkips() { + skips.set(maxSkips); + failures.incrementAndGet(); + } + + public boolean performAction() { + return skips.get() <= 0; + } + + public void skipped() { + skips.decrementAndGet(); + } + + public void handleHttpErrorCodes(int responseCode) { + if (responseCode == 404) { + maximizeSkips(); + LOGGER.error( + "Server said that the receiving endpoint at {} does not exist. Backing off to {} times our poll interval to avoid overloading server", + this.target, + maxSkips); + } else if (responseCode == 429) { + increaseSkipCount(); + LOGGER.info( + "RATE LIMITED for the {}. time. Further backing off. Current backoff at {} times our metrics post interval", + failures.get(), + skips.get()); + } else if (responseCode == HttpURLConnection.HTTP_UNAUTHORIZED + || responseCode == HttpURLConnection.HTTP_FORBIDDEN) { + maximizeSkips(); + LOGGER.error( + "Client was not authorized to post the Unleash API at {}. Backing off to {} times our poll interval to avoid overloading server", + this.target, + maxSkips); + } else if (responseCode >= 500) { + increaseSkipCount(); + LOGGER.info( + "Server failed with a {} status code. Backing off. Current backoff at {} times our poll interval", + responseCode, + skips.get()); + } + } + + public int getSkips() { + return this.skips.get(); + } + + public int getFailures() { + return this.failures.get(); + } +} From a063d0ae629b93285d1852e27f124461fee58ba7 Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Wed, 8 Nov 2023 10:32:59 +0100 Subject: [PATCH 12/15] return to having the refresh in a runnable --- .../repository/FeatureRepository.java | 68 +++++++++---------- 1 file changed, 33 insertions(+), 35 deletions(-) diff --git a/src/main/java/io/getunleash/repository/FeatureRepository.java b/src/main/java/io/getunleash/repository/FeatureRepository.java index 630f38953..611db85b3 100644 --- a/src/main/java/io/getunleash/repository/FeatureRepository.java +++ b/src/main/java/io/getunleash/repository/FeatureRepository.java @@ -113,43 +113,41 @@ private Integer calculateMaxSkips(int fetchTogglesInterval) { } private Runnable updateFeatures(@Nullable final Consumer handler) { - return () -> updateFeaturesInternal(handler); - } - - private void updateFeaturesInternal(@Nullable final Consumer handler) { - if (throttler.performAction()) { - try { - ClientFeaturesResponse response = featureFetcher.fetchFeatures(); - eventDispatcher.dispatch(response); - if (response.getStatus() == ClientFeaturesResponse.Status.CHANGED) { - SegmentCollection segmentCollection = response.getSegmentCollection(); - featureCollection = - new FeatureCollection( - response.getToggleCollection(), - segmentCollection != null - ? segmentCollection - : new SegmentCollection(Collections.emptyList())); - - featureBackupHandler.write(featureCollection); - } else if (response.getStatus() == ClientFeaturesResponse.Status.UNAVAILABLE) { - throttler.handleHttpErrorCodes(response.getHttpStatusCode()); - return; - } - throttler.decrementFailureCountAndResetSkips(); - if (!ready) { - eventDispatcher.dispatch(new UnleashReady()); - ready = true; - } - } catch (UnleashException e) { - if (handler != null) { - handler.accept(e); - } else { - throw e; + return () -> { + if (throttler.performAction()) { + try { + ClientFeaturesResponse response = featureFetcher.fetchFeatures(); + eventDispatcher.dispatch(response); + if (response.getStatus() == ClientFeaturesResponse.Status.CHANGED) { + SegmentCollection segmentCollection = response.getSegmentCollection(); + featureCollection = + new FeatureCollection( + response.getToggleCollection(), + segmentCollection != null + ? segmentCollection + : new SegmentCollection(Collections.emptyList())); + + featureBackupHandler.write(featureCollection); + } else if (response.getStatus() == ClientFeaturesResponse.Status.UNAVAILABLE) { + throttler.handleHttpErrorCodes(response.getHttpStatusCode()); + return; + } + throttler.decrementFailureCountAndResetSkips(); + if (!ready) { + eventDispatcher.dispatch(new UnleashReady()); + ready = true; + } + } catch (UnleashException e) { + if (handler != null) { + handler.accept(e); + } else { + throw e; + } } + } else { + throttler.skipped(); // We didn't do anything this iteration, just reduce the count } - } else { - throttler.skipped(); // We didn't do anything this iteration, just reduce the count - } + }; } @Override From 6de9f3983fd9e244f12386ebfd74c534a5eee306 Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Wed, 8 Nov 2023 10:38:12 +0100 Subject: [PATCH 13/15] Added poll interval time to log lines --- .../java/io/getunleash/util/Throttler.java | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/main/java/io/getunleash/util/Throttler.java b/src/main/java/io/getunleash/util/Throttler.java index 1585e36c8..8e9dbe7cb 100644 --- a/src/main/java/io/getunleash/util/Throttler.java +++ b/src/main/java/io/getunleash/util/Throttler.java @@ -11,6 +11,8 @@ public class Throttler { private static final Logger LOGGER = LoggerFactory.getLogger(Throttler.class); private final int maxSkips; + + private final int intervalLength; private final AtomicInteger skips = new AtomicInteger(0); private final AtomicInteger failures = new AtomicInteger(0); @@ -19,6 +21,7 @@ public class Throttler { public Throttler(int intervalLengthSeconds, int longestAcceptableIntervalSeconds, URL target) { this.maxSkips = max(longestAcceptableIntervalSeconds / max(intervalLengthSeconds, 1), 1); this.target = target; + this.intervalLength = intervalLengthSeconds; } /** @@ -61,28 +64,33 @@ public void handleHttpErrorCodes(int responseCode) { if (responseCode == 404) { maximizeSkips(); LOGGER.error( - "Server said that the receiving endpoint at {} does not exist. Backing off to {} times our poll interval to avoid overloading server", + "Server said that the endpoint at {} does not exist. Backing off to {} times our poll interval (of {} seconds) to avoid overloading server", this.target, - maxSkips); + maxSkips, + this.intervalLength + ); } else if (responseCode == 429) { increaseSkipCount(); LOGGER.info( - "RATE LIMITED for the {}. time. Further backing off. Current backoff at {} times our metrics post interval", + "RATE LIMITED for the {}. time. Further backing off. Current backoff at {} times our interval (of {} seconds)", failures.get(), - skips.get()); + skips.get(), + this.intervalLength); } else if (responseCode == HttpURLConnection.HTTP_UNAUTHORIZED || responseCode == HttpURLConnection.HTTP_FORBIDDEN) { maximizeSkips(); LOGGER.error( - "Client was not authorized to post the Unleash API at {}. Backing off to {} times our poll interval to avoid overloading server", + "Client was not authorized to talk to the Unleash API at {}. Backing off to {} times our poll interval (of {} seconds) to avoid overloading server", this.target, - maxSkips); + maxSkips, + this.intervalLength); } else if (responseCode >= 500) { increaseSkipCount(); LOGGER.info( - "Server failed with a {} status code. Backing off. Current backoff at {} times our poll interval", + "Server failed with a {} status code. Backing off. Current backoff at {} times our poll interval (of {} seconds)", responseCode, - skips.get()); + skips.get(), + this.intervalLength); } } From 198f9222b1aa46885290b70b0b9608ccbb5eb19e Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Wed, 8 Nov 2023 10:38:43 +0100 Subject: [PATCH 14/15] chore(formatting): spotless --- src/main/java/io/getunleash/util/Throttler.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/getunleash/util/Throttler.java b/src/main/java/io/getunleash/util/Throttler.java index 8e9dbe7cb..1b984e6fc 100644 --- a/src/main/java/io/getunleash/util/Throttler.java +++ b/src/main/java/io/getunleash/util/Throttler.java @@ -67,8 +67,7 @@ public void handleHttpErrorCodes(int responseCode) { "Server said that the endpoint at {} does not exist. Backing off to {} times our poll interval (of {} seconds) to avoid overloading server", this.target, maxSkips, - this.intervalLength - ); + this.intervalLength); } else if (responseCode == 429) { increaseSkipCount(); LOGGER.info( @@ -83,7 +82,7 @@ public void handleHttpErrorCodes(int responseCode) { "Client was not authorized to talk to the Unleash API at {}. Backing off to {} times our poll interval (of {} seconds) to avoid overloading server", this.target, maxSkips, - this.intervalLength); + this.intervalLength); } else if (responseCode >= 500) { increaseSkipCount(); LOGGER.info( From 952b5d736b8d6712e14cd714538436fd79e5ae03 Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Wed, 8 Nov 2023 10:59:26 +0100 Subject: [PATCH 15/15] Change order of http status codes to ascending --- .../java/io/getunleash/util/Throttler.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/main/java/io/getunleash/util/Throttler.java b/src/main/java/io/getunleash/util/Throttler.java index 1b984e6fc..6427f1c10 100644 --- a/src/main/java/io/getunleash/util/Throttler.java +++ b/src/main/java/io/getunleash/util/Throttler.java @@ -61,7 +61,15 @@ public void skipped() { } public void handleHttpErrorCodes(int responseCode) { - if (responseCode == 404) { + if (responseCode == HttpURLConnection.HTTP_UNAUTHORIZED || responseCode == HttpURLConnection.HTTP_FORBIDDEN) { + maximizeSkips(); + LOGGER.error( + "Client was not authorized to talk to the Unleash API at {}. Backing off to {} times our poll interval (of {} seconds) to avoid overloading server", + this.target, + maxSkips, + this.intervalLength); + } + if (responseCode == HttpURLConnection.HTTP_NOT_FOUND) { maximizeSkips(); LOGGER.error( "Server said that the endpoint at {} does not exist. Backing off to {} times our poll interval (of {} seconds) to avoid overloading server", @@ -75,15 +83,7 @@ public void handleHttpErrorCodes(int responseCode) { failures.get(), skips.get(), this.intervalLength); - } else if (responseCode == HttpURLConnection.HTTP_UNAUTHORIZED - || responseCode == HttpURLConnection.HTTP_FORBIDDEN) { - maximizeSkips(); - LOGGER.error( - "Client was not authorized to talk to the Unleash API at {}. Backing off to {} times our poll interval (of {} seconds) to avoid overloading server", - this.target, - maxSkips, - this.intervalLength); - } else if (responseCode >= 500) { + } else if (responseCode >= HttpURLConnection.HTTP_INTERNAL_ERROR) { increaseSkipCount(); LOGGER.info( "Server failed with a {} status code. Backing off. Current backoff at {} times our poll interval (of {} seconds)",