[jira] [Comment Edited] (CALCITE-5719) Issues with connection and authentication with Apache Druid

["Francis Chuang (Jira)" <ji...@apache.org>]

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

Francis Chuang edited comment on CALCITE-5719 at 5/29/23 5:49 AM:
------------------------------------------------------------------

[~pjain16] Appologies for not responding earlier, I totally missed the notification in my inbox.

Addressing the issue per your points (for brevity):
1. Looking at the the interface definition for SessionResetter (https://pkg.go.dev/database/sql/driver#SessionResetter), it should only return ErrBadConn if the connection is bad and do nothing more. The current implementation of reopening a new connection is incorrect and problematic. I think the solution would be to remove the code that opens a new connection.
2. I agree, the omission of the avatica username and password should be propagated to the connection info.
3. Great catch! Would certainly love to get this in.

Since these points all touch on orthogonal issues, I think the next course of action would be to open an issue for each point and their respective PRs.


was (Author: francischuang):
[~pjain16] Appologies for not responding earlier, I totally missed the notification in my inbox.

Addressing the issue per your points (for brevity):
1. Looking at the the interface definition for SessionResetter (https://pkg.go.dev/database/sql/driver#SessionResetter), it should only return ErrBadConn if the connection is bad and do nothing more. The current implementation of reopening a new connection is incorrect and problematic. I think the solution would be to remove the code that opens a new connection.
2. I agree, the omission of the avatica username and password should be propagated to the connection info.
3. Great catch! Would certainly love to get this in.

> Issues with connection and authentication with Apache Druid
> -----------------------------------------------------------
>
>                 Key: CALCITE-5719
>                 URL: https://issues.apache.org/jira/browse/CALCITE-5719
>             Project: Calcite
>          Issue Type: Bug
>          Components: avatica-go
>            Reporter: Parag Jain
>            Assignee: Francis Chuang
>            Priority: Major
>              Labels: avatica
>
> We are using calcite-avatica-go driver to connect to Apache Druid and facing few issues -
>  # In Go sql package when an idle connection is picked from the pool, *ResetSession* method is called on the connection. In the drivers implementation of this method [here|https://github.com/apache/calcite-avatica-go/blob/main/connection.go#L236], *registerConn* method is called which actually does an *OpenConnectionRequest* with the existing connectionId. On the Druid side it actually throws an exception [here|https://github.com/apache/druid/blob/master/sql/src/main/java/org/apache/druid/sql/avatica/DruidMeta.java#L823] saying the connection is already open. I checked other driver implementations like [postgres|https://github.com/jackc/pgx/blob/master/stdlib/sql.go#L307] and [mysql|https://github.com/go-sql-driver/mysql/blob/master/connection.go#L638] for ResetSession method and they actually don't open a new connection. So not sure about the reason for doing this here. Any thoughts on this ? Example exception stack trace -
> {code:java}
> 2023-05-22T19:24:47,357 ERROR [qtp411114562-140] org.apache.druid.sql.avatica.DruidMeta - Connection [fcf73260-cb72-b867-154b-e39a19569c5e] already open.
> org.apache.druid.java.util.common.ISE: Connection [fcf73260-cb72-b867-154b-e39a19569c5e] already open.
>     at org.apache.druid.sql.avatica.DruidMeta.openDruidConnection(DruidMeta.java:823) ~[classes/:?]
>     at org.apache.druid.sql.avatica.DruidMeta.openConnection(DruidMeta.java:208) ~[classes/:?]
>     at org.apache.calcite.avatica.remote.LocalService.apply(LocalService.java:285) ~[avatica-core-1.17.0.jar:1.17.0]
>     at org.apache.calcite.avatica.remote.Service$OpenConnectionRequest.accept(Service.java:1770) ~[avatica-core-1.17.0.jar:1.17.0]
>     at org.apache.calcite.avatica.remote.Service$OpenConnectionRequest.accept(Service.java:1750) ~[avatica-core-1.17.0.jar:1.17.0]
>     at org.apache.calcite.avatica.remote.AbstractHandler.apply(AbstractHandler.java:94) ~[avatica-core-1.17.0.jar:1.17.0]
>     at org.apache.calcite.avatica.remote.ProtobufHandler.apply(ProtobufHandler.java:46) ~[avatica-core-1.17.0.jar:1.17.0]
>     at org.apache.calcite.avatica.server.AvaticaProtobufHandler.handle(AvaticaProtobufHandler.java:126) ~[avatica-server-1.17.0.jar:1.17.0]
>     at org.apache.druid.sql.avatica.DruidAvaticaProtobufHandler.handle(DruidAvaticaProtobufHandler.java:61) ~[classes/:?] {code}
>     2. Username and pwd not being propagated in the JDBC context. Had to add the following code in the [Connect|https://github.com/apache/calcite-avatica-go/blob/main/driver.go#L64] method of driver.go to make it work
> {code:java}
>     if config.avaticaUser != "" {
>         c.Info["user"] = config.avaticaUser
>     }
>     if config.avaticaPassword != "" {
>         c.Info["password"] = config.avaticaPassword
>     }  {code}
>     3. My colleague (Benjamin) found open statement leak as well, fixed [here|https://github.com/apache/calcite-avatica-go/commit/9c0eba2fbe15b3f877ee276b889cbd39e1a8ce2d] We plan to contribute all back once the issues are fixed.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)