[GitHub] drill issue #858: DRILL-3640: Support JDBC Statement.setQueryTimeout(int)

Github user kkhatua commented on the issue: https://github.com/apache/drill/pull/858 Closing this in favor of #1024 ---

[GitHub] drill issue #858: DRILL-3640: Support JDBC Statement.setQueryTimeout(int)

Github user laurentgo commented on the issue: https://github.com/apache/drill/pull/858 I think you can actually do 2 and 3 with the same approach, depending on how you compute the remaining time. --- If your project is set up for it, you can reply to this email and have your reply ap

[GitHub] drill issue #858: DRILL-3640: Support JDBC Statement.setQueryTimeout(int)

Github user parthchandra commented on the issue: https://github.com/apache/drill/pull/858 Been looking at this and the first thing that occurs to me is that we are not too clear about what the timeout means in the context of ResultSet. The API specification is rather silent on that to

[GitHub] drill issue #858: DRILL-3640: Support JDBC Statement.setQueryTimeout(int)

Github user kkhatua commented on the issue: https://github.com/apache/drill/pull/858 Wouldn't a query cancellation automatically interrupt those potentially blocking operations? I'm simply looking up whether the trigger was a timeout DrillStatement.isTimedOut() to decide if the return

[GitHub] drill issue #858: DRILL-3640: Support JDBC Statement.setQueryTimeout(int)

Github user laurentgo commented on the issue: https://github.com/apache/drill/pull/858 As for System.currentTimeMillis(), maybe not as expensive as you think: if I remember correctly, it's a JVM intrinsic (no JNI cost), and if mapped to gettimeofday (which I believe is the case), it's

[GitHub] drill issue #858: DRILL-3640: Support JDBC Statement.setQueryTimeout(int)

Github user laurentgo commented on the issue: https://github.com/apache/drill/pull/858 you should look at: - https://github.com/kkhatua/drill/blob/c51473859d1dd81cf70e857f729c3a8491b2834a/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillCursor.java#L144 - https://github

[GitHub] drill issue #858: DRILL-3640: Support JDBC Statement.setQueryTimeout(int)

Github user kkhatua commented on the issue: https://github.com/apache/drill/pull/858 @laurentgo Within the DrillCursor, the only place I could do such a check was https://github.com/kkhatua/drill/blob/c51473859d1dd81cf70e857f729c3a8491b2834a/exec/jdbc/src/main/java/org/apache/dri

[GitHub] drill issue #858: DRILL-3640: Support JDBC Statement.setQueryTimeout(int)

Github user laurentgo commented on the issue: https://github.com/apache/drill/pull/858 Thanks for the changes. That said, I still believe that it should be done using DrillCursor and the async framework already in place in Drill, and so I'm not comfortable giving my approval. Maybe @p

[GitHub] drill issue #858: DRILL-3640: Support JDBC Statement.setQueryTimeout(int)

Github user kkhatua commented on the issue: https://github.com/apache/drill/pull/858 @laurentgo I'm hoping to commit and close the PR before the long weekend... just waiting on your review of the changes :) --- If your project is set up for it, you can reply to this email and have yo

[GitHub] drill issue #858: DRILL-3640: Support JDBC Statement.setQueryTimeout(int)

Github user kkhatua commented on the issue: https://github.com/apache/drill/pull/858 @laurentgo Updated based on the review comments. The Timeout Executor Service is now maintained for the lifetime of a connection and closed during shutdown. Also refactored by introducing `isTimedOut

[GitHub] drill issue #858: DRILL-3640: Support JDBC Statement.setQueryTimeout(int)

Github user kkhatua commented on the issue: https://github.com/apache/drill/pull/858 @laurentgo I'm done with the refactoring. Please review. @parthchandra , since this will be the basis of exploring the cause for DRILL-5420 , perhaps you could do a review as well. --- If your

[GitHub] drill issue #858: DRILL-3640: Support JDBC Statement.setQueryTimeout(int)

Github user kkhatua commented on the issue: https://github.com/apache/drill/pull/858 Got it... traced it to a change in the DrillResultSetImpl. Thanks! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does

[GitHub] drill issue #858: DRILL-3640: Support JDBC Statement.setQueryTimeout(int)

Github user kkhatua commented on the issue: https://github.com/apache/drill/pull/858 @laurentgo I'm seeing JUnit test failures with this commit > ConnectionTest.testPrepareStatementBasicCaseWorks:94 » ClassCast org.apache.dr... > PreparedStatementTest.testQueryMe