[ https://issues.apache.org/jira/browse/HIVE-26336?focusedWorklogId=784117&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-784117 ]
ASF GitHub Bot logged work on HIVE-26336: ----------------------------------------- Author: ASF GitHub Bot Created on: 23/Jun/22 10:25 Start Date: 23/Jun/22 10:25 Worklog Time Spent: 10m Work Description: pvary commented on code in PR #3379: URL: https://github.com/apache/hive/pull/3379#discussion_r904855264 ########## itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcTimeout.java: ########## @@ -0,0 +1,93 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hive.jdbc; + +import org.apache.hadoop.hive.conf.HiveConf; +import org.apache.hive.jdbc.miniHS2.MiniHS2; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +import java.sql.DriverManager; +import java.sql.Statement; +import java.util.HashMap; + +import static org.junit.Assert.assertEquals; + +public class TestJdbcTimeout { + + private static MiniHS2 miniHS2 = null; + + @BeforeClass + public static void beforeTest() throws Exception { + Class.forName(MiniHS2.getJdbcDriverName()); + HiveConf conf = new HiveConf(); + conf.setVar(HiveConf.ConfVars.HIVE_LOCK_MANAGER, "org.apache.hadoop.hive.ql.lockmgr.EmbeddedLockManager"); + miniHS2 = new MiniHS2.Builder().withConf(conf).build(); + miniHS2.start(new HashMap<>()); + } + + @AfterClass + public static void afterTest() throws Exception { + if (miniHS2 != null && miniHS2.isStarted()) { + miniHS2.stop(); + } + } + + @Test + public void testDriverManagerLoginTimeout() throws Exception { + DriverManager.setLoginTimeout(10); + String url = miniHS2.getJdbcURL("default"); + try (HiveConnection conn = (HiveConnection) DriverManager.getConnection(url)) { + assertEquals(10000, conn.getLoginTimeout()); + assertEquals(0, conn.getSocketTimeout()); + } + } + + @Test + public void testParameterLoginTimeout() throws Exception { + DriverManager.setLoginTimeout(10); + String url = miniHS2.getJdbcURL("default", "loginTimeout=20000"); + try (HiveConnection conn = (HiveConnection) DriverManager.getConnection(url)) { + assertEquals(20000, conn.getLoginTimeout()); + assertEquals(0, conn.getSocketTimeout()); + } + } + + @Test + public void testParameterSocketTimeout() throws Exception { + DriverManager.setLoginTimeout(0); + String url = miniHS2.getJdbcURL("default", "socketTimeout=10000"); + try (HiveConnection conn = (HiveConnection) DriverManager.getConnection(url)) { + assertEquals(0, conn.getLoginTimeout()); + assertEquals(10000, conn.getSocketTimeout()); + } + } + + @Test + public void testLoginTimeoutDoesNotAffectSocketTime() throws Exception { Review Comment: This test does not fail before the PR ########## itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcTimeout.java: ########## @@ -0,0 +1,93 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hive.jdbc; + +import org.apache.hadoop.hive.conf.HiveConf; +import org.apache.hive.jdbc.miniHS2.MiniHS2; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +import java.sql.DriverManager; +import java.sql.Statement; +import java.util.HashMap; + +import static org.junit.Assert.assertEquals; + +public class TestJdbcTimeout { + + private static MiniHS2 miniHS2 = null; + + @BeforeClass + public static void beforeTest() throws Exception { + Class.forName(MiniHS2.getJdbcDriverName()); + HiveConf conf = new HiveConf(); + conf.setVar(HiveConf.ConfVars.HIVE_LOCK_MANAGER, "org.apache.hadoop.hive.ql.lockmgr.EmbeddedLockManager"); + miniHS2 = new MiniHS2.Builder().withConf(conf).build(); + miniHS2.start(new HashMap<>()); + } + + @AfterClass + public static void afterTest() throws Exception { + if (miniHS2 != null && miniHS2.isStarted()) { + miniHS2.stop(); + } + } + + @Test + public void testDriverManagerLoginTimeout() throws Exception { + DriverManager.setLoginTimeout(10); + String url = miniHS2.getJdbcURL("default"); + try (HiveConnection conn = (HiveConnection) DriverManager.getConnection(url)) { + assertEquals(10000, conn.getLoginTimeout()); + assertEquals(0, conn.getSocketTimeout()); + } + } + + @Test + public void testParameterLoginTimeout() throws Exception { + DriverManager.setLoginTimeout(10); + String url = miniHS2.getJdbcURL("default", "loginTimeout=20000"); + try (HiveConnection conn = (HiveConnection) DriverManager.getConnection(url)) { + assertEquals(20000, conn.getLoginTimeout()); + assertEquals(0, conn.getSocketTimeout()); + } + } + + @Test + public void testParameterSocketTimeout() throws Exception { + DriverManager.setLoginTimeout(0); + String url = miniHS2.getJdbcURL("default", "socketTimeout=10000"); + try (HiveConnection conn = (HiveConnection) DriverManager.getConnection(url)) { + assertEquals(0, conn.getLoginTimeout()); + assertEquals(10000, conn.getSocketTimeout()); + } + } + + @Test + public void testLoginTimeoutDoesNotAffectSocketTime() throws Exception { + DriverManager.setLoginTimeout(5000); + String url = miniHS2.getJdbcURL("default"); + try (HiveConnection conn = (HiveConnection) DriverManager.getConnection(url)) { + try(Statement stmt = conn.createStatement()) { Review Comment: nit: missing space Issue Time Tracking ------------------- Worklog Id: (was: 784117) Time Spent: 2h 20m (was: 2h 10m) > Hive JDBC Driver should respect JDBC DriverManager#loginTimeout > --------------------------------------------------------------- > > Key: HIVE-26336 > URL: https://issues.apache.org/jira/browse/HIVE-26336 > Project: Hive > Issue Type: Bug > Components: JDBC > Affects Versions: 4.0.0-alpha-1 > Reporter: Cheng Pan > Priority: Major > Labels: pull-request-available > Time Spent: 2h 20m > Remaining Estimate: 0h > > Before HIVE-12371, the Hive JDBC Driver uses DriverManager#loginTimeout as > both connectTimeout and socketTimeout, which usually cause socket timeout > exceptions for users who use Hive JDBC Driver in Spring Boot project, because > Spring Boot will setLoginTimeout to 30s (default values). > HIVE-12371 introduced a new parameter socketTimeout, and does not care about > DriverManager#loginTimeout anymore, I think it's not a correct solution. > I think theĀ for loginTimeout, prefer to use loginTimeout (in milliseconds) > from jdbc connection url, and fallback to use DriverManger#getLoginTimeout > (in seconds). > For socketTimeout, use socketTimeout (in milliseconds) from jdbc connection > url if present. -- This message was sent by Atlassian Jira (v8.20.7#820007)