This is an automated email from the ASF dual-hosted git repository.
deniskuzZ pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hive.git
The following commit(s) were added to refs/heads/master by this push:
new aaa04ef6cdf HIVE-28265: Fix JDBC timeout message for
hive.query.timeout.seconds
aaa04ef6cdf is described below
commit aaa04ef6cdf44e1cdd672e3b016e9fcbe9edf997
Author: Asish Kumar Patra <[email protected]>
AuthorDate: Fri Jul 31 21:32:27 2026 +0530
HIVE-28265: Fix JDBC timeout message for hive.query.timeout.seconds
---
.../java/org/apache/hive/jdbc/TestJdbcDriver2.java | 174 ++++++++++++++++++---
.../java/org/apache/hive/jdbc/HiveConnection.java | 35 +++--
.../java/org/apache/hive/jdbc/HiveStatement.java | 97 +++++++-----
.../hive/service/cli/operation/Operation.java | 5 +
.../hive/service/cli/operation/SQLOperation.java | 11 ++
5 files changed, 254 insertions(+), 68 deletions(-)
diff --git
a/itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java
b/itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java
index 6947b0b4b88..ac5263cfd8d 100644
--- a/itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java
+++ b/itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java
@@ -42,6 +42,7 @@
import
org.apache.hive.service.cli.operation.ClassicTableTypeMapping.ClassicTableTypes;
import org.apache.hive.service.cli.operation.HiveTableTypeMapping;
import
org.apache.hive.service.cli.operation.TableTypeMappingFactory.TableTypeMappings;
+import org.junit.After;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.Rule;
@@ -128,8 +129,45 @@ public class TestJdbcDriver2 {
private static Connection con;
private static final float floatCompareDelta = 0.0001f;
- @Rule public ExpectedException thrown = ExpectedException.none();
- @Rule public final TestName testName = new TestName();
+ /**
+ * Required prefix of {@link SQLTimeoutException#getMessage()} for a 1s
limit. HS2 may append
+ * {@code ; Query ID: ...} after the base text from {@code HiveSQLException}.
+ */
+ private static final String QUERY_TIMED_OUT_AFTER_1_SECONDS = "Query timed
out after 1 seconds";
+
+ @Rule
+ public ExpectedException thrown = ExpectedException.none();
+
+ @Rule
+ public final TestName testName = new TestName();
+
+ /**
+ * {@code SET hive.query.timeout.seconds} applies to the whole HS2 session.
Tests such as
+ * {@link #testQueryTimeoutFromSetStatement()} must not leave a short limit
on the shared
+ * {@link #con}, or unrelated tests will see {@link SQLTimeoutException}.
+ */
+ @After
+ public void resetHiveSessionQueryTimeout() throws SQLException {
+ if (con == null || con.isClosed()) {
+ return;
+ }
+ try (Statement st = con.createStatement()) {
+ try {
+ st.execute("set hive.query.timeout.seconds=0s");
+ } catch (SQLException e) {
+ LOG.warn("Could not reset hive.query.timeout.seconds after {}",
testName.getMethodName(), e);
+ }
+ }
+ }
+
+ private static void assertTimeoutMessageShowsOneSecond(String context,
SQLTimeoutException e) {
+ String msg = e.getMessage();
+ assertNotNull(context + ": message should not be null", msg);
+ assertTrue(
+ context + ": should start with " + QUERY_TIMED_OUT_AFTER_1_SECONDS
+ + " (HS2 may append ; Query ID: ...); actual=" + msg,
+ msg.startsWith(QUERY_TIMED_OUT_AFTER_1_SECONDS));
+ }
private static Connection getConnection(String prefix, String postfix)
throws SQLException {
Connection con1;
@@ -336,11 +374,39 @@ private void checkBadUrl(String url) throws SQLException {
* @throws SQLException
*/
public void testURLWithFetchSize() throws SQLException {
- Connection con = getConnection(testDbName + ";fetchSize=1234", "");
- Statement stmt = con.createStatement();
- assertEquals(stmt.getFetchSize(), 1234);
- stmt.close();
- con.close();
+ try (Connection connectionWithFetchSize = getConnection(testDbName +
";fetchSize=1234", "");
+ Statement stmt = connectionWithFetchSize.createStatement()) {
+ assertEquals(stmt.getFetchSize(), 1234);
+ }
+ }
+
+ /**
+ * Same idea as {@link #testURLWithFetchSize}: drive session behavior from
the JDBC URL instead of
+ * only {@link Statement#setQueryTimeout(int)} or an explicit {@code SET}.
The timeout is supplied
+ * in the URL query ({@code ?hive_conf_list}) per the driver format
+ * {@code jdbc:hive2://.../db;sess?hive_conf#hive_var}.
+ * <p>
+ * {@link SQLTimeoutException#getMessage()} must reflect the configured
limit (1s),
+ * not {@code after 0 seconds}.
+ */
+ @Test
+ public void testURLWithHiveQueryTimeoutSeconds() throws Exception {
+ String udfName = SleepMsUDF.class.getName();
+ // Postfix appends to the query string after test overlay / lock manager
settings.
+ try (Connection connectionWithUrlQueryTimeout = getConnection(testDbName,
"hive.query.timeout.seconds=1");
+ Statement stmt = connectionWithUrlQueryTimeout.createStatement()) {
+ stmt.execute("create temporary function sleepMsUDF as '" + udfName +
"'");
+ try {
+ stmt.executeQuery("select sleepMsUDF(t1.under_col, 5) as u0,
t1.under_col as u1, "
+ + "t2.under_col as u2 from " + tableName + " t1 join " + tableName
+ + " t2 on t1.under_col = t2.under_col");
+ fail("Expecting SQLTimeoutException");
+ } catch (SQLTimeoutException e) {
+ assertTimeoutMessageShowsOneSecond("JDBC URL
hive.query.timeout.seconds=1 (query string)", e);
+ } catch (SQLException e) {
+ fail("Expecting SQLTimeoutException, but got SQLException: " + e);
+ }
+ }
}
@Test
@@ -349,13 +415,13 @@ public void testURLWithFetchSize() throws SQLException {
* @throws SQLException
*/
public void testCreateTableAsExternal() throws SQLException {
- Connection con = getConnection(testDbName +
";hiveCreateAsExternalLegacy=true", "");
- Statement stmt = con.createStatement();
- ResultSet res = stmt.executeQuery("set hive.create.as.external.legacy");
- assertTrue("ResultSet is empty", res.next());
- assertEquals("hive.create.as.external.legacy=true", res.getObject(1));
- stmt.close();
- con.close();
+ try (Connection connection =
+ getConnection(testDbName + ";hiveCreateAsExternalLegacy=true",
"");
+ Statement stmt = connection.createStatement();
+ ResultSet res = stmt.executeQuery("set
hive.create.as.external.legacy")) {
+ assertTrue("ResultSet is empty", res.next());
+ assertEquals("hive.create.as.external.legacy=true", res.getObject(1));
+ }
}
@Test
@@ -2653,7 +2719,6 @@ public void testQueryTimeout() throws Exception {
// Test a query where timeout kicks in
// Set query timeout to 1 second
stmt.setQueryTimeout(1);
- System.err.println("Executing query: ");
try {
// The test table has 500 rows, so total query time should be ~ 2500ms
stmt.executeQuery("select sleepMsUDF(t1.under_col, 5) as u0,
t1.under_col as u1, "
@@ -2661,11 +2726,10 @@ public void testQueryTimeout() throws Exception {
+ " t2 on t1.under_col = t2.under_col");
fail("Expecting SQLTimeoutException");
} catch (SQLTimeoutException e) {
- assertNotNull(e);
- System.err.println(e.toString());
+ assertTimeoutMessageShowsOneSecond(
+ "JDBC query timeout (1s)", e);
} catch (SQLException e) {
fail("Expecting SQLTimeoutException, but got SQLException: " + e);
- e.printStackTrace();
}
// Test a query where timeout does not kick in. Set it to 5s;
@@ -2675,7 +2739,79 @@ public void testQueryTimeout() throws Exception {
stmt.executeQuery("show tables");
} catch (SQLException e) {
fail("Unexpected SQLException: " + e);
- e.printStackTrace();
+ }
+ stmt.close();
+ }
+
+ /**
+ * When the session timeout is configured via a {@code SET
hive.query.timeout.seconds=...}
+ * statement and no {@link Statement#setQueryTimeout(int)} is called, the
+ * {@link SQLTimeoutException#getMessage()} must still reflect the real
limit.
+ * Message must begin with {@link #QUERY_TIMED_OUT_AFTER_1_SECONDS};
+ * HS2 may append {@code ; Query ID: ...}.
+ */
+ @Test
+ public void testQueryTimeoutFromSetStatement() throws Exception {
+ String udfName = SleepMsUDF.class.getName();
+ Statement stmt1 = con.createStatement();
+ stmt1.execute("create temporary function sleepMsUDF as '" + udfName + "'");
+ stmt1.close();
+
+ Statement stmt = con.createStatement();
+ stmt.execute("set hive.query.timeout.seconds=1s");
+
+ try {
+ stmt.executeQuery("select sleepMsUDF(t1.under_col, 5) as u0,
t1.under_col as u1, "
+ + "t2.under_col as u2 from " + tableName + " t1 join " + tableName
+ + " t2 on t1.under_col = t2.under_col");
+ fail("Expecting SQLTimeoutException");
+ } catch (SQLTimeoutException e) {
+ assertTimeoutMessageShowsOneSecond(
+ "Session query timeout (1s)", e);
+ } catch (SQLException e) {
+ fail("Expecting SQLTimeoutException, but got SQLException: " + e);
+ }
+ stmt.close();
+ }
+
+ /**
+ * Variant of {@link #testQueryTimeoutFromSetStatement}: the {@code SET} is
issued on a separate,
+ * already-closed statement; the timed-out query runs on a brand-new
statement with no
+ * {@link Statement#setQueryTimeout(int)} call. The timeout lives on the HS2
session, so it
+ * persists across client statement instances and the server-supplied {@link
SQLTimeoutException}
+ * message must still reflect it correctly.
+ */
+ @Test
+ public void testQueryTimeoutMessagePersistedAcrossStatements() throws
Exception {
+ String udfName = SleepMsUDF.class.getName();
+ Statement stmt1 = con.createStatement();
+ stmt1.execute("create temporary function sleepMsUDF as '" + udfName + "'");
+ stmt1.close();
+
+ // SET is issued on stmt2, which is then closed – timeout must survive on
the connection
+ Statement stmt2 = con.createStatement();
+ stmt2.execute("set hive.query.timeout.seconds=1s");
+ stmt2.close();
+
+ // Brand-new statement, no setQueryTimeout() call – relies solely on the
HS2 session timeout
+ Statement stmt = con.createStatement();
+ try {
+ stmt.executeQuery("select sleepMsUDF(t1.under_col, 5) as u0,
t1.under_col as u1, "
+ + "t2.under_col as u2 from " + tableName + " t1 join " + tableName
+ + " t2 on t1.under_col = t2.under_col");
+ fail("Expecting SQLTimeoutException");
+ } catch (SQLTimeoutException e) {
+ assertTimeoutMessageShowsOneSecond("SET on closed stmt2, executeQuery on
new stmt", e);
+ } catch (SQLException e) {
+ fail("Expecting SQLTimeoutException, but got SQLException: " + e);
+ }
+
+ // A fast query must still complete when the per-statement timeout
overrides
+ stmt.setQueryTimeout(5);
+ try {
+ stmt.executeQuery("show tables");
+ } catch (SQLException e) {
+ fail("Unexpected SQLException: " + e);
}
stmt.close();
}
diff --git a/jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java
b/jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java
index 8f7c3ea8acd..7e75cd1b390 100644
--- a/jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java
+++ b/jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java
@@ -163,6 +163,7 @@
*/
public class HiveConnection implements java.sql.Connection {
private static final Logger LOG =
LoggerFactory.getLogger(HiveConnection.class);
+
private String jdbcUriString;
private String host;
private int port;
@@ -396,18 +397,7 @@ protected HiveConnection(String uri, Properties info,
executeInitSql();
}
} else {
- long retryInterval = 1000L;
- try {
- String strRetries = sessConfMap.get(JdbcConnectionParams.RETRIES);
- if (StringUtils.isNotBlank(strRetries)) {
- maxRetries = Integer.parseInt(strRetries);
- }
- String strRetryInterval =
sessConfMap.get(JdbcConnectionParams.RETRY_INTERVAL);
- if(StringUtils.isNotBlank(strRetryInterval)){
- retryInterval = Long.parseLong(strRetryInterval);
- }
- } catch(NumberFormatException e) { // Ignore the exception
- }
+ long retryInterval = getRetryIntervalMillis();
for (int numRetries = 0;;) {
try {
@@ -469,6 +459,27 @@ protected HiveConnection(String uri, Properties info,
client = newSynchronizedClient(client);
}
+ /**
+ * Gets {@code retries} and {@code retryInterval} from the session
configuration, updating
+ * {@link #maxRetries} as a side-effect, and returns the interval in
milliseconds (default 1000).
+ * Extracted from the constructor to keep its length within Sonar's 150-line
limit.
+ */
+ private long getRetryIntervalMillis() {
+ long retryInterval = 1000L;
+ try {
+ String strRetries = sessConfMap.get(JdbcConnectionParams.RETRIES);
+ if (StringUtils.isNotBlank(strRetries)) {
+ maxRetries = Integer.parseInt(strRetries);
+ }
+ String strRetryInterval =
sessConfMap.get(JdbcConnectionParams.RETRY_INTERVAL);
+ if (StringUtils.isNotBlank(strRetryInterval)) {
+ retryInterval = Long.parseLong(strRetryInterval);
+ }
+ } catch (NumberFormatException e) { // Ignore — bad values are silently
skipped
+ }
+ return retryInterval;
+ }
+
private void executeInitSql() throws SQLException {
if (initFile != null) {
try (Statement st = createStatement()) {
diff --git a/jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
b/jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
index aba982670ac..8531780e3f7 100644
--- a/jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
+++ b/jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
@@ -398,20 +398,74 @@ private TGetOperationStatusResp waitForResultSetStatus()
throws SQLException {
return statusResp;
}
+ /**
+ * Returns the timeout message for a {@code TIMEDOUT_STATE} response. The
server is authoritative
+ * when the SQL state is {@code HYT00} ("timeout expired"): it reflects the
effective operation
+ * timeout (e.g. the minimum of session {@code hive.query.timeout.seconds}
and
+ * {@link #setQueryTimeout(int)}). Otherwise (e.g. an older server) falls
back to the
+ * per-statement {@link #setQueryTimeout(int)}.
+ */
+ private String sqlTimeoutMessageForTimedOutState(String serverMessage,
String sqlState) {
+ if ("HYT00".equals(sqlState) && StringUtils.isNotBlank(serverMessage)) {
+ return serverMessage;
+ }
+ if (queryTimeout > 0) {
+ return "Query timed out after " + queryTimeout + " seconds";
+ }
+ return "Query timed out";
+ }
+
+ /**
+ * Handles one {@code GetOperationStatus} response: applies a progress
update if in-place updates
+ * are enabled, verifies the Thrift status, and dispatches on the operation
state.
+ */
+ private void processOperationStatusResponse(TGetOperationStatusResp
statusResp) throws SQLException {
+ if (!isOperationComplete && inPlaceUpdateStream.isPresent()) {
+ inPlaceUpdateStream.get().update(statusResp.getProgressUpdateResponse());
+ }
+ Utils.verifySuccessWithInfo(statusResp.getStatus());
+ if (!statusResp.isSetOperationState()) {
+ return;
+ }
+ switch (statusResp.getOperationState()) {
+ case CLOSED_STATE:
+ case FINISHED_STATE:
+ isOperationComplete = true;
+ isLogBeingGenerated = false;
+ break;
+ case CANCELED_STATE:
+ // 01000 -> warning
+ final String errMsg = statusResp.getErrorMessage();
+ final String fullErrMsg =
+ (errMsg == null || errMsg.isEmpty()) ? QUERY_CANCELLED_MESSAGE :
QUERY_CANCELLED_MESSAGE + " " + errMsg;
+ throw new SQLException(fullErrMsg, "01000");
+ case TIMEDOUT_STATE:
+ throw new SQLTimeoutException(
+ sqlTimeoutMessageForTimedOutState(statusResp.getErrorMessage(),
statusResp.getSqlState()));
+ case ERROR_STATE:
+ throw new SQLException(statusResp.getErrorMessage(),
statusResp.getSqlState(), statusResp.getErrorCode());
+ case UKNOWN_STATE:
+ throw new SQLException("Unknown query", "HY000");
+ case INITIALIZED_STATE:
+ case PENDING_STATE:
+ case RUNNING_STATE:
+ break;
+ }
+ }
+
TGetOperationStatusResp waitForOperationToComplete() throws SQLException {
TGetOperationStatusResp statusResp = null;
final TGetOperationStatusReq statusReq = new
TGetOperationStatusReq(stmtHandle.get());
- statusReq.setGetProgressUpdate(inPlaceUpdateStream.isPresent());
+ boolean progressUpdates = inPlaceUpdateStream.isPresent();
+ statusReq.setGetProgressUpdate(progressUpdates);
- // Progress bar is completed if there is nothing to request
- if (inPlaceUpdateStream.isPresent()) {
+ if (progressUpdates) {
inPlaceUpdateStream.get().getEventNotifier().progressBarCompleted();
}
LOG.debug("Waiting on operation to complete: Polling operation status");
- // Poll on the operation status, till the operation is complete
do {
try {
if (Thread.currentThread().isInterrupted()) {
@@ -424,37 +478,7 @@ TGetOperationStatusResp waitForOperationToComplete()
throws SQLException {
*/
statusResp = client.GetOperationStatus(statusReq);
LOG.debug("Status response: {}", statusResp);
- if (!isOperationComplete && inPlaceUpdateStream.isPresent()) {
-
inPlaceUpdateStream.get().update(statusResp.getProgressUpdateResponse());
- }
- Utils.verifySuccessWithInfo(statusResp.getStatus());
- if (statusResp.isSetOperationState()) {
- switch (statusResp.getOperationState()) {
- case CLOSED_STATE:
- case FINISHED_STATE:
- isOperationComplete = true;
- isLogBeingGenerated = false;
- break;
- case CANCELED_STATE:
- // 01000 -> warning
- final String errMsg = statusResp.getErrorMessage();
- final String fullErrMsg =
- (errMsg == null || errMsg.isEmpty()) ? QUERY_CANCELLED_MESSAGE
: QUERY_CANCELLED_MESSAGE + " " + errMsg;
- throw new SQLException(fullErrMsg, "01000");
- case TIMEDOUT_STATE:
- throw new SQLTimeoutException("Query timed out after " +
queryTimeout + " seconds");
- case ERROR_STATE:
- // Get the error details from the underlying exception
- throw new SQLException(statusResp.getErrorMessage(),
statusResp.getSqlState(),
- statusResp.getErrorCode());
- case UKNOWN_STATE:
- throw new SQLException("Unknown query", "HY000");
- case INITIALIZED_STATE:
- case PENDING_STATE:
- case RUNNING_STATE:
- break;
- }
- }
+ processOperationStatusResponse(statusResp);
} catch (SQLException e) {
isLogBeingGenerated = false;
throw e;
@@ -464,8 +488,7 @@ TGetOperationStatusResp waitForOperationToComplete() throws
SQLException {
}
} while (!isOperationComplete);
- // set progress bar to be completed when hive query execution has completed
- if (inPlaceUpdateStream.isPresent()) {
+ if (progressUpdates) {
inPlaceUpdateStream.get().getEventNotifier().progressBarCompleted();
}
return statusResp;
diff --git
a/service/src/java/org/apache/hive/service/cli/operation/Operation.java
b/service/src/java/org/apache/hive/service/cli/operation/Operation.java
index 6a2b12bd558..9c51b1f14f3 100644
--- a/service/src/java/org/apache/hive/service/cli/operation/Operation.java
+++ b/service/src/java/org/apache/hive/service/cli/operation/Operation.java
@@ -202,6 +202,11 @@ public long getNumModifiedRows() {
}
protected void setOperationException(HiveSQLException operationException) {
+ // Do not overwrite a completed outcome (success, cancel, timeout, etc.).
ERROR is excluded so
+ // the background thread can still record the SQLException after
runQuery() sets ERROR.
+ if (state.isTerminal() && state != OperationState.ERROR) {
+ return;
+ }
this.operationException = operationException;
}
diff --git
a/service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java
b/service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java
index 5f781a3bbb6..4caa963e2a6 100644
--- a/service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java
+++ b/service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java
@@ -177,8 +177,19 @@ private void prepare(QueryState queryState) throws
HiveSQLException {
timeoutExecutor = Executors.newSingleThreadScheduledExecutor();
timeoutExecutor.schedule(() -> {
try {
+ // The query may have already finished, failed, or been cancelled
before this task
+ // fires (the executor is only shut down when the operation is
closed). Skip in that
+ // case so we don't overwrite a real result/exception with a
spurious timeout message.
+ if (getState().isTerminal()) {
+ return null;
+ }
final String queryId = queryState.getQueryId();
log.info("Query timed out after: {} seconds. Cancelling the
execution now: {}", queryTimeout, queryId);
+ setOperationException(new HiveSQLException(
+ "Query timed out after " + queryTimeout + " seconds",
+ "HYT00",
+ 0,
+ queryId));
SQLOperation.this.cancel(OperationState.TIMEDOUT);
} catch (HiveSQLException e) {
log.error("Error cancelling the query after timeout: {} seconds",
queryTimeout, e);
