You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2022/06/16 15:00:32 UTC
[GitHub] [hive] pan3793 opened a new pull request, #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
pan3793 opened a new pull request, #3379:
URL: https://github.com/apache/hive/pull/3379
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/Hive/HowToContribute
2. Ensure that you have created an issue on the Hive project JIRA: https://issues.apache.org/jira/projects/HIVE/summary
3. Ensure you have added or run the appropriate tests for your PR:
4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]HIVE-XXXXX: Your PR title ...'.
5. Be sure to keep the PR description updated to reflect all changes.
6. Please write your PR title to summarize what this PR proposes.
7. If possible, provide a concise example to reproduce the issue for a faster review.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
Introduce a new JDBC parameter `loginTimeout` and respect DriverManger#getLoginTimeout if `loginTimeout` is absent.
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
Before [HIVE-12371](https://issues.apache.org/jira/browse/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](https://issues.apache.org/jira/browse/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.
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description, screenshot and/or a reproducable example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Hive versions or within the unreleased branches such as master.
If no, write 'No'.
-->
Yes. New JDBC parameter `loginTimeout` is introduced.
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
Existing UT (maybe).
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r905922482
##########
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:
That sounds good!
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pan3793 commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r904952237
##########
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:
Because of HIVE-12371 PR https://github.com/apache/hive/pull/1611
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r907040232
##########
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:
I would go with the 1st approach
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pan3793 commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r904935038
##########
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:
Some frameworks like Spring Boot invoke `DriverManager#loginTimeout` (30s in default), it should only affect connection timeout, but not query timeout. Before HIVE-12371, a query that exceeds 30s will see socket timeout exception. After HIVE-12371, `DriverManager#loginTimeout` does not work at all.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r904993120
##########
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:
For Postgres: https://stackoverflow.com/questions/56261716/whats-the-difference-between-logintimeout-connecttimeout-and-sockettimeout-in#:~:text=connectTimeout%20specifies%20how%20long%20to,server%20before%20throwing%20an%20error
- connectTimeout specifies how long to wait for a TCP network connection to get established
- loginTimeout specifies how long the whole process of logging into the database is allowed to take
- socketTimeout specifies how long the client will wait for a response to a command from the server before throwing an error
For MySQL, I do not see `loginTimeout`, but I might miss something
So basically the difference is that if the `login` process takes long, then we might end up avoiding the `connectTimeout` and the `socketTimeout`, but might fail when the `loginTimeout` is reached. OTOH I am not sure how the HS2 handles long login times.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pan3793 commented on pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pan3793 commented on PR #3379:
URL: https://github.com/apache/hive/pull/3379#issuecomment-1164190815
Verification
```
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: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 10.699 s - in org.apache.hive.jdbc.TestJdbcTimeout
[INFO]
[INFO] Results:
[INFO]
[INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0
```
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pan3793 commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r904604636
##########
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcTimeout.java:
##########
@@ -0,0 +1,87 @@
+/*
+ * 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.fs.Path;
+import org.apache.hadoop.hdfs.DFSUtil;
+import org.apache.hadoop.hdfs.HAUtil;
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.conf.HiveConf.ConfVars;
+import org.apache.hive.jdbc.miniHS2.MiniHS2;
+import org.apache.hive.service.cli.HiveSQLException;
+import org.apache.hive.service.cli.session.HiveSessionHook;
+import org.apache.hive.service.cli.session.HiveSessionHookContext;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.junit.*;
+
+import java.sql.*;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+public class TestJdbcTimeout {
+
+ private static MiniHS2 miniHS2 = null;
+
+ @BeforeClass
+ public static void beforeTest() throws Exception {
+ Class.forName(MiniHS2.getJdbcDriverName());
+ HiveConf conf = new HiveConf();
+ conf.setBoolVar(ConfVars.HIVE_SUPPORT_CONCURRENCY, false);
+ 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 testConnection() throws Exception {
+ AtomicInteger goodCounter = new AtomicInteger(0);
+ AtomicInteger badCounter = new AtomicInteger(0);
+ DriverManager.setLoginTimeout(2);
+ String url = miniHS2.getJdbcURL();
+ Runnable run = () -> {
+ try (Connection conn = DriverManager.getConnection(url)) {
+ goodCounter.incrementAndGet();
+ try (Statement stmt = conn.createStatement()) {
+ stmt.execute("SELECT reflect('java.lang.Thread', 'sleep', 3000L)");
+ }
+ } catch (Exception exception) {
+ badCounter.incrementAndGet();
+ }
+ };
+ Thread conn1 = new Thread(run);
+ Thread conn2 = new Thread(run);
+ conn1.start();
+ conn2.start();
+ assertEquals(goodCounter.get(), 1);
Review Comment:
WIP: I can not compile Hive on M1 machine, just push and finding a x86 machine...
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r904604282
##########
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcTimeout.java:
##########
@@ -0,0 +1,87 @@
+/*
+ * 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.fs.Path;
+import org.apache.hadoop.hdfs.DFSUtil;
+import org.apache.hadoop.hdfs.HAUtil;
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.conf.HiveConf.ConfVars;
+import org.apache.hive.jdbc.miniHS2.MiniHS2;
+import org.apache.hive.service.cli.HiveSQLException;
+import org.apache.hive.service.cli.session.HiveSessionHook;
+import org.apache.hive.service.cli.session.HiveSessionHookContext;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.junit.*;
+
+import java.sql.*;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+public class TestJdbcTimeout {
+
+ private static MiniHS2 miniHS2 = null;
+
+ @BeforeClass
+ public static void beforeTest() throws Exception {
+ Class.forName(MiniHS2.getJdbcDriverName());
+ HiveConf conf = new HiveConf();
+ conf.setBoolVar(ConfVars.HIVE_SUPPORT_CONCURRENCY, false);
+ 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 testConnection() throws Exception {
+ AtomicInteger goodCounter = new AtomicInteger(0);
+ AtomicInteger badCounter = new AtomicInteger(0);
+ DriverManager.setLoginTimeout(2);
+ String url = miniHS2.getJdbcURL();
+ Runnable run = () -> {
+ try (Connection conn = DriverManager.getConnection(url)) {
+ goodCounter.incrementAndGet();
+ try (Statement stmt = conn.createStatement()) {
+ stmt.execute("SELECT reflect('java.lang.Thread', 'sleep', 3000L)");
+ }
+ } catch (Exception exception) {
+ badCounter.incrementAndGet();
Review Comment:
Are we sure that this is the exception what we expect here?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
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
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pan3793 commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r904980949
##########
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:
> What would be this situation?
I do not know how to explain it, because I think it's fundamental two different things.
MySQL[1], PostgreSQL[2], Thrift, Socket and everything distinguish them
[1] https://dev.mysql.com/doc/connector-j/5.1/en/connector-j-connp-props-connection-authentication.html
[2] https://jdbc.postgresql.org/documentation/head/connect.html
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pan3793 commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r905980870
##########
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:
> Was this `loginTimeout` used anywhere else?
I think no, the IDEA index indicates it is not used by others.
> Do we need to keep backward compatibility?
I think not necessary, the old implementation does not right. HIVE-12371 has already broken it one time.
To clarify I think we have 3 approaches for improving the "*timeout" stuff
1. Introduce new JDBC parameter `connectTimeout` w/ exsiting `socketTimeout`, ignore `DriverManager.loginTimeout`
2. Same as 1, but use `DriverManager.loginTimeout` as `connectTimeout` if `connectTimeout` absent.
3. Introduce new JDBC parameter `loginTimeout` and `connectTimeout` w/s exsiting `socketTime`, use `loginTimeout` as `socketTimeout` for openSession request, and reset `socketTimeout` if openSession successful.
Which one do you prefer?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pan3793 commented on pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pan3793 commented on PR #3379:
URL: https://github.com/apache/hive/pull/3379#issuecomment-1161512614
Let me try to add a unit test. (Not sure if easy because it's network-related)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r904899155
##########
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:
Could you use simple jdbc connection?
`miniHS2.getJdbcURL()` could provide the jdbc url to connect to the running HS2 server
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pan3793 commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r904748435
##########
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcTimeout.java:
##########
@@ -0,0 +1,87 @@
+/*
+ * 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.fs.Path;
+import org.apache.hadoop.hdfs.DFSUtil;
+import org.apache.hadoop.hdfs.HAUtil;
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.conf.HiveConf.ConfVars;
+import org.apache.hive.jdbc.miniHS2.MiniHS2;
+import org.apache.hive.service.cli.HiveSQLException;
+import org.apache.hive.service.cli.session.HiveSessionHook;
+import org.apache.hive.service.cli.session.HiveSessionHookContext;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.junit.*;
+
+import java.sql.*;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+public class TestJdbcTimeout {
+
+ private static MiniHS2 miniHS2 = null;
+
+ @BeforeClass
+ public static void beforeTest() throws Exception {
+ Class.forName(MiniHS2.getJdbcDriverName());
+ HiveConf conf = new HiveConf();
+ conf.setBoolVar(ConfVars.HIVE_SUPPORT_CONCURRENCY, false);
+ 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 testConnection() throws Exception {
+ AtomicInteger goodCounter = new AtomicInteger(0);
+ AtomicInteger badCounter = new AtomicInteger(0);
+ DriverManager.setLoginTimeout(2);
+ String url = miniHS2.getJdbcURL();
+ Runnable run = () -> {
+ try (Connection conn = DriverManager.getConnection(url)) {
+ goodCounter.incrementAndGet();
+ try (Statement stmt = conn.createStatement()) {
+ stmt.execute("SELECT reflect('java.lang.Thread', 'sleep', 3000L)");
+ }
+ } catch (Exception exception) {
+ badCounter.incrementAndGet();
+ }
+ };
+ Thread conn1 = new Thread(run);
+ Thread conn2 = new Thread(run);
+ conn1.start();
+ conn2.start();
+ assertEquals(goodCounter.get(), 1);
Review Comment:
I'm sorry I can not compile and run the test even on an x86 machine, the IDEA is stuck on building hive-exec module... and I simplified the test to check the variable initialization only, please let me know if this is sufficient or not.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r905917249
##########
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:
The current situation is:
- We have `socketTimeout`, and use it as a `socketTimeout` and a `connectTimeout` as well
And you suggestion is to add a `connectTimeout` too, which could be different, but if not provided we can fall back to use the socketTimeout?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] github-actions[bot] closed pull request #3379: [HIVE-26336] Introduce a new JDBC parameter connectTimeout
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #3379: [HIVE-26336] Introduce a new JDBC parameter connectTimeout
URL: https://github.com/apache/hive/pull/3379
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pan3793 commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r902398221
##########
jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java:
##########
@@ -1329,21 +1328,32 @@ private String getSessionValue(String varName, String varDefault) {
return varValue;
}
- // use socketTimeout from jdbc connection url. Thrift timeout needs to be in millis
- private void setupLoginTimeout() {
- String socketTimeoutStr = sessConfMap.getOrDefault(JdbcConnectionParams.SOCKET_TIMEOUT, "0");
- long timeOut = 0;
- try {
- timeOut = Long.parseLong(socketTimeoutStr);
- } catch (NumberFormatException e) {
- LOG.info("Failed to parse socketTimeout of value " + socketTimeoutStr);
- }
- if (timeOut > Integer.MAX_VALUE) {
- loginTimeout = Integer.MAX_VALUE;
- } else if (timeOut < 0) {
- loginTimeout = 0;
+ // 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.
+ private void setupTimeout() {
+ long _loginTimeout = 0;
Review Comment:
Any suggestions here? What about use `loginTimeout` and use `this.loginTimeout` to ref member variable?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r902394644
##########
jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java:
##########
@@ -1329,21 +1328,32 @@ private String getSessionValue(String varName, String varDefault) {
return varValue;
}
- // use socketTimeout from jdbc connection url. Thrift timeout needs to be in millis
- private void setupLoginTimeout() {
- String socketTimeoutStr = sessConfMap.getOrDefault(JdbcConnectionParams.SOCKET_TIMEOUT, "0");
- long timeOut = 0;
- try {
- timeOut = Long.parseLong(socketTimeoutStr);
- } catch (NumberFormatException e) {
- LOG.info("Failed to parse socketTimeout of value " + socketTimeoutStr);
- }
- if (timeOut > Integer.MAX_VALUE) {
- loginTimeout = Integer.MAX_VALUE;
- } else if (timeOut < 0) {
- loginTimeout = 0;
+ // 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.
+ private void setupTimeout() {
+ long _loginTimeout = 0;
Review Comment:
nit: we prefer not to use `_` in the variable names
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pan3793 commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r904866013
##########
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:
Actually, I don't know how to test `loginTimeout`, does Hive have something like "session init sql"?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pan3793 commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r905038156
##########
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:
Emm... it's not suitable to map `loginTimeout` to `connectTimeout`, how about providing `connectTimeout` and `socketTimeout` and ignore `DriverManager#loginTimeout` ?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pan3793 commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r905920265
##########
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:
yeah
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pan3793 commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r907063808
##########
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:
Thanks for the confirmation, will implement it in the next few days.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r904770698
##########
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcTimeout.java:
##########
@@ -0,0 +1,87 @@
+/*
+ * 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.fs.Path;
+import org.apache.hadoop.hdfs.DFSUtil;
+import org.apache.hadoop.hdfs.HAUtil;
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.conf.HiveConf.ConfVars;
+import org.apache.hive.jdbc.miniHS2.MiniHS2;
+import org.apache.hive.service.cli.HiveSQLException;
+import org.apache.hive.service.cli.session.HiveSessionHook;
+import org.apache.hive.service.cli.session.HiveSessionHookContext;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.junit.*;
+
+import java.sql.*;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+public class TestJdbcTimeout {
+
+ private static MiniHS2 miniHS2 = null;
+
+ @BeforeClass
+ public static void beforeTest() throws Exception {
+ Class.forName(MiniHS2.getJdbcDriverName());
+ HiveConf conf = new HiveConf();
+ conf.setBoolVar(ConfVars.HIVE_SUPPORT_CONCURRENCY, false);
+ 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 testConnection() throws Exception {
+ AtomicInteger goodCounter = new AtomicInteger(0);
+ AtomicInteger badCounter = new AtomicInteger(0);
+ DriverManager.setLoginTimeout(2);
+ String url = miniHS2.getJdbcURL();
+ Runnable run = () -> {
+ try (Connection conn = DriverManager.getConnection(url)) {
+ goodCounter.incrementAndGet();
+ try (Statement stmt = conn.createStatement()) {
+ stmt.execute("SELECT reflect('java.lang.Thread', 'sleep', 3000L)");
+ }
+ } catch (Exception exception) {
+ badCounter.incrementAndGet();
+ }
+ };
+ Thread conn1 = new Thread(run);
+ Thread conn2 = new Thread(run);
+ conn1.start();
+ conn2.start();
+ assertEquals(goodCounter.get(), 1);
Review Comment:
I would like to see some test where we make sure that no future change will break the `loginTimeout`, `socketTimeout` behaviour.
Not sure about your env, but we sometime create virtual machines and run our tests there to have a linux env. This might help you too if you have access to some env.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pan3793 commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r905152566
##########
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:
New idea
```
loginTimeout = parameter.loginTimeout ? parameter.loginTimeout : DriverManager#loginTimeout
connectTimeout = parameter.connectTimeout
socketTimeout = parameter.socketTimeout
```
Before openSession
```
TSocket(connectTimeout = connectTimeout, scoketTimeout = loginTimeout)
THttpClient(connectTimeout = connectTimeout, readTimeout = loginTimeout)
```
After openSession
```
TSocket.setScoketTimeout(socketTimeout)
THttpClient.setReadTimeout(socketTimeout)
```
Does it make sense?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pan3793 commented on pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pan3793 commented on PR #3379:
URL: https://github.com/apache/hive/pull/3379#issuecomment-1185254554
> Introduce new JDBC parameter `connectTimeout` w/ exsiting `socketTimeout`, ignore `DriverManager.loginTimeout`
Implement it as we discussed before, but I found it 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 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))");
}
}
}
```
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pvary commented on PR #3379:
URL: https://github.com/apache/hive/pull/3379#issuecomment-1161507115
It would be good to create unit tests to define the expected behaviour and prevent changes in it
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] github-actions[bot] commented on pull request #3379: [HIVE-26336] Introduce a new JDBC parameter connectTimeout
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #3379:
URL: https://github.com/apache/hive/pull/3379#issuecomment-1257089693
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 dev@hive.apache.org list if the patch is in need of reviews.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r907040232
##########
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:
I would go with the 1st approach
Also set the `DriverManager.loginTimeout` to `@deprecated`
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r904962412
##########
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:
> Before [HIVE-12371](https://issues.apache.org/jira/browse/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, and does not care about DriverManager#loginTimeout anymore
So by my understanding the PR is aimed for separating the loginTimeout and the socketTimeout. So there should be a situation where having these different configurations would allow us to connect to HS2 after the PR, but not before the PR.
What would be this situation?
Thanks,
Peter
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pan3793 commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r905912850
##########
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:
> Is it possible to change the socketTimeout after opening the socket?
Yes.
At least I think it's important to have `connectTimeout` and `socketTimeout`, and nice to have `loginTimeout`.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r905951317
##########
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:
I think we should honor the `DriverManager.loginTimeout` as a socketTimeout and possible connectionTimeout if they are not set otherwise, just remember that the `DriverManager.loginTimeout` supposed to be in seconds.
As for the testing I have found this:
https://stackoverflow.com/questions/100841/artificially-create-a-connection-timeout-error
> Connect to a non-routable IP address, such as 10.255.255.1
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r905951317
##########
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:
I think we should honor the `DriverManager.loginTimeout` as a socketTimeout and possible conectionTimeout if they are not set otherwise, just remember that the `DriverManager.loginTimeout` supposed to be in seconds.
As for the testing I have found this:
https://stackoverflow.com/questions/100841/artificially-create-a-connection-timeout-error
> Connect to a non-routable IP address, such as 10.255.255.1
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pan3793 commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r904923310
##########
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:
I guess there is no difference because HiveConnection invokes openSession in the constructor
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r904943255
##########
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:
Couldn't we do the same in our tests?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pan3793 commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r904980949
##########
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:
> What would be this situation?
I do not know how to explain it, because I think they're fundamental two different things.
MySQL[1], PostgreSQL[2], Thrift, Socket and everything distinguish them
[1] https://dev.mysql.com/doc/connector-j/5.1/en/connector-j-connp-props-connection-authentication.html
[2] https://jdbc.postgresql.org/documentation/head/connect.html
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r905909040
##########
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:
Is it possible to change the socketTimeout after opening the socket?
Do we really want to have 2 different timeouts? Maybe we just have to deprecate the loginTimeout
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pan3793 commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r905941226
##########
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:
I don't know how to test `connectTimeout`, but for `socketTimeout`, I think we just need to rename `loginTimeout` to `connectTime` in this method.
Then we do not care about `loginTimeout` at all?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r904950920
##########
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:
But then, why does it not fail before the patch?
If I just copy the test and run with the current master code the test does not fail. So if Spring uses the same process, then it should not fail either. So something is missing from the picture, but I do not know what 😢
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r904929121
##########
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:
Then what does Spring Boot use? How do we get exceptions with Spring?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pan3793 commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r904862167
##########
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:
fixed
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pan3793 commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r905980870
##########
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:
> Was this `loginTimeout` used anywhere else?
I think no, the IDEA index indicates it is not used by others.
> Do we need to keep backward compatibility?
I think not necessary, the old implementation does not right. HIVE-12371 has already broken it one time.
To clarify I think we have 3 approaches for improving the "*timeout" stuff
1. Introduce new JDBC parameter `connectTimeout` w/ exsiting `socketTime`, ignore `DriverManager.loginTimeout`
2. Same as 1, but use `DriverManager.loginTimeout` as `connectTimeout` if `connectTimeout` absent.
3. Introduce new JDBC parameter `loginTimeout` and `connectTimeout` w/s exsiting `socketTime`, use `loginTimeout` as `socketTimeout` for openSession request, and reset `socketTimeout` if openSession successful.
Which one do you prefer?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pan3793 commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r905955135
##########
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:
> I think we should honor the `DriverManager.loginTimeout` as a `socketTimeout` and possible `connectionTimeout` if they are not set otherwise.
I think it make sense to use `DriverManager.loginTimeout` as `connectTimeout` but not `socketTimeout`. Comparing login or connect, a query may take hours, that is why users report socket timeout exceptions.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r905969853
##########
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:
> I think it make sense to use DriverManager.loginTimeout as connectTimeout but not socketTimeout. Comparing login or connect, a query may take hours, that is why users report socket timeout exceptions.
Was this loginTimeout used anywhere else? Do we need to keep backward compatibility?
> >As for the testing I have found this:
> >https://stackoverflow.com/questions/100841/artificially-create-a-connection-timeout-error
>
> Not sure if it's suitable, from my understanding, it can be used to test socket timeout but not connect timeout. Because the server must accept the connection and establish a TCP connection before reading a parameter(sleep=6000) passed by HTTP protocol.
We can have different tests. One for the connectTimeout, and one for the socketTimeout, and set the other to a value which does not interfere with the test.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pan3793 commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r905152566
##########
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:
New idea
```
loginTimeout = parameter.loginTimeout ? parameter.loginTimeout : DriverManager#loginTimeout
connectTimeout = parameter.connectTimeout
socketTimeout = parameter.connectTimeout
```
Before openSession
```
TSocket(connectTimeout = connectTimeout, scoketTimeout = loginTimeout)
THttpClient(connectTimeout = connectTimeout, readTimeout = loginTimeout)
```
After openSession
```
TSocket.setScoketTimeout(socketTimeout)
THttpClient.setReadTimeout(socketTimeout)
```
Does it make sense?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pan3793 commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r904948654
##########
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:
I think I have already done it in this case...
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r905922653
##########
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:
How will we test it?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pan3793 commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r904856863
##########
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:
It should fail before HIVE-12371, HIVE-12371 changes behavior but does not provide test
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pan3793 commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r905961157
##########
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:
> As for the testing I have found this:
> https://stackoverflow.com/questions/100841/artificially-create-a-connection-timeout-error
Not sure if it's suitable, from my understanding, it can be used to test socket timeout but not connect timeout. Because the server must accept the connection and establish a TCP connection before reading a parameter(sleep=6000) passed by HTTP protocol.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r904602688
##########
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcTimeout.java:
##########
@@ -0,0 +1,87 @@
+/*
+ * 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.fs.Path;
+import org.apache.hadoop.hdfs.DFSUtil;
+import org.apache.hadoop.hdfs.HAUtil;
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.conf.HiveConf.ConfVars;
+import org.apache.hive.jdbc.miniHS2.MiniHS2;
+import org.apache.hive.service.cli.HiveSQLException;
+import org.apache.hive.service.cli.session.HiveSessionHook;
+import org.apache.hive.service.cli.session.HiveSessionHookContext;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.junit.*;
+
+import java.sql.*;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+public class TestJdbcTimeout {
+
+ private static MiniHS2 miniHS2 = null;
+
+ @BeforeClass
+ public static void beforeTest() throws Exception {
+ Class.forName(MiniHS2.getJdbcDriverName());
+ HiveConf conf = new HiveConf();
+ conf.setBoolVar(ConfVars.HIVE_SUPPORT_CONCURRENCY, false);
+ 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 testConnection() throws Exception {
+ AtomicInteger goodCounter = new AtomicInteger(0);
+ AtomicInteger badCounter = new AtomicInteger(0);
+ DriverManager.setLoginTimeout(2);
+ String url = miniHS2.getJdbcURL();
+ Runnable run = () -> {
+ try (Connection conn = DriverManager.getConnection(url)) {
+ goodCounter.incrementAndGet();
+ try (Statement stmt = conn.createStatement()) {
+ stmt.execute("SELECT reflect('java.lang.Thread', 'sleep', 3000L)");
+ }
+ } catch (Exception exception) {
+ badCounter.incrementAndGet();
+ }
+ };
+ Thread conn1 = new Thread(run);
+ Thread conn2 = new Thread(run);
+ conn1.start();
+ conn2.start();
+ assertEquals(goodCounter.get(), 1);
Review Comment:
Couldn't this become flaky, if the `getConnection` is slow?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a diff in pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3379:
URL: https://github.com/apache/hive/pull/3379#discussion_r902473928
##########
jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java:
##########
@@ -1329,21 +1328,32 @@ private String getSessionValue(String varName, String varDefault) {
return varValue;
}
- // use socketTimeout from jdbc connection url. Thrift timeout needs to be in millis
- private void setupLoginTimeout() {
- String socketTimeoutStr = sessConfMap.getOrDefault(JdbcConnectionParams.SOCKET_TIMEOUT, "0");
- long timeOut = 0;
- try {
- timeOut = Long.parseLong(socketTimeoutStr);
- } catch (NumberFormatException e) {
- LOG.info("Failed to parse socketTimeout of value " + socketTimeoutStr);
- }
- if (timeOut > Integer.MAX_VALUE) {
- loginTimeout = Integer.MAX_VALUE;
- } else if (timeOut < 0) {
- loginTimeout = 0;
+ // 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.
+ private void setupTimeout() {
+ long _loginTimeout = 0;
Review Comment:
That's the hardest part of the development. Coming up with good variable names 😄
`loginTimeoutFromConf` or `confLoginTimeout` or `parsedLoginTimeout` or `longLoginTimeout`?
But if you have better ideas then I would happy to see them 😄
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pan3793 commented on pull request #3379: [HIVE-26336] Hive JDBC Driver should respect JDBC DriverManager#loginTimeout
Posted by GitBox <gi...@apache.org>.
pan3793 commented on PR #3379:
URL: https://github.com/apache/hive/pull/3379#issuecomment-1158779054
@prasanthj @pvary would you please take a look?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] shalk commented on pull request #3379: [HIVE-26336] Introduce a new JDBC parameter connectTimeout
Posted by "shalk (via GitHub)" <gi...@apache.org>.
shalk commented on PR #3379:
URL: https://github.com/apache/hive/pull/3379#issuecomment-1500098280
i think this pr can be reopen. in somecase connection timeout is different from socketTimeout.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org