HIVE-28265: Fix JDBC timeout message for hive.query.timeout.seconds by ashniku · Pull Request #6412 · apache/hive

ashniku · 2026-04-06T08:09:54Z

What changes were proposed in this pull request?

hive.query.timeout.seconds is enforced correctly, but Beeline/JDBC reported Query timed out after 0 seconds when Statement.setQueryTimeout was not used.

This PR:

SQLOperation (HiveServer2)
Before cancel(OperationState.TIMEDOUT), set a HiveSQLException whose message is Query timed out after seconds, using the effective operation timeout ( is the same value used to schedule the cancel). GetOperationStatus then exposes the right text via operationException.
For async execution, do not call setOperationException from the background thread if the operation is already TIMEDOUT, so the timeout message is not overwritten.
HiveStatement (JDBC)
On TIMEDOUT_STATE, prefer the server errorMessage. If it is missing or clearly wrong (contains after 0 seconds), build the client message from Statement query timeout or from the last SET hive.query.timeout.seconds=... value tracked on HiveConnection. SET is detected with a regex find() so assignments can appear inside a longer script (last match wins).
HiveConnection
Stores the last parsed hive.query.timeout.seconds from a successful SET for use in the timeout message when needed.
Tests

==> testQueryTimeoutMessageUsesHiveConf: session SET hive.query.timeout.seconds=1s, no setQueryTimeout, slow query via existing SleepMsUDF — expects SQLTimeoutException and message not claiming after 0 seconds, and containing 1.
==> testQueryTimeout: same checks for the existing setQueryTimeout(1) path.

-->

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

