[jira] [Commented] (DRILL-8009) DrillConnectionImpl#isValid() doesn't correspond JDBC API

["ASF GitHub Bot (Jira)" <ji...@apache.org>]

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

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

rymarm opened a new pull request #2333:
URL: https://github.com/apache/drill/pull/2333


   # [DRILL-8009](https://issues.apache.org/jira/browse/DRILL-8009): DrillConnectionImpl#isValid() doesn't correspond JDBC API
   
   ## Description
   
   I've add ping function which send message with [RpcMode.PING](http://drill.apache.org/apidocs/org/apache/drill/exec/proto/GeneralRPCProtos.RpcMode.html#PING) to drillbit and after that waits for a response message with [RpcMode.PONG](drill.apache.org/apidocs/org/apache/drill/exec/proto/GeneralRPCProtos.RpcMode.html#PONG). This method is used to validate connection. I add this verification to [DrillConnectionImpl#isValid(int timeout)](https://github.com/apache/drill/blob/5f5ae89392471d237605c0b5e1a337e5bc0e10ff/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillConnectionImpl.java#L522) as it is required by JDBC API [documentation](https://docs.oracle.com/en/java/javase/11/docs/api/java.sql/java/sql/Connection.html#isValid(int)) and gives opportunity accurately validate connection:
   
   >  The driver shall submit a query on the connection or use some other mechanism that positively verifies the connection is still valid when this method is called
   
   Also, I've add to [DrillConnectionImpl#isValid(int timeout)](https://github.com/apache/drill/blob/5f5ae89392471d237605c0b5e1a337e5bc0e10ff/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillConnectionImpl.java#L522) simple check for [channel#isActive()](https://netty.io/4.1/api/io/netty/channel/Channel.html#isActive--).
   
   [Previously, in Drill](https://github.com/apache/drill/commit/960f876a1945eee4eeb0d6a98c5c58dfe2eea1a9) were add `PING` and `PONG` RpcMode's, but they was primarily used for "hearbeat" function — to let drillbit know, that connection with client is still alive and now I also used it for client verification of connection to drillbit.
   
   I've used listener pattern for checking whether client got `PONG` answer or not. If you think, that this realization is not best, I have ideas how it can be implemented in other, more "netty's" way.
   
   ## Documentation
   No doc impact
   
   ## Testing
   Manual tests with cases:
   * server (drillbit) closes connection
   * server (drillbit) shut down
   * server (drillbit) process has been killed
   * client (jdbc) lost network connection
   * client (jdbc) temporary lost connection and restored it until JDBC's ping timeout has been elapsed. 
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


> DrillConnectionImpl#isValid() doesn't correspond JDBC API
> ---------------------------------------------------------
>
>                 Key: DRILL-8009
>                 URL: https://issues.apache.org/jira/browse/DRILL-8009
>             Project: Apache Drill
>          Issue Type: Bug
>          Components: Client - JDBC
>            Reporter: Maksym Rymar
>            Assignee: Maksym Rymar
>            Priority: Major
>
> {{DrillConnectionImpl#isValid()}} doesn't correspond [Java JDBC API documentation|https://docs.oracle.com/javase/7/docs/api/java/sql/Connection.html#isValid(int)]. Current implementation doesn't do actual connection verify and doesn't cover cases like drillbit termination and network issues.
> {{java.sql.Connection#isValid()}} (which extends {{DrillConnectionImpl}}) widely used in JDBC connection pools like HikariCP and without right implementation, Drill JDBC driver cannot be used with them, because connections will keep alive forever.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)