Skip to content

Commit

Permalink
[client][test] Increase socket timeout for HTTP client5 (#544)
Browse files Browse the repository at this point in the history
Co-authored-by: Sourav Maji <[email protected]>
  • Loading branch information
majisourav99 and Sourav Maji authored Jul 25, 2023
1 parent b93d928 commit 115f6cf
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import java.util.Arrays;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import javax.net.ssl.SSLContext;
import org.apache.hc.client5.http.async.methods.SimpleHttpResponse;
import org.apache.hc.client5.http.async.methods.SimpleRequestBuilder;
Expand All @@ -29,6 +30,8 @@
* TODO: get rid of R2 Client inferface completely from venice-client.
*/
public class HttpClient5BasedR2Client {
private static final long requestTimeOutInMilliseconds = TimeUnit.SECONDS.toMillis(1); // 1s by default

public static final String HTTP_METHOD_GET_LOWER_CASE = "get";
public static final String HTTP_METHOD_POST_LOWER_CASE = "post";

Expand All @@ -44,12 +47,17 @@ public static Client getR2Client(SSLContext sslContext) throws Exception {
}

public static Client getR2Client(SSLContext sslContext, int ioThreadCount) throws Exception {
return getR2Client(sslContext, ioThreadCount, requestTimeOutInMilliseconds);
}

public static Client getR2Client(SSLContext sslContext, int ioThreadCount, long responseTimeout) throws Exception {
if (ioThreadCount <= 0) {
throw new VeniceClientException("ioThreadCount should be greater than 0");
}

final CloseableHttpAsyncClient client = new HttpClient5Utils.HttpClient5Builder().setIoThreadCount(ioThreadCount)
.setSslContext(sslContext)
.setRequestTimeOutInMilliseconds(responseTimeout)
// Disable cipher check for now.
.setSkipCipherCheck(true)
.buildAndStart();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public static class HttpClient5Builder {
private long requestTimeOutInMilliseconds = TimeUnit.SECONDS.toMillis(1); // 1s by default
/**
* We need to use a high connect timeout to avoid reconnect issue, which might result in confusing logging and unhealthy requests.
* For now, we remove the functions updating the connect timeout to avoid mistakes.
* For now, we remove the functions updating to connect timeout to avoid mistakes.
*/
private final Timeout CONNECT_TIMEOUT_IN_MILLISECONDS = Timeout.ofMilliseconds(TimeUnit.MINUTES.toMillis(1)); // 1m
// by
Expand All @@ -54,7 +54,7 @@ public HttpClient5Builder setSslContext(SSLContext sslContext) {
return this;
}

public HttpClient5Builder setRequestTimeOutInMilliseconds(int requestTimeOutInMilliseconds) {
public HttpClient5Builder setRequestTimeOutInMilliseconds(long requestTimeOutInMilliseconds) {
this.requestTimeOutInMilliseconds = requestTimeOutInMilliseconds;
return this;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public static Client getR2Client(FastClientHTTPVariant fastClientHTTPVariant) th
return setupTransportClientFactory(fastClientHTTPVariant);

case HTTP_2_BASED_HTTPCLIENT5:
return HttpClient5BasedR2Client.getR2Client(SslUtils.getVeniceLocalSslFactory().getSSLContext(), 8);
return HttpClient5BasedR2Client.getR2Client(SslUtils.getVeniceLocalSslFactory().getSSLContext(), 8, 5000);

default:
throw new VeniceException("Unsupported Http type: " + fastClientHTTPVariant);
Expand Down

0 comments on commit 115f6cf

Please sign in to comment.