Skip to content

Commit 31c7e48

Browse files
mredolattimredolatti
committed
ask for target changeNumber when sending till 
1 parent 9043ace commit 31c7e48

File tree

8 files changed

+52
-47
lines changed

8 files changed

+52
-47
lines changed

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,8 @@ public SplitChange fetch(long since, FetchOptions options) {
7373

7474
try {
7575
URIBuilder uriBuilder = new URIBuilder(_target).addParameter(SINCE, "" + since);
76-
if (options.cdnBypass()) {
77-
uriBuilder.addParameter(TILL, "" + makeRandomTill());
76+
if (options.hasCustomCN()) {
77+
uriBuilder.addParameter(TILL, "" + options.targetCN());
7878
}
7979
URI uri = uriBuilder.build();
8080

‎client/src/main/java/io/split/engine/common/FetchOptions.java

+17-12
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,15 @@
55
import java.util.function.Function;
66

77
public class FetchOptions {
8-
8+
9+
public static final Long DEFAULT_TARGET_CHANGENUMBER = -1L;
10+
911
public static class Builder {
1012

1113
public Builder() {}
1214

1315
public Builder(FetchOptions opts) {
14-
_cdnBypass = opts._cdnBypass;
16+
_targetCN = opts._targetCN;
1517
_cacheControlHeaders = opts._cacheControlHeaders;
1618
_fastlyDebugHeader = opts._fastlyDebugHeader;
1719
_responseHeadersCallback = opts._responseHeadersCallback;
@@ -32,16 +34,16 @@ public Builder responseHeadersCallback(Function<Map<String, String>, Void> callb
3234
return this;
3335
}
3436

35-
public Builder cdnBypass(boolean bypass) {
36-
_cdnBypass = bypass;
37+
public Builder targetChangeNumber(long targetCN) {
38+
_targetCN = targetCN;
3739
return this;
3840
}
3941

4042
public FetchOptions build() {
41-
return new FetchOptions(_cacheControlHeaders, _cdnBypass, _responseHeadersCallback, _fastlyDebugHeader);
43+
return new FetchOptions(_cacheControlHeaders, _targetCN, _responseHeadersCallback, _fastlyDebugHeader);
4244
}
4345

44-
private boolean _cdnBypass = false;
46+
private long _targetCN = DEFAULT_TARGET_CHANGENUMBER;
4547
private boolean _cacheControlHeaders = false;
4648
private boolean _fastlyDebugHeader = false;
4749
private Function<Map<String, String>, Void> _responseHeadersCallback = null;
@@ -55,7 +57,9 @@ public boolean fastlyDebugHeaderEnabled() {
5557
return _fastlyDebugHeader;
5658
}
5759

58-
public boolean cdnBypass() { return _cdnBypass; }
60+
public long targetCN() { return _targetCN; }
61+
62+
public boolean hasCustomCN() { return _targetCN != DEFAULT_TARGET_CHANGENUMBER; }
5963

6064
public void handleResponseHeaders(Map<String, String> headers) {
6165
if (Objects.isNull(_responseHeadersCallback) || Objects.isNull(headers)) {
@@ -65,11 +69,11 @@ public void handleResponseHeaders(Map<String, String> headers) {
6569
}
6670

6771
private FetchOptions(boolean cacheControlHeaders,
68-
boolean cdnBypass,
72+
long targetCN,
6973
Function<Map<String, String>, Void> responseHeadersCallback,
7074
boolean fastlyDebugHeader) {
7175
_cacheControlHeaders = cacheControlHeaders;
72-
_cdnBypass = cdnBypass;
76+
_targetCN = targetCN;
7377
_responseHeadersCallback = responseHeadersCallback;
7478
_fastlyDebugHeader = fastlyDebugHeader;
7579
}
@@ -84,16 +88,17 @@ public boolean equals(Object obj) {
8488

8589
return Objects.equals(_cacheControlHeaders, other._cacheControlHeaders)
8690
&& Objects.equals(_fastlyDebugHeader, other._fastlyDebugHeader)
87-
&& Objects.equals(_responseHeadersCallback, other._responseHeadersCallback);
91+
&& Objects.equals(_responseHeadersCallback, other._responseHeadersCallback)
92+
&& Objects.equals(_targetCN, other._targetCN);
8893
}
8994

9095
@Override
9196
public int hashCode() {
92-
return com.google.common.base.Objects.hashCode(_cacheControlHeaders, _fastlyDebugHeader, _responseHeadersCallback);
97+
return com.google.common.base.Objects.hashCode(_cacheControlHeaders, _fastlyDebugHeader, _responseHeadersCallback, _targetCN);
9398
}
9499

95100
private final boolean _cacheControlHeaders;
96101
private final boolean _fastlyDebugHeader;
97-
private final boolean _cdnBypass;
102+
private final long _targetCN;
98103
private final Function<Map<String, String>, Void> _responseHeadersCallback;
99104
}

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -159,16 +159,16 @@ public void refreshSplits(long targetChangeNumber) {
159159
}
160160

161161
_log.info(String.format("No changes fetched after %s attempts. Will retry bypassing CDN.", attempts));
162-
FetchOptions withCdnBypass = new FetchOptions.Builder(opts).cdnBypass(true).build();
162+
FetchOptions withCdnBypass = new FetchOptions.Builder(opts).targetChangeNumber(targetChangeNumber).build();
163163
Backoff backoff = new Backoff(ON_DEMAND_FETCH_BACKOFF_BASE_MS, ON_DEMAND_FETCH_BACKOFF_MAX_WAIT_MS);
164164
SyncResult withCDNBypassed = attemptSync(targetChangeNumber, withCdnBypass,
165165
(discard) -> backoff.interval(), ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES);
166166

167167
int withoutCDNAttempts = ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES - withCDNBypassed._remainingAttempts;
168168
if (withCDNBypassed.success()) {
169-
_log.debug(String.format("Refresh completed bypassing the CDN in %s attempts.", attempts));
169+
_log.debug(String.format("Refresh completed bypassing the CDN in %s attempts.", withoutCDNAttempts));
170170
} else {
171-
_log.debug(String.format("No changes fetched after %s attempts, even with CDN bypassed.", attempts));
171+
_log.debug(String.format("No changes fetched after %s attempts with CDN bypassed.", withoutCDNAttempts));
172172
}
173173

174174
if (_cdnResponseHeadersLogging) {

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ public void forceRefresh(FetchOptions options) {
5757
// for the next fetches. (This will clear a local copy of the fetch options,
5858
// not the original object that was passed to this method).
5959
if (initialCN == start) {
60-
options = new FetchOptions.Builder(options).cdnBypass(false).build();
60+
options = new FetchOptions.Builder(options).targetChangeNumber(FetchOptions.DEFAULT_TARGET_CHANGENUMBER).build();
6161
}
6262

6363
if (start >= end) {

‎client/src/test/java/io/split/client/HttpSplitChangeFetcherTest.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -108,11 +108,11 @@ public void testFetcherWithCDNBypassOption() throws IOException, URISyntaxExcept
108108
Metrics.NoopMetrics metrics = new Metrics.NoopMetrics();
109109
HttpSplitChangeFetcher fetcher = HttpSplitChangeFetcher.create(httpClientMock, rootTarget, metrics);
110110

111-
fetcher.fetch(-1, new FetchOptions.Builder().cdnBypass(true).build());
111+
fetcher.fetch(-1, new FetchOptions.Builder().targetChangeNumber(123).build());
112112
fetcher.fetch(-1, new FetchOptions.Builder().build());
113113
List<ClassicHttpRequest> captured = requestCaptor.getAllValues();
114114
Assert.assertEquals(captured.size(), 2);
115-
Assert.assertTrue(captured.get(0).getUri().toString().contains("till="));
115+
Assert.assertTrue(captured.get(0).getUri().toString().contains("till=123"));
116116
Assert.assertFalse(captured.get(1).getUri().toString().contains("till="));
117117
}
118118

‎client/src/test/java/io/split/engine/common/FetcherOptionsTest.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,12 @@ public Void apply(Map<String, String> unused) {
2525
.cacheControlHeaders(true)
2626
.fastlyDebugHeader(true)
2727
.responseHeadersCallback(func)
28-
.cdnBypass(true)
28+
.targetChangeNumber(123)
2929
.build();
3030

3131
assertEquals(options.cacheControlHeadersEnabled(), true);
3232
assertEquals(options.fastlyDebugHeaderEnabled(), true);
33-
assertEquals(options.cdnBypass(), true);
33+
assertEquals(options.targetCN(), 123);
3434
options.handleResponseHeaders(new HashMap<>());
3535
assertEquals(called[0], true);
3636
}

‎client/src/test/java/io/split/engine/common/SynchronizerTest.java

+21-21
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public void streamingRetryOnSegment() {
8383
}
8484

8585
@Test
86-
public void testCDNBypassIsRequestedAfterNFailures() {
86+
public void testCDNBypassIsRequestedAfterNFailures() throws NoSuchFieldException, IllegalAccessException {
8787

8888
SplitCache cache = new InMemoryCacheImp();
8989
Synchronizer imp = new SynchronizerImp(_refreshableSplitFetcherTask,
@@ -101,19 +101,19 @@ public void testCDNBypassIsRequestedAfterNFailures() {
101101
Mockito.doAnswer(invocationOnMock -> {
102102
calls.getAndIncrement();
103103
switch (calls.get()) {
104-
case 4: cache.setChangeNumber(1);
104+
case 4: cache.setChangeNumber(123);
105105
}
106106
return null;
107107
}).when(_splitFetcher).forceRefresh(optionsCaptor.capture());
108108

109-
imp.refreshSplits(1);
109+
imp.refreshSplits(123);
110110

111111
List<FetchOptions> options = optionsCaptor.getAllValues();
112112
Assert.assertEquals(options.size(), 4);
113-
Assert.assertFalse(options.get(0).cdnBypass());
114-
Assert.assertFalse(options.get(1).cdnBypass());
115-
Assert.assertFalse(options.get(2).cdnBypass());
116-
Assert.assertTrue(options.get(3).cdnBypass());
113+
Assert.assertFalse(options.get(0).hasCustomCN());
114+
Assert.assertFalse(options.get(1).hasCustomCN());
115+
Assert.assertFalse(options.get(2).hasCustomCN());
116+
Assert.assertTrue(options.get(3).hasCustomCN());
117117
}
118118

119119
@Test
@@ -154,20 +154,20 @@ public void testCDNBypassRequestLimitAndBackoff() throws NoSuchFieldException, I
154154

155155
List<FetchOptions> options = optionsCaptor.getAllValues();
156156
Assert.assertEquals(options.size(), 13);
157-
Assert.assertFalse(options.get(0).cdnBypass());
158-
Assert.assertFalse(options.get(1).cdnBypass());
159-
Assert.assertFalse(options.get(2).cdnBypass());
160-
Assert.assertTrue(options.get(3).cdnBypass());
161-
Assert.assertTrue(options.get(4).cdnBypass());
162-
Assert.assertTrue(options.get(5).cdnBypass());
163-
Assert.assertTrue(options.get(6).cdnBypass());
164-
Assert.assertTrue(options.get(7).cdnBypass());
165-
Assert.assertTrue(options.get(8).cdnBypass());
166-
Assert.assertTrue(options.get(9).cdnBypass());
167-
Assert.assertTrue(options.get(10).cdnBypass());
168-
Assert.assertTrue(options.get(11).cdnBypass());
169-
Assert.assertTrue(options.get(12).cdnBypass());
170-
157+
Assert.assertFalse(options.get(0).hasCustomCN());
158+
Assert.assertFalse(options.get(1).hasCustomCN());
159+
Assert.assertFalse(options.get(2).hasCustomCN());
160+
Assert.assertTrue(options.get(3).hasCustomCN());
161+
Assert.assertTrue(options.get(4).hasCustomCN());
162+
Assert.assertTrue(options.get(5).hasCustomCN());
163+
Assert.assertTrue(options.get(6).hasCustomCN());
164+
Assert.assertTrue(options.get(7).hasCustomCN());
165+
Assert.assertTrue(options.get(8).hasCustomCN());
166+
Assert.assertTrue(options.get(9).hasCustomCN());
167+
Assert.assertTrue(options.get(10).hasCustomCN());
168+
Assert.assertTrue(options.get(11).hasCustomCN());
169+
Assert.assertTrue(options.get(12).hasCustomCN());
170+
171171
Assert.assertEquals(calls.get(), 13);
172172
long minDiffExpected = 1 + 2 + 4 + 8 + 16 + 32 + 64 + 128 + 256;
173173
Assert.assertTrue((after - before) > minDiffExpected);

‎client/src/test/java/io/split/engine/experiments/SplitFetcherTest.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ public void testBypassCdnClearedAfterFirstHit() {
247247
ArgumentCaptor<Long> cnCaptor = ArgumentCaptor.forClass(Long.class);
248248
when(mockFetcher.fetch(cnCaptor.capture(), optionsCaptor.capture())).thenReturn(response1, response2);
249249

250-
FetchOptions originalOptions = new FetchOptions.Builder().cdnBypass(true).build();
250+
FetchOptions originalOptions = new FetchOptions.Builder().targetChangeNumber(123).build();
251251
fetcher.forceRefresh(originalOptions);
252252
List<Long> capturedCNs = cnCaptor.getAllValues();
253253
List<FetchOptions> capturedOptions = optionsCaptor.getAllValues();
@@ -258,11 +258,11 @@ public void testBypassCdnClearedAfterFirstHit() {
258258
Assert.assertEquals(capturedCNs.get(0), Long.valueOf(-1));
259259
Assert.assertEquals(capturedCNs.get(1), Long.valueOf(1));
260260

261-
Assert.assertTrue(capturedOptions.get(0).cdnBypass());
262-
Assert.assertFalse(capturedOptions.get(1).cdnBypass());
261+
Assert.assertEquals(capturedOptions.get(0).targetCN(), 123);
262+
Assert.assertEquals(capturedOptions.get(1).targetCN(), -1);
263263

264264
// Ensure that the original value hasn't been modified
265-
Assert.assertTrue(originalOptions.cdnBypass());
265+
Assert.assertEquals(originalOptions.targetCN(), 123);
266266
}
267267

268268
private SegmentChange getSegmentChange(long since, long till, String segmentName){

0 commit comments

Comments
 (0)