From a6fd325f5a277eb3b4e11f787d2202e6d1a723a9 Mon Sep 17 00:00:00 2001 From: Martin Redolatti Date: Wed, 12 May 2021 20:20:40 -0300 Subject: [PATCH 1/2] allow the user to change amount of attempts and how many attempts to wait until loging --- client/pom.xml | 2 +- .../io/split/client/SplitClientConfig.java | 31 +++++++++++++++++++ .../io/split/client/SplitFactoryImpl.java | 2 ++ .../split/engine/common/SyncManagerImp.java | 27 ++++++++++++++-- .../split/engine/common/SynchronizerImp.java | 20 +++++++----- .../split/engine/common/SynchronizerTest.java | 2 +- pom.xml | 2 +- testing/pom.xml | 2 +- 8 files changed, 74 insertions(+), 14 deletions(-) diff --git a/client/pom.xml b/client/pom.xml index c797762fa..dea2b550e 100644 --- a/client/pom.xml +++ b/client/pom.xml @@ -5,7 +5,7 @@ io.split.client java-client-parent - 4.1.7-rc2 + 4.1.7-rc3 java-client jar diff --git a/client/src/main/java/io/split/client/SplitClientConfig.java b/client/src/main/java/io/split/client/SplitClientConfig.java index e245125c0..3e21dd5f9 100644 --- a/client/src/main/java/io/split/client/SplitClientConfig.java +++ b/client/src/main/java/io/split/client/SplitClientConfig.java @@ -47,6 +47,8 @@ public class SplitClientConfig { private final String _authServiceURL; private final String _streamingServiceURL; private final int _onDemandFetchRetryDelayMs; + private final int _onDemandFetchMaxRetries; + private final int _failedAttemptsBeforeLogging; private final boolean _cdnDebugLogging; // Proxy configs @@ -93,6 +95,8 @@ private SplitClientConfig(String endpoint, String authServiceURL, String streamingServiceURL, int onDemandFetchRetryDelayMs, + int onDemandFetchMaxRetries, + int failedAttemptsBeforeLogging, boolean cdnDebugLogging) { _endpoint = endpoint; _eventsEndpoint = eventsEndpoint; @@ -125,6 +129,8 @@ private SplitClientConfig(String endpoint, _authServiceURL = authServiceURL; _streamingServiceURL = streamingServiceURL; _onDemandFetchRetryDelayMs = onDemandFetchRetryDelayMs; + _onDemandFetchMaxRetries = onDemandFetchMaxRetries; + _failedAttemptsBeforeLogging = failedAttemptsBeforeLogging; _cdnDebugLogging = cdnDebugLogging; Properties props = new Properties(); @@ -256,6 +262,10 @@ public String streamingServiceURL() { public int streamingRetryDelay() {return _onDemandFetchRetryDelayMs;} + public int streamingFetchMaxRetries() {return _onDemandFetchMaxRetries;} + + public int failedAttemptsBeforeLogging() {return _failedAttemptsBeforeLogging;} + public boolean cdnDebugLogging() { return _cdnDebugLogging; } @@ -295,6 +305,8 @@ public static final class Builder { private String _authServiceURL = "https://auth.split.io/api/auth"; private String _streamingServiceURL = "https://streaming.split.io/sse"; private int _onDemandFetchRetryDelayMs = 50; + private int _onDemandFetchMaxRetries = 10; + private int _failedAttemptsBeforeLogging = -1; private boolean _cdnDebugLogging = true; public Builder() { @@ -697,6 +709,16 @@ public Builder streamingRetryDelay(int onDemandFetchRetryDelayMs) { return this; } + public Builder streamingFetchMaxRetries(int maxRetries) { + _onDemandFetchMaxRetries = maxRetries; + return this; + } + + public Builder failedAttemptsBeforeLoggingCDNInfo(int failedAttemptsBeforeLogging) { + _failedAttemptsBeforeLogging = failedAttemptsBeforeLogging; + return this; + } + /** * Enable logging response headers for requests made to our CDN. * @param cdnDebugLogging @@ -780,6 +802,13 @@ public SplitClientConfig build() { if(_onDemandFetchRetryDelayMs <= 0) { throw new IllegalStateException("streamingRetryDelay must be > 0"); } + if(_onDemandFetchMaxRetries <= 0) { + throw new IllegalStateException("streamingRetryDelay must be > 0"); + } + + if (_failedAttemptsBeforeLogging < 0) { + _failedAttemptsBeforeLogging = _onDemandFetchMaxRetries / 2; + } return new SplitClientConfig( _endpoint, @@ -813,6 +842,8 @@ public SplitClientConfig build() { _authServiceURL, _streamingServiceURL, _onDemandFetchRetryDelayMs, + _onDemandFetchMaxRetries, + _failedAttemptsBeforeLogging, _cdnDebugLogging); } } diff --git a/client/src/main/java/io/split/client/SplitFactoryImpl.java b/client/src/main/java/io/split/client/SplitFactoryImpl.java index e927460b2..afce36af4 100644 --- a/client/src/main/java/io/split/client/SplitFactoryImpl.java +++ b/client/src/main/java/io/split/client/SplitFactoryImpl.java @@ -159,6 +159,8 @@ public SplitFactoryImpl(String apiToken, SplitClientConfig config) throws URISyn buildSSEdHttpClient(config), _segmentCache, config.streamingRetryDelay(), + config.streamingFetchMaxRetries(), + config.failedAttemptsBeforeLogging(), config.cdnDebugLogging()); _syncManager.start(); diff --git a/client/src/main/java/io/split/engine/common/SyncManagerImp.java b/client/src/main/java/io/split/engine/common/SyncManagerImp.java index 4095275d0..2534d067b 100644 --- a/client/src/main/java/io/split/engine/common/SyncManagerImp.java +++ b/client/src/main/java/io/split/engine/common/SyncManagerImp.java @@ -61,11 +61,32 @@ public static SyncManagerImp build(boolean streamingEnabledConfig, CloseableHttpClient sseHttpClient, SegmentCache segmentCache, int streamingRetryDelay, + int maxOnDemandFetchRetries, + int failedAttemptsBeforeLogging, boolean cdnDebugLogging) { LinkedBlockingQueue pushMessages = new LinkedBlockingQueue<>(); - Synchronizer synchronizer = new SynchronizerImp(splitSynchronizationTask, splitFetcher, segmentSynchronizationTaskImp, splitCache, segmentCache, streamingRetryDelay, cdnDebugLogging); - PushManager pushManager = PushManagerImp.build(synchronizer, streamingServiceUrl, authUrl, httpClient, pushMessages, sseHttpClient); - return new SyncManagerImp(streamingEnabledConfig, synchronizer, pushManager, pushMessages, authRetryBackOffBase); + Synchronizer synchronizer = new SynchronizerImp(splitSynchronizationTask, + splitFetcher, + segmentSynchronizationTaskImp, + splitCache, + segmentCache, + streamingRetryDelay, + maxOnDemandFetchRetries, + failedAttemptsBeforeLogging, + cdnDebugLogging); + + PushManager pushManager = PushManagerImp.build(synchronizer, + streamingServiceUrl, + authUrl, + httpClient, + pushMessages, + sseHttpClient); + + return new SyncManagerImp(streamingEnabledConfig, + synchronizer, + pushManager, + pushMessages, + authRetryBackOffBase); } @Override diff --git a/client/src/main/java/io/split/engine/common/SynchronizerImp.java b/client/src/main/java/io/split/engine/common/SynchronizerImp.java index 155af6767..3223827c9 100644 --- a/client/src/main/java/io/split/engine/common/SynchronizerImp.java +++ b/client/src/main/java/io/split/engine/common/SynchronizerImp.java @@ -21,8 +21,6 @@ public class SynchronizerImp implements Synchronizer { - private static final int MAX_ATTEMPTS = 10; - private static final Logger _log = LoggerFactory.getLogger(Synchronizer.class); private final SplitSynchronizationTask _splitSynchronizationTask; private final SplitFetcher _splitFetcher; @@ -31,7 +29,10 @@ public class SynchronizerImp implements Synchronizer { private final SplitCache _splitCache; private final SegmentCache _segmentCache; private final int _onDemandFetchRetryDelayMs; + private final int _onDemandFetchMaxRetries; + private final int _failedAttemptsBeforeLogging; private final boolean _cdnResponseHeadersLogging; + private final Gson gson = new GsonBuilder().create(); public SynchronizerImp(SplitSynchronizationTask splitSynchronizationTask, @@ -40,6 +41,8 @@ public SynchronizerImp(SplitSynchronizationTask splitSynchronizationTask, SplitCache splitCache, SegmentCache segmentCache, int onDemandFetchRetryDelayMs, + int onDemandFetchMaxRetries, + int failedAttemptsBeforeLogging, boolean cdnResponseHeadersLogging) { _splitSynchronizationTask = checkNotNull(splitSynchronizationTask); _splitFetcher = checkNotNull(splitFetcher); @@ -48,6 +51,8 @@ public SynchronizerImp(SplitSynchronizationTask splitSynchronizationTask, _segmentCache = checkNotNull(segmentCache); _onDemandFetchRetryDelayMs = checkNotNull(onDemandFetchRetryDelayMs); _cdnResponseHeadersLogging = cdnResponseHeadersLogging; + _onDemandFetchMaxRetries = onDemandFetchMaxRetries; + _failedAttemptsBeforeLogging = failedAttemptsBeforeLogging; ThreadFactory splitsThreadFactory = new ThreadFactoryBuilder() .setDaemon(true) @@ -92,15 +97,15 @@ public void refreshSplits(long targetChangeNumber) { .responseHeadersCallback(_cdnResponseHeadersLogging ? captor::handle : null) .build(); - int remainingAttempts = MAX_ATTEMPTS; + int remainingAttempts = _onDemandFetchMaxRetries; while(true) { remainingAttempts--; _splitFetcher.forceRefresh(opts); if (targetChangeNumber <= _splitCache.getChangeNumber()) { - _log.debug(String.format("Refresh completed in %s attempts.", MAX_ATTEMPTS - remainingAttempts)); + _log.debug(String.format("Refresh completed in %s attempts.", _onDemandFetchMaxRetries - remainingAttempts)); break; } else if (remainingAttempts <= 0) { - _log.info(String.format("No changes fetched after %s attempts.", MAX_ATTEMPTS)); + _log.info(String.format("No changes fetched after %s attempts.", _onDemandFetchMaxRetries)); break; } try { @@ -111,7 +116,8 @@ public void refreshSplits(long targetChangeNumber) { } } - if (_cdnResponseHeadersLogging && remainingAttempts <= (MAX_ATTEMPTS / 2)) { + if (_cdnResponseHeadersLogging && + (_onDemandFetchMaxRetries - remainingAttempts) > _failedAttemptsBeforeLogging) { _log.info(String.format("CDN Debug headers: %s", gson.toJson(captor.get()))); } } @@ -127,7 +133,7 @@ public void localKillSplit(String splitName, String defaultTreatment, long newCh @Override public void refreshSegment(String segmentName, long changeNumber) { int retries = 1; - while(changeNumber > _segmentCache.getChangeNumber(segmentName) && retries <= MAX_ATTEMPTS) { + while(changeNumber > _segmentCache.getChangeNumber(segmentName) && retries <= _onDemandFetchMaxRetries) { SegmentFetcher fetcher = _segmentSynchronizationTaskImp.getFetcher(segmentName); try{ fetcher.fetch(true); diff --git a/client/src/test/java/io/split/engine/common/SynchronizerTest.java b/client/src/test/java/io/split/engine/common/SynchronizerTest.java index 2438bbdac..13f692b75 100644 --- a/client/src/test/java/io/split/engine/common/SynchronizerTest.java +++ b/client/src/test/java/io/split/engine/common/SynchronizerTest.java @@ -26,7 +26,7 @@ public void beforeMethod() { _splitCache = Mockito.mock(SplitCache.class); _segmentCache = Mockito.mock(SegmentCache.class); - _synchronizer = new SynchronizerImp(_refreshableSplitFetcherTask, _splitFetcher, _segmentFetcher, _splitCache, _segmentCache, 50, false); + _synchronizer = new SynchronizerImp(_refreshableSplitFetcherTask, _splitFetcher, _segmentFetcher, _splitCache, _segmentCache, 50, 10, 5, false); } @Test diff --git a/pom.xml b/pom.xml index b6fe51919..d3d9043bf 100644 --- a/pom.xml +++ b/pom.xml @@ -4,7 +4,7 @@ 4.0.0 io.split.client java-client-parent - 4.1.7-rc2 + 4.1.7-rc3 diff --git a/testing/pom.xml b/testing/pom.xml index d52d88807..e1a6bc52a 100644 --- a/testing/pom.xml +++ b/testing/pom.xml @@ -6,7 +6,7 @@ io.split.client java-client-parent - 4.1.7-rc2 + 4.1.7-rc3 java-client-testing From d387762998ae9dfeb9c4f0fd3dc0ce66afaa557a Mon Sep 17 00:00:00 2001 From: Martin Redolatti Date: Wed, 12 May 2021 20:44:12 -0300 Subject: [PATCH 2/2] fix error message --- client/src/main/java/io/split/client/SplitClientConfig.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/src/main/java/io/split/client/SplitClientConfig.java b/client/src/main/java/io/split/client/SplitClientConfig.java index 3e21dd5f9..0cd01f2f7 100644 --- a/client/src/main/java/io/split/client/SplitClientConfig.java +++ b/client/src/main/java/io/split/client/SplitClientConfig.java @@ -803,7 +803,7 @@ public SplitClientConfig build() { throw new IllegalStateException("streamingRetryDelay must be > 0"); } if(_onDemandFetchMaxRetries <= 0) { - throw new IllegalStateException("streamingRetryDelay must be > 0"); + throw new IllegalStateException("_onDemandFetchMaxRetries must be > 0"); } if (_failedAttemptsBeforeLogging < 0) {