[
https://issues.apache.org/jira/browse/DRILL-8307?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17606261#comment-17606261
]
ASF GitHub Bot commented on DRILL-8307:
---------------------------------------
jnturton commented on code in PR #2650:
URL: https://github.com/apache/drill/pull/2650#discussion_r973688571
##########
contrib/storage-druid/src/main/java/org/apache/drill/exec/store/druid/rest/RestClientWrapper.java:
##########
@@ -17,38 +17,36 @@
*/
package org.apache.drill.exec.store.druid.rest;
-import org.apache.http.HttpEntity;
-import org.apache.http.HttpResponse;
-import org.apache.http.client.HttpClient;
-import org.apache.http.client.methods.HttpGet;
-import org.apache.http.client.methods.HttpPost;
-import org.apache.http.entity.StringEntity;
-import org.apache.http.impl.client.DefaultHttpClient;
-
-import javax.ws.rs.core.HttpHeaders;
-import java.io.IOException;
-import java.nio.charset.Charset;
-import java.nio.charset.StandardCharsets;
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.RequestBody;
+import okhttp3.Response;
-import static javax.ws.rs.core.MediaType.APPLICATION_JSON;
-import static org.apache.http.protocol.HTTP.CONTENT_TYPE;
+import java.nio.charset.StandardCharsets;
+import java.io.IOException;
public class RestClientWrapper implements RestClient {
- private static final HttpClient httpClient = new DefaultHttpClient();
- private static final Charset DEFAULT_ENCODING = StandardCharsets.UTF_8;
+ // OkHttp client is designed to be shared across threads.
+ private final OkHttpClient httpClient = new OkHttpClient();
+
+ public Response get(String url) throws IOException {
+ Request get = new Request.Builder()
+ .url(url)
+ .addHeader("Content-Type", "application/json")
+ .build();
- public HttpResponse get(String url) throws IOException {
- HttpGet httpget = new HttpGet(url);
- httpget.addHeader(HttpHeaders.CONTENT_TYPE, APPLICATION_JSON);
- return httpClient.execute(httpget);
+ return httpClient.newCall(get).execute();
}
- public HttpResponse post(String url, String body) throws IOException {
- HttpPost httppost = new HttpPost(url);
- httppost.addHeader(CONTENT_TYPE, APPLICATION_JSON);
- HttpEntity entity = new StringEntity(body, DEFAULT_ENCODING);
- httppost.setEntity(entity);
+ public Response post(String url, String body) throws IOException {
+ RequestBody postBody =
RequestBody.create(body.getBytes(StandardCharsets.UTF_8));
Review Comment:
> Would it be possible to use the RequestBody.create method that takes a
String instead of converting to a byte[]?
>
> https://square.github.io/okhttp/#post-to-a-server
Ugh I missed that. @cgivre please may we incorporate @pjfanning's suggestion
here the EVF conversion PR.
##########
contrib/storage-druid/src/main/java/org/apache/drill/exec/store/druid/rest/RestClientWrapper.java:
##########
@@ -17,38 +17,36 @@
*/
package org.apache.drill.exec.store.druid.rest;
-import org.apache.http.HttpEntity;
-import org.apache.http.HttpResponse;
-import org.apache.http.client.HttpClient;
-import org.apache.http.client.methods.HttpGet;
-import org.apache.http.client.methods.HttpPost;
-import org.apache.http.entity.StringEntity;
-import org.apache.http.impl.client.DefaultHttpClient;
-
-import javax.ws.rs.core.HttpHeaders;
-import java.io.IOException;
-import java.nio.charset.Charset;
-import java.nio.charset.StandardCharsets;
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.RequestBody;
+import okhttp3.Response;
-import static javax.ws.rs.core.MediaType.APPLICATION_JSON;
-import static org.apache.http.protocol.HTTP.CONTENT_TYPE;
+import java.nio.charset.StandardCharsets;
+import java.io.IOException;
public class RestClientWrapper implements RestClient {
- private static final HttpClient httpClient = new DefaultHttpClient();
- private static final Charset DEFAULT_ENCODING = StandardCharsets.UTF_8;
+ // OkHttp client is designed to be shared across threads.
+ private final OkHttpClient httpClient = new OkHttpClient();
+
+ public Response get(String url) throws IOException {
+ Request get = new Request.Builder()
+ .url(url)
+ .addHeader("Content-Type", "application/json")
+ .build();
- public HttpResponse get(String url) throws IOException {
- HttpGet httpget = new HttpGet(url);
- httpget.addHeader(HttpHeaders.CONTENT_TYPE, APPLICATION_JSON);
- return httpClient.execute(httpget);
+ return httpClient.newCall(get).execute();
}
- public HttpResponse post(String url, String body) throws IOException {
- HttpPost httppost = new HttpPost(url);
- httppost.addHeader(CONTENT_TYPE, APPLICATION_JSON);
- HttpEntity entity = new StringEntity(body, DEFAULT_ENCODING);
- httppost.setEntity(entity);
+ public Response post(String url, String body) throws IOException {
+ RequestBody postBody =
RequestBody.create(body.getBytes(StandardCharsets.UTF_8));
Review Comment:
> Would it be possible to use the RequestBody.create method that takes a
String instead of converting to a byte[]?
>
> https://square.github.io/okhttp/#post-to-a-server
Ugh I missed that. @cgivre please may we incorporate @pjfanning's suggestion
here in the EVF conversion PR.
> Ensure thread safety in the Druid plugin HTTP client
> ----------------------------------------------------
>
> Key: DRILL-8307
> URL: https://issues.apache.org/jira/browse/DRILL-8307
> Project: Apache Drill
> Issue Type: Bug
> Components: Storage - Other
> Affects Versions: 1.20.2
> Reporter: James Turton
> Priority: Major
> Fix For: 1.20.3
>
>
> When multiple concurrent queries are run against a single Druid storage
> plugin then an error such as is shown below is reported by the Apache
> HttpClient used in that plugin. The Druid storage plugin uses a single static
> HttpClient instance which should be replaced with something like
> PoolingHttpClientConnectionManager or the OkHttp3 library for multithreaded
> access.
> {code:java}
> [1cdd2b75-1310-xxxx-xxxx-5a638567ed07:foreman] INFO
> o.a.d.e.s.d.s.DruidSchemaFactory
> User Error Occurred: Failure while loading druid datasources for database
> 'druid-egsmd300'. (Invalid use of BasicClientConnManager: connection still
> allocated.
> Make sure to release the connection before allocating another one.)
> org.apache.drill.common.exceptions.UserException: DATA_READ ERROR: Failure
> while loading druid datasources for database '<connection name>'.
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
