Pr comments

Author: ldecheverz-split

client/src/main/java/io/split/client/SplitClientConfig.java

@@ -46,7 +46,7 @@ public class SplitClientConfig {
     private final int _streamingReconnectBackoffBase;
     private final String _authServiceURL;
     private final String _streamingServiceURL;
-    private final int _streamingRetryDelay;
+    private final int _onDemandFetchRetryDelayMs;
 
     // Proxy configs
     private final HttpHost _proxy;
@@ -91,7 +91,7 @@ private SplitClientConfig(String endpoint,
                               int streamingReconnectBackoffBase,
                               String authServiceURL,
                               String streamingServiceURL,
-                              int streamingRetryDelay) {
+                              int onDemandFetchRetryDelayMs) {
         _endpoint = endpoint;
         _eventsEndpoint = eventsEndpoint;
         _featuresRefreshRate = pollForFeatureChangesEveryNSeconds;
@@ -122,7 +122,7 @@ private SplitClientConfig(String endpoint,
         _streamingReconnectBackoffBase = streamingReconnectBackoffBase;
         _authServiceURL = authServiceURL;
         _streamingServiceURL = streamingServiceURL;
-        _streamingRetryDelay = streamingRetryDelay;
+        _onDemandFetchRetryDelayMs = onDemandFetchRetryDelayMs;
 
         Properties props = new Properties();
         try {
@@ -251,7 +251,7 @@ public String streamingServiceURL() {
         return _streamingServiceURL;
     }
 
-    public int streamingRetryDelay() {return _streamingRetryDelay;}
+    public int streamingRetryDelay() {return _onDemandFetchRetryDelayMs;}
 
     public static final class Builder {
 
@@ -288,7 +288,7 @@ public static final class Builder {
         private int _streamingReconnectBackoffBase = 1;
         private String _authServiceURL = "https://auth.split.io/api/auth";
         private String _streamingServiceURL = "https://streaming.split.io/sse";
-        private int _streamingRetryDelay = 50;
+        private int _onDemandFetchRetryDelayMs = 50;
 
         public Builder() {
         }
@@ -682,11 +682,11 @@ public Builder streamingServiceURL(String streamingServiceURL) {
 
         /**
          * Set Streaming retry delay.
-         * @param streamingRetryDelay
+         * @param onDemandFetchRetryDelayMs
          * @return
          */
-        public Builder streamingRetryDelay(int streamingRetryDelay) {
-            _streamingRetryDelay = streamingRetryDelay;
+        public Builder streamingRetryDelay(int onDemandFetchRetryDelayMs) {
+            _onDemandFetchRetryDelayMs = onDemandFetchRetryDelayMs;
             return this;
         }
 
@@ -760,7 +760,7 @@ public SplitClientConfig build() {
                 throw new IllegalArgumentException("streamingServiceURL must not be null");
             }
 
-            if(_streamingRetryDelay <= 0) {
+            if(_onDemandFetchRetryDelayMs <= 0) {
                 throw new IllegalStateException("streamingRetryDelay must be > 0");
             }
 
@@ -795,7 +795,7 @@ public SplitClientConfig build() {
                     _streamingReconnectBackoffBase,
                     _authServiceURL,
                     _streamingServiceURL,
-                    _streamingRetryDelay);
+                    _onDemandFetchRetryDelayMs);
         }
     }
 }

client/src/main/java/io/split/engine/common/SynchronizerImp.java

@@ -27,20 +27,20 @@ public class SynchronizerImp implements Synchronizer {
     private final ScheduledExecutorService _syncAllScheduledExecutorService;
     private final SplitCache _splitCache;
     private final SegmentCache _segmentCache;
-    private final int _streamingRetryDelay;
+    private final int _onDemandFetchRetryDelayMs;
 
     public SynchronizerImp(SplitSynchronizationTask splitSynchronizationTask,
                            SplitFetcher splitFetcher,
                            SegmentSynchronizationTask segmentSynchronizationTaskImp,
                            SplitCache splitCache,
                            SegmentCache segmentCache,
-                           int streamingRetryDelay) {
+                           int onDemandFetchRetryDelayMs) {
         _splitSynchronizationTask = checkNotNull(splitSynchronizationTask);
         _splitFetcher = checkNotNull(splitFetcher);
         _segmentSynchronizationTaskImp = checkNotNull(segmentSynchronizationTaskImp);
         _splitCache = checkNotNull(splitCache);
         _segmentCache = checkNotNull(segmentCache);
-        _streamingRetryDelay = checkNotNull(streamingRetryDelay);
+        _onDemandFetchRetryDelayMs = checkNotNull(onDemandFetchRetryDelayMs);
 
         ThreadFactory splitsThreadFactory = new ThreadFactoryBuilder()
                 .setDaemon(true)
@@ -73,14 +73,23 @@ public void stopPeriodicFetching() {
 
     @Override
     public void refreshSplits(long targetChangeNumber) {
-        int retries = 1;
-        while(targetChangeNumber > _splitCache.getChangeNumber() && retries <= RETRIES_NUMBER) {
-            checkNumberAttepmt(retries);
+        int retries = RETRIES_NUMBER;
+        while(true) {
+            retries--;
             _splitFetcher.forceRefresh(true);
-            retries++;
-        }
-        if(retries > 1) {
-            _log.debug("Refresh completed in %s attempts.", retries);
+            if (targetChangeNumber <= _splitCache.getChangeNumber()) {
+                _log.debug("Refresh completed in %s attempts.", RETRIES_NUMBER - retries);
+                return;
+            } else if (retries <= 0) {
+                _log.warn("No changes fetched after %s attempts.", RETRIES_NUMBER);
+                return;
+            }
+            try {
+                Thread.sleep(_onDemandFetchRetryDelayMs);
+            } catch (InterruptedException e) {
+                Thread.currentThread().interrupt();
+                _log.debug("Error trying to sleep current Thread.");
+            }
         }
     }
 
@@ -107,14 +116,4 @@ public void refreshSegment(String segmentName, long changeNumber) {
             retries++;
         }
     }
-
-    private void checkNumberAttepmt(int retry) {
-        if(retry > 1) {
-            try {
-                Thread.sleep(_streamingRetryDelay);
-            } catch (InterruptedException e) {
-                e.printStackTrace();
-            }
-        }
-    }
 }