[
https://issues.apache.org/jira/browse/DRILL-8307?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17606287#comment-17606287
]
ASF GitHub Bot commented on DRILL-8307:
---------------------------------------
jnturton commented on code in PR #2650:
URL: https://github.com/apache/drill/pull/2650#discussion_r973710035
##########
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:
@pjfanning: also a valid question, but remember that this PR was a thread
safety fix intended for backporting to stable so I strove to disturb as little
possible while fixing the bug. Since the plugin specified this content-type
here before, and we have no bug report related to it doing so, it continues to
do that after this PR.
Maybe when @cgivre sends in the EVF version of this plugin we can see if
Druid is indeed happy for this content-type header to disappear. I'm almost
sure you're right and that will be the case.
> 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)
