s434730/RandomSec
3
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Add exponential backoff retries for reconciliation calls. (#3770)

Browse Source 
* Update test to demonstrate retry mechanism for recon queries.

This was fixed earlier by #3237. Closes #3369.

* Ensure non-zero retry interval in HttpClient

* Restore original bound
This commit is contained in:
Antonin Delpeuch 2021-04-01 17:10:17 +02:00 committed by GitHub
parent fa9d670d30
commit 577caabec1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 12 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
main/src/com/google/refine/util/HttpClient.java
View File

@ -46,13 +46,19 @@ public class HttpClient {
private HttpClientBuilder httpClientBuilder;
private CloseableHttpClient httpClient;
private int _delay;
private int _retryInterval; // delay between original request and first retry, in ms
public HttpClient() {
this(0);
}
public HttpClient(int delay) {
this(delay, Math.max(delay, 200));
}
public HttpClient(int delay, int retryInterval) {
_delay = delay;
_retryInterval = retryInterval;
// Create a connection manager with a custom socket timeout
PoolingHttpClientConnectionManager connManager = new PoolingHttpClientConnectionManager();
final SocketConfig socketConfig = SocketConfig.custom()
@ -70,7 +76,7 @@ public class HttpClient {
.setDefaultRequestConfig(defaultRequestConfig)
.setConnectionManager(connManager)
// Default Apache HC retry is 1x @1 sec (or the value in Retry-Header)
.setRetryStrategy(new ExponentialBackoffRetryStrategy(3, TimeValue.ofMilliseconds(_delay)))
.setRetryStrategy(new ExponentialBackoffRetryStrategy(3, TimeValue.ofMilliseconds(_retryInterval)))
// .setRedirectStrategy(new LaxRedirectStrategy()) // TODO: No longer needed since default doesn't exclude POST?
// .setConnectionBackoffStrategy(ConnectionBackoffStrategy)
.addRequestInterceptorFirst(new HttpRequestInterceptor() {
@ -198,7 +204,7 @@ public class HttpClient {
// If it's the same as the default, there was no Retry-After, so use binary
// exponential backoff
if (interval.compareTo(defaultInterval) == 0) {
interval = TimeValue.of(((Double) (Math.pow(2, execCount) * defaultInterval.getDuration())).longValue(),
interval = TimeValue.of(((Double) (Math.pow(2, execCount - 1) * defaultInterval.getDuration())).longValue(),
defaultInterval.getTimeUnit() );
return interval;
}

9
main/tests/server/src/com/google/refine/model/recon/StandardReconConfigTests.java
View File

@ -337,9 +337,8 @@ public class StandardReconConfigTests extends RefineTest {
try (MockWebServer server = new MockWebServer()) {
server.start();
HttpUrl url = server.url("/openrefine-wikidata/en/api");
// FIXME: Retry doesn't currently work, but should be tested
// server.enqueue(new MockResponse().setResponseCode(503)); // service overloaded
server.enqueue(new MockResponse().setBody(reconResponse));
server.enqueue(new MockResponse().setResponseCode(503)); // service initially overloaded
server.enqueue(new MockResponse().setBody(reconResponse)); // service returns successfully
server.enqueue(new MockResponse());
String configJson = " {\n" +
@ -366,13 +365,13 @@ public class StandardReconConfigTests extends RefineTest {
process.startPerforming(pm);
Assert.assertTrue(process.isRunning());
try {
Thread.sleep(1000); // TODO: timeout will need to increase for retries
Thread.sleep(1500);
} catch (InterruptedException e) {
Assert.fail("Test interrupted");
}
Assert.assertFalse(process.isRunning());
// RecordedRequest scratchFirstRquest = server.takeRequest();
server.takeRequest(); // ignore the first request which was a 503 error
RecordedRequest request1 = server.takeRequest();
assertNotNull(request1);