[
https://issues.apache.org/jira/browse/DRILL-8307?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17605889#comment-17605889
]
ASF GitHub Bot commented on DRILL-8307:
---------------------------------------
cgivre commented on code in PR #2650:
URL: https://github.com/apache/drill/pull/2650#discussion_r973137352
##########
contrib/storage-druid/src/main/java/org/apache/drill/exec/store/druid/rest/DruidQueryClient.java:
##########
@@ -48,20 +48,21 @@ public DruidQueryClient(String brokerURI, RestClient
restClient) {
public DruidScanResponse executeQuery(String query) throws Exception {
logger.debug("Executing Query - {}", query);
- HttpResponse response = restClient.post(queryUrl, query);
- if (response.getStatusLine().getStatusCode() != HttpStatus.SC_OK) {
- throw UserException
- .dataReadError()
- .message("Error executing druid query. HTTP request failed")
- .addContext("Response code",
response.getStatusLine().getStatusCode())
- .addContext("Response message",
response.getStatusLine().getReasonPhrase())
- .build(logger);
- }
+ try (Response response = restClient.post(queryUrl, query)) {
+ if (!response.isSuccessful()) {
+ throw UserException
Review Comment:
Would it be possible to get the `errorContext` from the `DruidBatchReader`
so that we get good error messages?
##########
contrib/storage-druid/src/main/java/org/apache/drill/exec/store/druid/rest/DruidQueryClient.java:
##########
@@ -48,20 +48,21 @@ public DruidQueryClient(String brokerURI, RestClient
restClient) {
public DruidScanResponse executeQuery(String query) throws Exception {
logger.debug("Executing Query - {}", query);
- HttpResponse response = restClient.post(queryUrl, query);
- if (response.getStatusLine().getStatusCode() != HttpStatus.SC_OK) {
- throw UserException
- .dataReadError()
- .message("Error executing druid query. HTTP request failed")
- .addContext("Response code",
response.getStatusLine().getStatusCode())
- .addContext("Response message",
response.getStatusLine().getReasonPhrase())
- .build(logger);
- }
+ try (Response response = restClient.post(queryUrl, query)) {
+ if (!response.isSuccessful()) {
+ throw UserException
Review Comment:
I see that these objects are created in the `DruidPlguin` but maybe we could
add a method `addErrorContext` or something like that and call it in the
constructor of the Batch Reader.
> 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)
