[HIVE-26336] Introduce a new JDBC parameter connectTimeout

[pan3793]

What changes were proposed in this pull request?

Introduce a new JDBC parameter connectTimeout.

Why are the changes needed?

Before HIVE-12371, the Hive JDBC Driver uses DriverManager#loginTimeout as both connectTimeout and socketTimeout, which usually cause socket timeout exception to users who use Hive JDBC Driver in Spring Boot project, because Spring Boot will setLoginTimeout to 30s (default value).

HIVE-12371 introduced a new parameter socketTimeout to replace loginTimeout, and does not care about DriverManager#loginTimeout anymore.

This PR proposes to add a new JDBC parameter connectTimeout

Does this PR introduce any user-facing change?

Yes. New JDBC parameter connectTimeout is introduced.

How was this patch tested?

New test added, and pass the existing UT.

[pan3793]

@prasanthj @pvary would you please take a look?

[pvary]

It would be good to create unit tests to define the expected behaviour and prevent changes in it

[pan3793]

Let me try to add a unit test. (Not sure if easy because it's network-related)

[pan3793]

Passed test on x86 machine, #3377 may helpful for fixing M1 building.

mvn test -Pitests -pl :hive-it-unit,:hive-jdbc -Dtest=org.apache.hive.jdbc.TestJdbcTimeout

[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running org.apache.hive.jdbc.TestJdbcTimeout
[INFO] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 23.204 s - in org.apache.hive.jdbc.TestJdbcTimeout
[INFO]
[INFO] Results:
[INFO]
[INFO] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for Hive JDBC 4.0.0-alpha-2-SNAPSHOT:
[INFO]
[INFO] Hive JDBC .......................................... SUCCESS [  4.952 s]
[INFO] Hive Integration - Unit Tests ...................... SUCCESS [ 32.634 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  38.254 s
[INFO] Finished at: 2022-06-23T18:15:33+08:00
[INFO] ------------------------------------------------------------------------

[pan3793]

Introduce new JDBC parameter connectTimeout w/ exsiting socketTimeout, ignore DriverManager.loginTimeout

Implement it as we discussed before, but I found it is not easy to add integration tests.

The following tests are what I want to add at first, but finally, I realized it does not make sense, because JDBC always runs in async mode, a "sleep" query will not block the server return the response immediately.

@Test(expected = SocketTimeoutException.class)
  public void testThrowSocketTimeoutException() throws Exception {
    String url = miniHS2.getJdbcURL("default", "socketTimeout=5000");
    try (HiveConnection conn = (HiveConnection) DriverManager.getConnection(url)) {
      try (Statement stmt = conn.createStatement()) {
        stmt.executeQuery("SELECT reflect('java.lang.Thread', 'sleep', bigint(6000))");
      }
    }
  }

  @Test
  public void testConnectTimeoutDoesNotAffectSocketTime() throws Exception {
    String url = miniHS2.getJdbcURL("default", "connectTimeout=5000");
    try (HiveConnection conn = (HiveConnection) DriverManager.getConnection(url)) {
      try (Statement stmt = conn.createStatement()) {
        stmt.executeQuery("SELECT reflect('java.lang.Thread', 'sleep', bigint(6000))");
      }
    }
  }

[github-actions[bot]]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Feel free to reach out on the [email protected] list if the patch is in need of reviews.

[shalk]

i think this pr can be reopen. in somecase connection timeout is different from socketTimeout.