[ 
https://issues.apache.org/jira/browse/DRILL-8307?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17606262#comment-17606262
 ] 

ASF GitHub Bot commented on DRILL-8307:
---------------------------------------

pjfanning commented on code in PR #2650:
URL: https://github.com/apache/drill/pull/2650#discussion_r973688572


##########
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")

Review Comment:
   Why would a get request need a content-type? Content-Type on a request 
refers to the type of the request body but for GETs, this is empty. Maybe I've 
misunderstood the intent, but shouldn't this be an Accept header? An Accept 
header is used to tell the HTTP server that the client (us) would prefer to get 
the response with a particular content-type. See 
https://en.wikipedia.org/wiki/List_of_HTTP_header_fields which has a 
description of what the headers mean.





> 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)

Reply via email to