Skip to content

Commit a5e2dcc

Browse files
committed
fix: persist preferred host updates correctly in HttpScheduler retry logic
- Prevent successful response from re-establish fallback host as the preference after fallback timeout - Ensured the preferred host is only updated when necessary during retry attempts
1 parent 40f2af8 commit a5e2dcc

2 files changed

Lines changed: 130 additions & 2 deletions

File tree

lib/src/main/java/io/ably/lib/http/HttpScheduler.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,14 +196,18 @@ private String extendMessage(String msg) {
196196

197197
@Override
198198
public void run() {
199-
String candidateHost = httpCore.hosts.getPreferredHost();
199+
String preferredHost = httpCore.hosts.getPreferredHost();
200+
String candidateHost = preferredHost;
200201
int retryCountRemaining = (httpCore.hosts.fallbackHostsRemaining(candidateHost) > 0) ? httpCore.options.httpMaxRetryCount : 0;
201202

202203
while(!isCancelled) {
203204
try {
205+
boolean shouldPersist = !candidateHost.equals(preferredHost);
204206
result = httpExecuteWithRetry(candidateHost, path, requireAblyAuth);
205207
setResult(result);
206-
httpCore.hosts.setPreferredHost(candidateHost, true);
208+
if (shouldPersist) {
209+
httpCore.hosts.setPreferredHost(candidateHost, true);
210+
}
207211
break;
208212
} catch (AblyException.HostFailedException e) {
209213
if(--retryCountRemaining < 0) {

lib/src/test/java/io/ably/lib/test/rest/HttpTest.java

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,23 @@
3434
import org.mockito.invocation.InvocationOnMock;
3535
import org.mockito.stubbing.Answer;
3636

37+
import io.ably.lib.debug.DebugOptions;
38+
import io.ably.lib.network.HttpRequest;
39+
3740
import java.io.IOException;
3841
import java.net.MalformedURLException;
3942
import java.net.URL;
4043
import java.util.ArrayList;
4144
import java.util.Arrays;
45+
import java.util.Collections;
4246
import java.util.List;
47+
import java.util.Map;
48+
import java.util.concurrent.CountDownLatch;
49+
import java.util.concurrent.ExecutorService;
50+
import java.util.concurrent.Executors;
51+
import java.util.concurrent.Future;
52+
import java.util.concurrent.TimeUnit;
53+
import java.util.concurrent.atomic.AtomicInteger;
4354

4455
import static org.hamcrest.Matchers.is;
4556
import static org.hamcrest.Matchers.equalTo;
@@ -1382,4 +1393,117 @@ public void describeTo(Description description) {
13821393
description.appendText(errorInfo.toString());
13831394
}
13841395
}
1396+
1397+
/**
1398+
* Verifies RSC15f: a late-arriving success from a fallback host must not
1399+
* re-pin the fallback when the fallbackRetryTimeout has already expired.
1400+
*
1401+
* Sequence:
1402+
* req1 primary → 500 (triggers fallback)
1403+
* req2 fallback → 200 immediate (pins fallback, timeout = 100 ms)
1404+
* req3 fallback → 200 delayed 200 ms (in-flight when timeout expires)
1405+
* req4 primary → 200 immediate (timeout expired → primary tried again)
1406+
* req5 primary → 200 immediate (late req3 success must NOT have re-pinned)
1407+
*/
1408+
@Test
1409+
public void http_fallback_late_success_does_not_repin_expired_timeout() throws Exception {
1410+
final String primaryHost = "main.realtime.ably.net";
1411+
final String fallbackHost = "main.a.fallback.ably-realtime.com";
1412+
1413+
final List<String> capturedHosts = Collections.synchronizedList(new ArrayList<>());
1414+
final CountDownLatch delayedRequestStarted = new CountDownLatch(1);
1415+
final AtomicInteger requestCount = new AtomicInteger(0);
1416+
1417+
DebugOptions opts = new DebugOptions("appId.keyId:keySecret");
1418+
opts.restHost = primaryHost;
1419+
opts.fallbackHosts = new String[]{fallbackHost};
1420+
opts.fallbackRetryTimeout = 100L;
1421+
opts.httpMaxRetryCount = 1;
1422+
opts.httpListener = new DebugOptions.RawHttpListener() {
1423+
@Override
1424+
public HttpCore.Response onRawHttpRequest(String id, HttpRequest request, String authHeader,
1425+
Map<String, List<String>> requestHeaders, HttpCore.RequestBody requestBody) {
1426+
int n = requestCount.incrementAndGet();
1427+
capturedHosts.add(request.getUrl().getHost());
1428+
1429+
HttpCore.Response r = new HttpCore.Response();
1430+
switch (n) {
1431+
case 1:
1432+
r.statusCode = 500;
1433+
r.statusLine = "Internal Server Error";
1434+
break;
1435+
case 2:
1436+
r.statusCode = 200;
1437+
r.contentType = "application/json";
1438+
r.body = "[1000]".getBytes();
1439+
r.contentLength = r.body.length;
1440+
break;
1441+
case 3:
1442+
delayedRequestStarted.countDown();
1443+
try { Thread.sleep(200L); } catch (InterruptedException e) {
1444+
Thread.currentThread().interrupt();
1445+
}
1446+
r.statusCode = 200;
1447+
r.contentType = "application/json";
1448+
r.body = "[2000]".getBytes();
1449+
r.contentLength = r.body.length;
1450+
break;
1451+
case 4:
1452+
r.statusCode = 200;
1453+
r.contentType = "application/json";
1454+
r.body = "[3000]".getBytes();
1455+
r.contentLength = r.body.length;
1456+
break;
1457+
case 5:
1458+
r.statusCode = 200;
1459+
r.contentType = "application/json";
1460+
r.body = "[4000]".getBytes();
1461+
r.contentLength = r.body.length;
1462+
break;
1463+
default:
1464+
r.statusCode = 500;
1465+
r.statusLine = "Unexpected extra request";
1466+
}
1467+
return r;
1468+
}
1469+
@Override public void onRawHttpResponse(String id, String method, HttpCore.Response r) {}
1470+
@Override public void onRawHttpException(String id, String method, Throwable t) {}
1471+
};
1472+
1473+
AblyRest client = new AblyRest(opts);
1474+
1475+
// req1 (primary→500) + req2 (fallback→200); fallback is now pinned
1476+
client.time();
1477+
1478+
// req3: fire in background — hits pinned fallback, sleeps 200 ms inside listener
1479+
ExecutorService executor = Executors.newSingleThreadExecutor();
1480+
Future<Long> requestFuture = executor.submit(() -> {
1481+
try { return client.time(); }
1482+
catch (AblyException e) { throw new RuntimeException(e); }
1483+
});
1484+
1485+
// Wait until req3 has actually entered the listener before starting the clock
1486+
assertTrue("Delayed request must start within 5 s", delayedRequestStarted.await(5, TimeUnit.SECONDS));
1487+
1488+
// Wait 150 ms so that fallbackRetryTimeout (100 ms) expires
1489+
Thread.sleep(150L);
1490+
1491+
// req4: timeout expired → primary tried again
1492+
client.time();
1493+
1494+
// Wait for req3's delayed response to arrive (late fallback success)
1495+
requestFuture.get(5, TimeUnit.SECONDS);
1496+
1497+
// req5: late success from req3 must NOT have re-pinned the fallback
1498+
client.time();
1499+
1500+
executor.shutdown();
1501+
1502+
assertEquals(5, capturedHosts.size());
1503+
assertEquals(primaryHost, capturedHosts.get(0)); // req1 – primary fails
1504+
assertEquals(fallbackHost, capturedHosts.get(1)); // req2 – fallback pins
1505+
assertEquals(fallbackHost, capturedHosts.get(2)); // req3 – in-flight fallback
1506+
assertEquals(primaryHost, capturedHosts.get(3)); // req4 – timeout expired
1507+
assertEquals(primaryHost, capturedHosts.get(4)); // req5 – late success did not re-pin
1508+
}
13851509
}

0 commit comments

Comments
 (0)