Skip to content

Commit bed5bfd

Browse files
ldecheverz-splitldecheverz-split
committed
Fix PR comments
1 parent 2dd455c commit bed5bfd

14 files changed

+103
-205
lines changed

‎client/src/main/java/io/split/client/ApiKeyCounter.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,4 +75,9 @@ public Map<String, Long> getFactoryInstances() {
7575

7676
return factoryInstances;
7777
}
78+
79+
@VisibleForTesting
80+
void clearApiKeys() {
81+
USED_API_KEYS.clear();
82+
}
7883
}

‎client/src/main/java/io/split/client/LocalhostSplitFactory.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,9 @@ public LocalhostSplitFactory(String directory, String file) throws IOException {
5656
SplitCache splitCache = new InMemoryCacheImp();
5757
SDKReadinessGates sdkReadinessGates = new SDKReadinessGates();
5858

59-
sdkReadinessGates.splitsAreReady();
6059
_cacheUpdaterService = new CacheUpdaterService(splitCache);
6160
_cacheUpdaterService.updateCache(splitAndKeyToTreatment);
61+
sdkReadinessGates.sdkInternalReady();
6262
_client = new SplitClientImpl(this, splitCache,
6363
new ImpressionsManager.NoOpImpressionsManager(), new NoopEventClient(),
6464
SplitClientConfig.builder().setBlockUntilReadyTimeout(1).build(), sdkReadinessGates, new EvaluatorImp(splitCache), new NoopTelemetryStorage(), new NoopTelemetryStorage());

‎client/src/main/java/io/split/client/SplitClientImpl.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
import io.split.inputValidation.KeyValidator;
1616
import io.split.inputValidation.SplitNameValidator;
1717
import io.split.inputValidation.TrafficTypeValidator;
18-
import io.split.telemetry.domain.enums.LastSynchronizationRecordsEnum;
1918
import io.split.telemetry.domain.enums.MethodEnum;
2019
import io.split.telemetry.storage.TelemetryConfigProducer;
2120
import io.split.telemetry.storage.TelemetryEvaluationProducer;
@@ -138,7 +137,7 @@ public void blockUntilReady() throws TimeoutException, InterruptedException {
138137
if (_config.blockUntilReady() <= 0) {
139138
throw new IllegalArgumentException("setBlockUntilReadyTimeout must be positive but in config was: " + _config.blockUntilReady());
140139
}
141-
if (!_gates.isSDKReady(_config.blockUntilReady())) {
140+
if (!_gates.waitUntilInternalReady(_config.blockUntilReady())) {
142141
throw new TimeoutException("SDK was not ready in " + _config.blockUntilReady()+ " milliseconds");
143142
}
144143
_log.debug(String.format("Split SDK ready in %d ms", (System.currentTimeMillis() - startTime)));
@@ -188,7 +187,7 @@ private boolean track(Event event) {
188187
private SplitResult getTreatmentWithConfigInternal(String method, String matchingKey, String bucketingKey, String split, Map<String, Object> attributes, MethodEnum methodEnum) {
189188
long initTime = System.currentTimeMillis();
190189
try {
191-
if(!_gates.isSDKReadyNow()){
190+
if(!_gates.isSDKReady()){
192191
_log.warn(method + ": the SDK is not ready, results may be incorrect. Make sure to wait for SDK readiness before using this method");
193192
_telemetryConfigProducer.recordNonReadyUsage();
194193
}
@@ -215,7 +214,7 @@ private SplitResult getTreatmentWithConfigInternal(String method, String matchin
215214

216215
EvaluatorImp.TreatmentLabelAndChangeNumber result = _evaluator.evaluateFeature(matchingKey, bucketingKey, split, attributes);
217216

218-
if (result.treatment.equals(Treatments.CONTROL) && result.label.equals(Labels.DEFINITION_NOT_FOUND) && _gates.isSDKReadyNow()) {
217+
if (result.treatment.equals(Treatments.CONTROL) && result.label.equals(Labels.DEFINITION_NOT_FOUND) && _gates.isSDKReady()) {
219218
_log.warn(
220219
"getTreatment: you passed \"" + split + "\" that does not exist in this environment, " +
221220
"please double check what Splits exist in the web console.");

‎client/src/main/java/io/split/client/SplitManagerImpl.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
import io.split.cache.SplitCache;
77
import io.split.engine.experiments.ParsedSplit;
88
import io.split.inputValidation.SplitNameValidator;
9-
import io.split.telemetry.domain.enums.MethodEnum;
109
import io.split.telemetry.storage.TelemetryConfigProducer;
1110
import org.slf4j.Logger;
1211
import org.slf4j.LoggerFactory;
@@ -43,7 +42,7 @@ public SplitManagerImpl(SplitCache splitCache,
4342

4443
@Override
4544
public List<SplitView> splits() {
46-
if (!_gates.isSDKReadyNow()) { {
45+
if (!_gates.isSDKReady()) { {
4746
_log.warn("splits: the SDK is not ready, results may be incorrect. Make sure to wait for SDK readiness before using this method");
4847
_telemetryConfigProducer.recordNonReadyUsage();
4948
}}
@@ -58,7 +57,7 @@ public List<SplitView> splits() {
5857

5958
@Override
6059
public SplitView split(String featureName) {
61-
if (!_gates.isSDKReadyNow()) { {
60+
if (!_gates.isSDKReady()) { {
6261
_log.warn("split: the SDK is not ready, results may be incorrect. Make sure to wait for SDK readiness before using this method");
6362
_telemetryConfigProducer.recordNonReadyUsage();
6463
}}
@@ -70,7 +69,7 @@ public SplitView split(String featureName) {
7069

7170
ParsedSplit parsedSplit = _splitCache.get(featureName);
7271
if (parsedSplit == null) {
73-
if (_gates.isSDKReadyNow()) {
72+
if (_gates.isSDKReady()) {
7473
_log.warn("split: you passed \"" + featureName + "\" that does not exist in this environment, " +
7574
"please double check what Splits exist in the web console.");
7675
}
@@ -82,7 +81,7 @@ public SplitView split(String featureName) {
8281

8382
@Override
8483
public List<String> splitNames() {
85-
if (!_gates.isSDKReadyNow()) { {
84+
if (!_gates.isSDKReady()) { {
8685
_log.warn("splitNames: the SDK is not ready, results may be incorrect. Make sure to wait for SDK readiness before using this method");
8786
_telemetryConfigProducer.recordNonReadyUsage();
8887
}}
@@ -100,7 +99,7 @@ public void blockUntilReady() throws TimeoutException, InterruptedException {
10099
if (_config.blockUntilReady() <= 0) {
101100
throw new IllegalArgumentException("setBlockUntilReadyTimeout must be positive but in config was: " + _config.blockUntilReady());
102101
}
103-
if (!_gates.isSDKReady(_config.blockUntilReady())) {
102+
if (!_gates.waitUntilInternalReady(_config.blockUntilReady())) {
104103
_telemetryConfigProducer.recordBURTimeout();
105104
throw new TimeoutException("SDK was not ready in " + _config.blockUntilReady()+ " milliseconds");
106105
}

‎client/src/main/java/io/split/engine/SDKReadinessGates.java‎

Lines changed: 3 additions & 124 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,7 @@
1515
public class SDKReadinessGates {
1616
private static final Logger _log = LoggerFactory.getLogger(SDKReadinessGates.class);
1717

18-
private final CountDownLatch _splitsAreReady = new CountDownLatch(1);
1918
private final CountDownLatch _internalReady = new CountDownLatch(1);
20-
private final ConcurrentMap<String, CountDownLatch> _segmentsAreReady = new ConcurrentHashMap<>();
2119

2220
/**
2321
* Returns true if the SDK is ready. The SDK is ready when:
@@ -34,134 +32,15 @@ public class SDKReadinessGates {
3432
* @return true if the sdk is ready, false otherwise.
3533
* @throws InterruptedException if this operation was interrupted.
3634
*/
37-
public boolean isSDKReady(long milliseconds) throws InterruptedException {
35+
public boolean waitUntilInternalReady(long milliseconds) throws InterruptedException {
3836
return _internalReady.await(milliseconds, TimeUnit.MILLISECONDS);
3937
}
4038

41-
public boolean isSDKReadyNow() {
42-
try {
43-
return isSDKReady(0);
44-
} catch (InterruptedException e) {
45-
return false;
46-
}
47-
}
48-
49-
/**
50-
* Records that the SDK split initialization is done.
51-
* This operation is atomic and idempotent. Repeated invocations
52-
* will not have any impact on the state.
53-
*/
54-
public void splitsAreReady() {
55-
long originalCount = _splitsAreReady.getCount();
56-
_splitsAreReady.countDown();
57-
if (originalCount > 0L) {
58-
_log.info("splits are ready");
59-
}
60-
}
61-
62-
/**
63-
* Registers a segment that the SDK should download before it is ready.
64-
* This method should be called right after the first successful download
65-
* of split definitions.
66-
* <p/>
67-
* Note that if this method is called in subsequent fetches of splits,
68-
* it will return false; meaning any segments used in new splits
69-
* will not be able to block the SDK from being marked as complete.
70-
*
71-
* @param segmentName the segment to register
72-
* @return true if the segments were registered, false otherwise.
73-
* @throws InterruptedException
74-
*/
75-
public boolean registerSegment(String segmentName) throws InterruptedException {
76-
if (segmentName == null || segmentName.isEmpty() || areSplitsReady(0L)) {
77-
return false;
78-
}
79-
80-
_segmentsAreReady.putIfAbsent(segmentName, new CountDownLatch(1));
81-
_log.info("Registered segment: " + segmentName);
82-
return true;
83-
}
84-
85-
/**
86-
* Records that the SDK segment initialization for this segment is done.
87-
* This operation is atomic and idempotent. Repeated invocations
88-
* will not have any impact on the state.
89-
*/
90-
public void segmentIsReady(String segmentName) {
91-
CountDownLatch cdl = _segmentsAreReady.get(segmentName);
92-
if (cdl == null) {
93-
return;
94-
}
95-
96-
long originalCount = cdl.getCount();
97-
98-
cdl.countDown();
99-
100-
if (originalCount > 0L) {
101-
_log.info(segmentName + " segment is ready");
102-
}
103-
}
104-
105-
public boolean isSegmentRegistered(String segmentName) {
106-
return _segmentsAreReady.get(segmentName) != null;
107-
}
108-
109-
/**
110-
* Returns true if the SDK is ready w.r.t segments. In other words, this method returns true if:
111-
* <ol>
112-
* <li>The SDK has fetched segment definitions the first time.</li>
113-
* </ol>
114-
* <p/>
115-
* This operation will block until the SDK is ready or 'milliseconds' have passed. If the milliseconds
116-
* are less than or equal to zero, the operation will not block and return immediately
117-
*
118-
* @param milliseconds time to wait for an answer. if the value is zero or negative, we will not
119-
* block for an answer.
120-
* @return true if the sdk is ready w.r.t splits, false otherwise.
121-
* @throws InterruptedException if this operation was interrupted.
122-
*/
123-
public boolean areSegmentsReady(long milliseconds) throws InterruptedException {
124-
long end = System.currentTimeMillis() + milliseconds;
125-
long timeLeft = milliseconds;
126-
127-
for (Map.Entry<String, CountDownLatch> entry : _segmentsAreReady.entrySet()) {
128-
String segmentName = entry.getKey();
129-
CountDownLatch cdl = entry.getValue();
130-
131-
if (!cdl.await(timeLeft, TimeUnit.MILLISECONDS)) {
132-
_log.error(segmentName + " is not ready yet");
133-
return false;
134-
}
135-
136-
timeLeft = end - System.currentTimeMillis();
137-
}
138-
139-
return true;
140-
}
141-
142-
/**
143-
* Returns true if the SDK is ready w.r.t splits. In other words, this method returns true if:
144-
* <ol>
145-
* <li>The SDK has fetched Split definitions the first time.</li>
146-
* </ol>
147-
* <p/>
148-
* This operation will block until the SDK is ready or 'milliseconds' have passed. If the milliseconds
149-
* are less than or equal to zero, the operation will not block and return immediately
150-
*
151-
* @param milliseconds time to wait for an answer. if the value is zero or negative, we will not
152-
* block for an answer.
153-
* @return true if the sdk is ready w.r.t splits, false otherwise.
154-
* @throws InterruptedException if this operation was interrupted.
155-
*/
156-
public boolean areSplitsReady(long milliseconds) throws InterruptedException {
157-
return _splitsAreReady.await(milliseconds, TimeUnit.MILLISECONDS);
39+
public boolean isSDKReady() {
40+
return _internalReady.getCount() == 0;
15841
}
15942

16043
public void sdkInternalReady() {
16144
_internalReady.countDown();
16245
}
163-
164-
public void waitUntilInternalReady() throws InterruptedException {
165-
_internalReady.await();
166-
}
16746
}

‎client/src/main/java/io/split/engine/experiments/SplitFetcherImp.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -145,26 +145,25 @@ private void runWithoutExceptionHandling(boolean addCacheHeader) throws Interrup
145145
}
146146
@Override
147147
public boolean fetchAll(boolean addCacheHeader) {
148-
boolean fetchAllStatus = true;
149148
_log.debug("Fetch splits starting ...");
150149
long start = _splitCache.getChangeNumber();
151150
try {
152151
runWithoutExceptionHandling(addCacheHeader);
153152
} catch (InterruptedException e) {
154-
fetchAllStatus = false;
155153
_log.warn("Interrupting split fetcher task");
156154
Thread.currentThread().interrupt();
155+
return false;
157156
} catch (Throwable t) {
158-
fetchAllStatus = false;
159157
_log.error("RefreshableSplitFetcher failed: " + t.getMessage());
160158
if (_log.isDebugEnabled()) {
161159
_log.debug("Reason:", t);
162160
}
161+
return false;
163162
} finally {
164163
if (_log.isDebugEnabled()) {
165164
_log.debug("split fetch before: " + start + ", after: " + _splitCache.getChangeNumber());
166165
}
167166
}
168-
return fetchAllStatus;
167+
return true;
169168
}
170169
}

‎client/src/main/java/io/split/engine/segments/SegmentFetcherImp.java‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -142,11 +142,7 @@ public void runWhitCacheHeader(){
142142
private void fetchAndUpdate(boolean addCacheHeader) {
143143
try {
144144
// Do this again in case the previous call errored out.
145-
_gates.registerSegment(_segmentName);
146145
callLoopRun(true, addCacheHeader);
147-
148-
_gates.segmentIsReady(_segmentName);
149-
150146
} catch (Throwable t) {
151147
_log.error("RefreshableSegmentFetcher failed: " + t.getMessage());
152148
if (_log.isDebugEnabled()) {

‎client/src/main/java/io/split/engine/segments/SegmentSynchronizationTaskImp.java‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,6 @@ public void initializeSegment(String segmentName) {
8282
return;
8383
}
8484

85-
try {
86-
_gates.registerSegment(segmentName);
87-
} catch (InterruptedException e) {
88-
_log.error("Unable to register segment " + segmentName);
89-
}
90-
9185
segment = new SegmentFetcherImp(segmentName, _segmentChangeFetcher, _gates, _segmentCache, _telemetryRuntimeProducer);
9286

9387
if (_running.get()) {

0 commit comments

Comments
 (0)