InvisibleProgrammer · 2026-04-07T07:18:31Z

      fail("Expecting SQLTimeoutException");
    } catch (SQLTimeoutException e) {
      assertNotNull(e);
+      assertTrue("Message should reflect JDBC query timeout (1s): " + e.getMessage(),


Can you show us an example about the whole output that demonstrates the change?

I wonder if it is possible to get any number other than the timeout in the message. Like a timestamp or maybe a query id, host name, etc. Asserting to a single number in a string looks a little bit fragile to me.

Introduced constant QUERY_TIMED_OUT_AFTER_1_SECONDS = "Query timed out after 1 seconds" with Javadoc that this is the full message from HS2 / client (no query id, host, timestamp in that string for these paths).
testQueryTimeout now uses assertEquals, expected value = that constant, with a failure message that repeats the example text.

InvisibleProgrammer · 2026-04-07T07:54:32Z

+   * {@code N == 1} with flexible whitespace so we do not treat {@code 10} or unrelated digits as {@code 1}.
+   */
+  private static boolean isQueryTimedOutAfterOneSecondMessage(String msg) {
+    return msg != null && msg.matches("(?is).*timed out after\\s+1\\s+seconds.*");


We know it is 1 sec. And we don't accept any other output in that case.
In my opinion, regex here can be a little bit overkill.

What about something like:

final String expectedMessage = "Query timed out after 1 seconds"; assertEquals("Message should reflect JDBC query timeout", expectedMesage, message);

Removed isQueryTimedOutAfterOneSecondMessage (regex helper).
Positive assertions use assertEquals("…", QUERY_TIMED_OUT_AFTER_1_SECONDS, e.getMessage()) in both timeout-related tests.

InvisibleProgrammer · 2026-04-07T07:55:46Z

      assertNotNull(e);
+      assertTrue("Message should reflect JDBC query timeout (1s): " + e.getMessage(),
+          isQueryTimedOutAfterOneSecondMessage(e.getMessage()));
+      assertFalse("Message should not claim 0 seconds: " + e.getMessage(),


Considering the previous assertion, is that assertion possible at all?

(assertFalse(… "after 0 seconds") after the positive check in testQueryTimeout — “is that even possible?”)

Changes

Removed assertFalse(..., e.getMessage().contains("after 0 seconds")) from testQueryTimeout.

InvisibleProgrammer · 2026-04-07T07:57:57Z

+          + " t2 on t1.under_col = t2.under_col");
+      fail("Expecting SQLTimeoutException");
+    } catch (SQLTimeoutException e) {
+      assertNotNull(e);


Is it possible having an exception with null value in a catch block?

(assertNotNull(e) in catch (SQLTimeoutException e) — can e be null?)

Changes

Removed assertNotNull(e) from both SQLTimeoutException catch blocks in these tests.

InvisibleProgrammer · 2026-04-07T08:00:10Z

+      assertNotNull(e);
+      assertTrue("Message should include session timeout (1s): " + e.getMessage(),
+          isQueryTimedOutAfterOneSecondMessage(e.getMessage()));
+      assertFalse("Message should not claim 0 seconds (HIVE-28265): " + e.getMessage(),


What is the benefits of putting the ticket number into assertions or comments?

Dropped ticket id from assertion messages.
Kept HIVE-28265 and expected message behavior in testQueryTimeoutMessageUsesHiveConf Javadoc (and the constant / assertEquals text describes behavior without the ticket).

InvisibleProgrammer · 2026-04-07T08:00:27Z

+      assertNotNull(e);
+      assertTrue("Message should include session timeout (1s): " + e.getMessage(),
+          isQueryTimedOutAfterOneSecondMessage(e.getMessage()));
+      assertFalse("Message should not claim 0 seconds (HIVE-28265): " + e.getMessage(),


Considering the previous assertion, is that assertion possible at all?

Removed assertFalse(..., "after 0 seconds") from testQueryTimeoutMessageUsesHiveConf.

InvisibleProgrammer · 2026-04-07T08:07:08Z

+   * Sentinel: no {@code SET hive.query.timeout.seconds} has been observed on this connection yet.
+   */
+  static final long SESSION_QUERY_TIMEOUT_NOT_TRACKED = -1L;
+  private final AtomicLong sessionQueryTimeoutSeconds = new AtomicLong(SESSION_QUERY_TIMEOUT_NOT_TRACKED);


Thinking out loud: I wonder if a connection can have concurrency issue: I mean, you can have multiple individual connections to Hive, but inside a connection itself, can we have multiple hive statements in parallel?
I have no such use case in my mind, but let me ping Ayush about this question.

@ayushtkn , what do you think?

a single JDBC Connection can be shared across multiple threads, and it is entirely possible to have multiple HiveStatement objects executing concurrently on the same connection (which maps to a single session on the HS2 side).

via Beeline or so maybe not but In Hive Server 2 (HS2), a single JDBC Connection corresponds to a single HS2 Session. You can absolutely execute multiple queries concurrently within the same session by spawning multiple threads on the client side, each using a different HiveStatement created from that single HiveConnection.

InvisibleProgrammer · 2026-04-07T08:08:03Z

+   * Records the effective {@code hive.query.timeout.seconds} (in seconds) after a successful
+   * {@code SET hive.query.timeout.seconds=...} on this connection. Used for JDBC timeout messages.
+   */
+  void recordSessionQueryTimeoutFromSet(long seconds) {


Can we keep the getter..setter naming pattern?

recordSessionQueryTimeoutFromSet(long) → setSessionQueryTimeoutSeconds(long)
getSessionQueryTimeoutSecondsTracked() → getSessionQueryTimeoutSeconds()
HiveStatement updated to call the new names.

Parse hive.query.timeout.seconds from connParams.getHiveConfs() at connect time using HiveConf.getTimeVar (same semantics as HiveStatement SET path) so JDBC timeout messages work when the timeout is set via URL only. Made-with: Cursor

…ements testQueryTimeoutMessagePersistedAcrossStatements verifies that when SET hive.query.timeout.seconds is issued on a separate closed statement, the tracked value on HiveConnection still drives the SQLTimeoutException message on a subsequent new statement (no setQueryTimeout call). Made-with: Cursor

…ctor Extract retry-config parsing from HiveConnection(uri,info,...,initSession) into readRetryIntervalMillis() so the constructor fits within Sonar's 150-line limit (was 151, now 142). Made-with: Cursor

Made-with: Cursor

The reviewer (InvisibleProgrammer) correctly pointed out that scanning every executed SQL string with a regex to detect SET hive.query.timeout.seconds is the wrong approach. The right source is TGetOperationStatusResp.errorMessage: SQLOperation already sets "Query timed out after N seconds" with the real value before cancel(TIMEDOUT), so the client-side regex fallback is unnecessary. Remove: - Pattern SET_HIVE_QUERY_TIMEOUT_SECONDS field - trackSessionQueryTimeoutIfSet(sql) method and its call site - Unused imports (HiveConf, TimeUnit, Matcher, Pattern) The timeout message resolution is now: 1. Server errorMessage from TGetOperationStatusResp (primary - correct approach) 2. Statement-level setQueryTimeout() (JDBC standard) 3. URL-seeded hive.query.timeout.seconds from applySessionQueryTimeoutFromJdbcUrl() All three paths avoid per-statement SQL parsing. Made-with: Cursor

Made-with: Cursor

- Remove SESSION_QUERY_TIMEOUT_NOT_TRACKED constant; inline -1L - Rewrite Javadocs on setSessionQueryTimeoutSeconds, applySessionQueryTimeoutFromJdbcUrl, sqlTimeoutMessageForTimedOutState, and processOperationStatusResponse to drop ticket numbers and implementation-history notes - Rename testQueryTimeoutMessageUsesHiveConf to testQueryTimeoutFromSetStatement; update its Javadoc and cross-refs - Remove redundant assertFalse from assertTimeoutMessageShowsOneSecond

…000 comment - sqlTimeoutMessageForTimedOutState: check statusResp.getSqlState() == "HYT00" instead of inspecting message text; remove needsLocalTimeoutMessageForTimedOut - sqlExceptionForCanceledState: restore // SQLSTATE 01000 = warning comment

…veConf) Replace new HiveConf() + getTimeVar with HiveConf.toTime(raw, SECONDS, SECONDS) in applySessionQueryTimeoutFromJdbcUrl. This parses the duration string directly without loading Hadoop/Hive configuration resources at connect time.

Restore PartitionManagementTask to upstream master. Test counters were re-introduced during rebase but upstream removed them in HIVE-29642; they are unrelated to the JDBC timeout fix and can fail CI.

Fork PRs cannot publish GitHub Checks from Jenkins, which marks the PostProcess junit step UNSTABLE with "No suitable checks publisher found" even when all tests pass. Use skipPublishingChecks so results are still recorded without failing the build on that infrastructure limitation. Co-authored-by: Cursor <cursoragent@cursor.com>

AtomicLong is unnecessary because we only perform simple get/set on this field; volatile provides the required cross-thread visibility.

Remove redundant sessionQueryTimeoutSeconds field and parse hive.query.timeout.seconds from connParams.getHiveConfs() on demand. Revert out-of-scope sqlExceptionForCanceledState refactor and Jenkinsfile change per review.

Remove client-side session timeout tracking; rely on server HYT00 message. Client fallback is per-statement setQueryTimeout() only. Skip timeout cancel when the operation is already in a terminal state.

Follow existing getXXX naming convention in HiveConnection.

Centralize the guard in Operation.setOperationException() per review. The timeout executor sets the HYT00 message before cancel(TIMEDOUT); the background query thread must not overwrite it from its catch blocks.

Do not let a late timeout overwrite FINISHED/CANCELED outcomes. Keep ERROR excluded so the background thread can still record the SQLException. Synchronize the timeout handler check, setOperationException, and cancel.

Ignore HYT00 server text containing "after 0 seconds" and fall back to client timeout message. Add TestHiveStatement coverage. Remove synchronized from timeout handler and redundant TIMEDOUT check per review.

Document that the server HYT00 message reflects the effective operation timeout (min of session hive.query.timeout.seconds and setQueryTimeout), not the session setting alone.

Trust server HYT00 message; client fallback is setQueryTimeout() only. Remove isUsableServerTimeoutMessage and related test.

@after

Use try-with-resources, inner catch in @after, reuse stmt, and remove debug System.err/printStackTrace from timeout tests.

sonarqubecloud · 2026-07-28T15:46:43Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

ashniku

@deniskuzZ Reformatted as requested in 8072ae7 — constant on one line, @rule on separate lines. Also rebased onto latest master to fix Jenkins checkout failure. Thanks!

asf-ci-hive added tests pending tests failed tests unstable and removed tests pending tests failed tests unstable labels Apr 6, 2026

ashniku force-pushed the HIVE-28265 branch from dc768f0 to ab425d3 Compare April 7, 2026 03:21

asf-ci-hive added tests pending tests unstable and removed tests unstable tests pending labels Apr 7, 2026

InvisibleProgrammer reviewed Apr 7, 2026

View reviewed changes

asf-ci-hive added tests pending tests passed and removed tests unstable tests pending tests passed labels Apr 7, 2026

ashniku and others added 28 commits July 28, 2026 19:48

HIVE-28265: Fix Sonar method-length warning in HiveConnection constru…

8544da7

…ctor Extract retry-config parsing from HiveConnection(uri,info,...,initSession) into readRetryIntervalMillis() so the constructor fits within Sonar's 150-line limit (was 151, now 142). Made-with: Cursor

HIVE-28265: Trigger CI

c428e35

Made-with: Cursor

HIVE-28265: Trigger CI re-run

57cc228

Made-with: Cursor

HIVE-28265: Trigger CI re-run

31d2bbd

HIVE-28265: Drop stray PartitionManagementTask changes from PR

af78bf9

Restore PartitionManagementTask to upstream master. Test counters were re-introduced during rebase but upstream removed them in HIVE-29642; they are unrelated to the JDBC timeout fix and can fail CI.

HIVE-28265: Trigger CI re-run

c6459b4

HIVE-28265: Re-trigger CI

5ecfc3d

HIVE-28265: Use volatile long for session query timeout tracking

2ae8009

AtomicLong is unnecessary because we only perform simple get/set on this field; volatile provides the required cross-thread visibility.

HIVE-28265: Simplify JDBC timeout handling per review

0250957

Remove client-side session timeout tracking; rely on server HYT00 message. Client fallback is per-statement setQueryTimeout() only. Skip timeout cancel when the operation is already in a terminal state.

HIVE-28265: Rename readRetryIntervalMillis to getRetryIntervalMillis

64c8ebe

Follow existing getXXX naming convention in HiveConnection.

HIVE-28265: Re-trigger CI

fd87e4b

HIVE-28265: Do not override exception when operation is TIMEDOUT

fccbd55

Centralize the guard in Operation.setOperationException() per review. The timeout executor sets the HYT00 message before cancel(TIMEDOUT); the background query thread must not overwrite it from its catch blocks.

HIVE-28265: Guard setOperationException for all terminal states

c4acfd5

Do not let a late timeout overwrite FINISHED/CANCELED outcomes. Keep ERROR excluded so the background thread can still record the SQLException. Synchronize the timeout handler check, setOperationException, and cancel.

HIVE-28265: Reject bad server timeout messages; simplify guards

7cf38b6

Ignore HYT00 server text containing "after 0 seconds" and fall back to client timeout message. Add TestHiveStatement coverage. Remove synchronized from timeout handler and redundant TIMEDOUT check per review.

HIVE-28265: Clarify timeout message Javadoc for effective limit

70ca620

Document that the server HYT00 message reflects the effective operation timeout (min of session hive.query.timeout.seconds and setQueryTimeout), not the session setting alone.

HIVE-28265: Remove after-0-seconds message filter per review

42f8d16

Trust server HYT00 message; client fallback is setQueryTimeout() only. Remove isUsableServerTimeoutMessage and related test.

HIVE-28265: Re-trigger CI

2c3ba70

HIVE-28265: Clean up TestJdbcDriver2 per review

8c23995

Use try-with-resources, inner catch in @after, reuse stmt, and remove debug System.err/printStackTrace from timeout tests.

HIVE-28265: Reformat @rule annotations in TestJdbcDriver2

8072ae7

HIVE-28265: Re-trigger CI

0a1f5f2

ashniku commented Jul 29, 2026

View reviewed changes

Uh oh!

Conversation

ashniku commented Apr 6, 2026

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Jul 28, 2026

Quality Gate passed

Uh oh!

ashniku left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants