Skip to content

Commit e421982

Browse files
loggkreta
andauthored
Add setRetryStrategy to ApacheHttpClient5TransportBuilder (#2086)
* Allow enabling automatic retries in ApacheHttpClient5TransportBuilder Signed-off-by: Logan Kennedy <kennedylogan22@gmail.com> * Apply suggestion from @reta Signed-off-by: Andriy Redko <drreta@gmail.com> * Apply suggestion from @reta Signed-off-by: Andriy Redko <drreta@gmail.com> * Apply suggestion from @reta Signed-off-by: Andriy Redko <drreta@gmail.com> --------- Signed-off-by: Logan Kennedy <kennedylogan22@gmail.com> Signed-off-by: Andriy Redko <drreta@gmail.com> Co-authored-by: Andriy Redko <drreta@gmail.com>
1 parent 50e47c8 commit e421982

3 files changed

Lines changed: 180 additions & 2 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
2424
- Add document lifecycle guide and runnable sample ([#2017](https://github.com/opensearch-project/opensearch-java/pull/2017))
2525
- Add transparent gRPC transport with HybridTransport (bulk over gRPC, REST fallback), translation layer, TLS, basic auth, AWS SigV4, and JWT support ([#2062](https://github.com/opensearch-project/opensearch-java/pull/2062))
2626
- Add search over gRPC with match_all query support, SearchRequestConverter, SearchResponseConverter, and _source deserialization ([#2071](https://github.com/opensearch-project/opensearch-java/pull/2071))
27+
- Add `setAutomaticRetriesDisabled` to `ApacheHttpClient5TransportBuilder` to allow enabling automatic retries ([#2086](https://github.com/opensearch-project/opensearch-java/pull/2086))
2728

2829
### Fixed
2930
- Fix `unitTest` task not running the tests in the `test` source set ([#2074](https://github.com/opensearch-project/opensearch-java/pull/2074))

java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5TransportBuilder.java

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ public class ApacheHttpClient5TransportBuilder {
7272
private HttpClientConfigCallback httpClientConfigCallback;
7373
private RequestConfigCallback requestConfigCallback;
7474
private ConnectionConfigCallback connectionConfigCallback;
75+
private boolean automaticRetriesDisabled = true;
7576
private String pathPrefix;
7677
private NodeSelector nodeSelector = NodeSelector.ANY;
7778
private boolean strictDeprecationMode = false;
@@ -163,6 +164,19 @@ public ApacheHttpClient5TransportBuilder setConnectionConfigCallback(ConnectionC
163164
return this;
164165
}
165166

167+
/**
168+
* Whether the http client should automatically retry requests. Automatic retries are disabled
169+
* by default, on par with the previous 4.x http client.
170+
*
171+
* When enabled, the http client's default retry strategy is used, and a custom
172+
* {@link org.apache.hc.client5.http.HttpRequestRetryStrategy} can be set through
173+
* {@link HttpClientConfigCallback}.
174+
*/
175+
public ApacheHttpClient5TransportBuilder setAutomaticRetriesDisabled(boolean automaticRetriesDisabled) {
176+
this.automaticRetriesDisabled = automaticRetriesDisabled;
177+
return this;
178+
}
179+
166180
/**
167181
* Sets the path's prefix for every request used by the http client.
168182
* <p>
@@ -378,8 +392,11 @@ public TlsDetails create(final SSLEngine sslEngine) {
378392
HttpAsyncClientBuilder httpClientBuilder = HttpAsyncClientBuilder.create()
379393
.setDefaultRequestConfig(requestConfigBuilder.build())
380394
.setConnectionManager(connectionManager)
381-
.setTargetAuthenticationStrategy(DefaultAuthenticationStrategy.INSTANCE)
382-
.disableAutomaticRetries();
395+
.setTargetAuthenticationStrategy(DefaultAuthenticationStrategy.INSTANCE);
396+
if (automaticRetriesDisabled) {
397+
// Keep behavior on par with the 4.x http client, which had no automatic retries
398+
httpClientBuilder = httpClientBuilder.disableAutomaticRetries();
399+
}
383400
if (httpClientConfigCallback != null) {
384401
httpClientBuilder = httpClientConfigCallback.customizeHttpClient(httpClientBuilder);
385402
}
Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.client.transport.httpclient5;
10+
11+
import static org.junit.Assert.assertEquals;
12+
import static org.junit.Assert.assertThrows;
13+
14+
import java.io.IOException;
15+
import java.io.InputStream;
16+
import java.io.OutputStream;
17+
import java.net.ServerSocket;
18+
import java.net.Socket;
19+
import java.nio.charset.StandardCharsets;
20+
import java.util.concurrent.ExecutorService;
21+
import java.util.concurrent.Executors;
22+
import java.util.concurrent.atomic.AtomicInteger;
23+
import org.apache.hc.client5.http.impl.DefaultHttpRequestRetryStrategy;
24+
import org.apache.hc.core5.http.HttpHost;
25+
import org.apache.hc.core5.util.TimeValue;
26+
import org.junit.After;
27+
import org.junit.Before;
28+
import org.junit.Test;
29+
import org.opensearch.client.opensearch.OpenSearchClient;
30+
import org.opensearch.client.opensearch.generic.Requests;
31+
import org.opensearch.client.opensearch.generic.Response;
32+
33+
/**
34+
* Tests that automatic retries can be enabled through
35+
* {@link ApacheHttpClient5TransportBuilder#setAutomaticRetriesDisabled}, and that requests are
36+
* not retried by default.
37+
*/
38+
public class AutomaticRetriesTest {
39+
private ServerSocket serverSocket;
40+
private ExecutorService serverExecutor;
41+
private volatile boolean serverRunning;
42+
private final AtomicInteger requestCount = new AtomicInteger();
43+
44+
@Before
45+
public void startRawSocketServer() throws IOException {
46+
serverSocket = new ServerSocket(0);
47+
serverExecutor = Executors.newCachedThreadPool();
48+
serverRunning = true;
49+
50+
serverExecutor.submit(() -> {
51+
while (serverRunning) {
52+
try {
53+
Socket clientSocket = serverSocket.accept();
54+
serverExecutor.submit(() -> handleClientRequest(clientSocket));
55+
} catch (IOException e) {
56+
if (serverRunning) {
57+
e.printStackTrace();
58+
}
59+
}
60+
}
61+
});
62+
}
63+
64+
@After
65+
public void stopRawSocketServer() throws IOException {
66+
serverRunning = false;
67+
if (serverSocket != null && !serverSocket.isClosed()) {
68+
serverSocket.close();
69+
}
70+
serverExecutor.shutdownNow();
71+
}
72+
73+
private void handleClientRequest(Socket clientSocket) {
74+
try (InputStream in = clientSocket.getInputStream(); OutputStream out = clientSocket.getOutputStream()) {
75+
readRequestHead(in);
76+
77+
String response;
78+
if (requestCount.incrementAndGet() == 1) {
79+
response = "HTTP/1.1 503 Service Unavailable\r\n" + "Content-Length: 0\r\n" + "Connection: close\r\n" + "\r\n";
80+
} else {
81+
String body = "{\"acknowledged\":true}";
82+
response = "HTTP/1.1 200 OK\r\n"
83+
+ "Content-Type: application/json\r\n"
84+
+ "Content-Length: "
85+
+ body.length()
86+
+ "\r\n"
87+
+ "Connection: close\r\n"
88+
+ "\r\n"
89+
+ body;
90+
}
91+
out.write(response.getBytes(StandardCharsets.UTF_8));
92+
out.flush();
93+
} catch (IOException e) {
94+
System.err.println("Server: Error handling client connection: " + e.getMessage());
95+
} finally {
96+
try {
97+
clientSocket.close();
98+
} catch (IOException e) {
99+
e.printStackTrace();
100+
}
101+
}
102+
}
103+
104+
// Reads the request line and headers, up to the terminating blank line
105+
private static void readRequestHead(InputStream in) throws IOException {
106+
int c;
107+
int newLines = 0;
108+
while (newLines < 2 && (c = in.read()) != -1) {
109+
if (c == '\n') {
110+
newLines++;
111+
} else if (c != '\r') {
112+
newLines = 0;
113+
}
114+
}
115+
}
116+
117+
@Test
118+
public void testDefaultRetryStrategyRetriesFailedRequest() throws IOException {
119+
OpenSearchClient client = createClient(builder().setAutomaticRetriesDisabled(false));
120+
121+
Response response = client.generic().execute(Requests.builder().method("GET").endpoint("/").build());
122+
123+
assertEquals(200, response.getStatus());
124+
assertEquals(2, requestCount.get());
125+
}
126+
127+
@Test
128+
public void testCustomRetryStrategySetThroughCallback() throws IOException {
129+
OpenSearchClient client = createClient(
130+
builder().setAutomaticRetriesDisabled(false)
131+
.setHttpClientConfigCallback(
132+
httpClientBuilder -> httpClientBuilder.setRetryStrategy(
133+
new DefaultHttpRequestRetryStrategy(3, TimeValue.ofMilliseconds(50))
134+
)
135+
)
136+
);
137+
138+
Response response = client.generic().execute(Requests.builder().method("GET").endpoint("/").build());
139+
140+
assertEquals(200, response.getStatus());
141+
assertEquals(2, requestCount.get());
142+
}
143+
144+
@Test
145+
public void testNoRetriesByDefault() {
146+
OpenSearchClient client = createClient(builder());
147+
148+
assertThrows(ResponseException.class, () -> client.generic().execute(Requests.builder().method("GET").endpoint("/").build()));
149+
150+
assertEquals(1, requestCount.get());
151+
}
152+
153+
private ApacheHttpClient5TransportBuilder builder() {
154+
return ApacheHttpClient5TransportBuilder.builder(new HttpHost("http", "localhost", serverSocket.getLocalPort()));
155+
}
156+
157+
private static OpenSearchClient createClient(ApacheHttpClient5TransportBuilder builder) {
158+
return new OpenSearchClient(builder.build());
159+
}
160+
}

0 commit comments

Comments
 (0)