Skip to content

Commit 58b95bb

Browse files
authored
Add MetadataOperationTimeout support for SHOW commands metadata path (#1417)
## Summary `MetadataOperationTimeout` (default 300s) only worked for native Thrift RPCs. With `UseQueryForMetadata=1` (new default for warehouses), metadata operations had **no timeout** — they could hang indefinitely because `parentStatement=null` results in `timeoutInSeconds=0`. ## Root Cause Both `DatabricksSdkClient.executeStatement()` and `DatabricksThriftAccessor.pollTillOperationFinished()` compute timeout as: ```java int timeoutInSeconds = (parentStatement == null) ? 0 : parentStatement.getStatement().getQueryTimeout(); ``` Metadata calls pass `parentStatement=null`, so timeout was always 0 (infinite). ## Fix When `parentStatement==null` AND `statementType==METADATA`, read `MetadataOperationTimeout` from `connectionContext` instead of defaulting to 0. Uses `OPERATION_TIMEOUT_ERROR` code for metadata timeouts (matching native Thrift path). ## Files Changed - `DatabricksSdkClient.java` — SEA metadata path timeout - `DatabricksThriftAccessor.java` — Thrift SHOW commands path timeout (added `statementType` param to `pollTillOperationFinished`) - `DatabricksSdkClientTest.java` — 2 new tests ## Test plan - [x] `testMetadataOperationUsesMetadataTimeout` — verifies MetadataOperationTimeout=1s triggers for metadata with parentStatement=null - [x] `testNonMetadataWithNullParentHasNoTimeout` — verifies non-metadata with parentStatement=null still has no timeout - [x] Full core suite: 3297 tests, 0 failures This pull request was AI-assisted by Isaac. --------- Signed-off-by: Gopal Lal <gopal.lal@databricks.com>
1 parent 6b708f0 commit 58b95bb

5 files changed

Lines changed: 135 additions & 12 deletions

File tree

NEXT_CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
- Arrow schema deserialization failures (Thrift metadata path) now surface a dedicated driver error code `ARROW_SCHEMA_PARSING_ERROR` (vendor code `22000`) and a proper SQLSTATE `22000` (Data Exception) on the thrown `SQLException`, instead of the generic `RESULT_SET_ERROR` (1004) and the enum name as SQLSTATE. The exception message is unchanged.
1818

1919
### Fixed
20+
- Fixed `MetadataOperationTimeout` not being applied when metadata operations use SHOW commands. Operations like `getTables`, `getSchemas`, and `getColumns` now respect the `MetadataOperationTimeout` connection property instead of hanging indefinitely with no timeout.
2021

2122
- Reclassify transient/mis-categorized server errors so callers can identify
2223
retryable failures. The remap is applied at all Thrift error sites

src/main/java/com/databricks/jdbc/common/DatabricksJdbcConstants.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ public final class DatabricksJdbcConstants {
121121
* concurrent-modification errors where the operation is potentially retryable.
122122
*/
123123
public static final String SERIALIZATION_FAILURE_SQLSTATE = "40001";
124+
124125
/** Standard SQL state for data exception (SQLSTATE 22000). */
125126
public static final String DATA_EXCEPTION_SQLSTATE = "22000";
126127

src/main/java/com/databricks/jdbc/dbclient/impl/sqlexec/DatabricksSdkClient.java

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -248,16 +248,23 @@ public DatabricksResultSet executeStatement(
248248
parentStatement.setStatementId(typedStatementId);
249249
}
250250

251-
int timeoutInSeconds =
252-
parentStatement != null ? parentStatement.getStatement().getQueryTimeout() : 0;
251+
int timeoutInSeconds;
252+
if (parentStatement != null) {
253+
timeoutInSeconds = parentStatement.getStatement().getQueryTimeout();
254+
} else if (statementType == StatementType.METADATA) {
255+
timeoutInSeconds = connectionContext.getMetadataOperationTimeout();
256+
} else {
257+
timeoutInSeconds = 0;
258+
}
253259

254-
// Create timeout handler
260+
// Create timeout handler — use OPERATION_TIMEOUT_ERROR for metadata to match
261+
// the native Thrift metadata path (DatabricksThriftAccessor.fetchMetadataResults)
262+
DatabricksDriverErrorCode timeoutErrorCode =
263+
(statementType == StatementType.METADATA && parentStatement == null)
264+
? DatabricksDriverErrorCode.OPERATION_TIMEOUT_ERROR
265+
: DatabricksDriverErrorCode.STATEMENT_EXECUTION_TIMEOUT;
255266
TimeoutHandler timeoutHandler =
256-
TimeoutHandler.forStatement(
257-
timeoutInSeconds,
258-
typedStatementId,
259-
this,
260-
DatabricksDriverErrorCode.STATEMENT_EXECUTION_TIMEOUT);
267+
TimeoutHandler.forStatement(timeoutInSeconds, typedStatementId, this, timeoutErrorCode);
261268

262269
StatementState responseState = response.getStatus().getState();
263270
while (responseState == StatementState.PENDING || responseState == StatementState.RUNNING) {

src/main/java/com/databricks/jdbc/dbclient/impl/thrift/DatabricksThriftAccessor.java

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ DatabricksResultSet execute(
237237

238238
TGetOperationStatusResp statusResp =
239239
pollTillOperationFinished(
240-
response, parentStatement, session, statementId, sessionDebugInfo);
240+
response, parentStatement, session, statementId, sessionDebugInfo, statementType);
241241
boolean isDirectResults = hasResultDataInDirectResults(response);
242242
if (isDirectResults) {
243243
// The first response has result data
@@ -303,10 +303,17 @@ private TGetOperationStatusResp pollTillOperationFinished(
303303
IDatabricksStatementInternal parentStatement,
304304
IDatabricksSession session,
305305
StatementId statementId,
306-
String sessionDebugInfo)
306+
String sessionDebugInfo,
307+
StatementType statementType)
307308
throws SQLException, TException {
308-
int timeoutInSeconds =
309-
(parentStatement == null) ? 0 : parentStatement.getStatement().getQueryTimeout();
309+
int timeoutInSeconds;
310+
if (parentStatement != null) {
311+
timeoutInSeconds = parentStatement.getStatement().getQueryTimeout();
312+
} else if (statementType == StatementType.METADATA) {
313+
timeoutInSeconds = connectionContext.getMetadataOperationTimeout();
314+
} else {
315+
timeoutInSeconds = 0;
316+
}
310317

311318
TGetOperationStatusResp statusResp = null;
312319
if (response.isSetDirectResults()) {

src/test/java/com/databricks/jdbc/dbclient/impl/sqlexec/DatabricksSdkClientTest.java

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -602,6 +602,113 @@ public void testExecuteStatementWithTimeoutExpired() throws Exception {
602602
verify(databricksSdkClient).cancelStatement(eq(STATEMENT_ID));
603603
}
604604

605+
@Test
606+
public void testMetadataOperationUsesMetadataTimeout() throws Exception {
607+
// MetadataOperationTimeout=1 with parentStatement=null (metadata path)
608+
IDatabricksConnectionContext connectionContext =
609+
DatabricksConnectionContext.parse(
610+
JDBC_URL,
611+
new Properties() {
612+
{
613+
setProperty("MetadataOperationTimeout", "1");
614+
setProperty("asyncExecPollInterval", "1000");
615+
}
616+
});
617+
DatabricksSdkClient databricksSdkClient =
618+
spy(new DatabricksSdkClient(connectionContext, statementExecutionService, apiClient));
619+
DatabricksConnection connection =
620+
new DatabricksConnection(connectionContext, databricksSdkClient);
621+
622+
CreateSessionResponse sessionResponse = new CreateSessionResponse().setSessionId(SESSION_ID);
623+
when(apiClient.execute(any(Request.class), eq(CreateSessionResponse.class)))
624+
.thenReturn(sessionResponse);
625+
connection.open();
626+
627+
ExecuteStatementResponse executeResponse =
628+
new ExecuteStatementResponse()
629+
.setStatementId(STATEMENT_ID.toSQLExecStatementId())
630+
.setStatus(new StatementStatus().setState(StatementState.RUNNING));
631+
GetStatementResponse runningResponse =
632+
new GetStatementResponse()
633+
.setStatus(new StatementStatus().setState(StatementState.RUNNING));
634+
635+
when(apiClient.execute(
636+
argThat(req -> req != null && STATEMENT_PATH.equals(req.getUrl())),
637+
eq(ExecuteStatementResponse.class)))
638+
.thenReturn(executeResponse);
639+
when(apiClient.execute(
640+
argThat(
641+
req ->
642+
req != null
643+
&& req.getUrl() != null
644+
&& req.getUrl().contains(STATEMENT_ID.toSQLExecStatementId())),
645+
eq(GetStatementResponse.class)))
646+
.thenReturn(runningResponse);
647+
648+
// Metadata with parentStatement=null should use MetadataOperationTimeout (1s)
649+
DatabricksTimeoutException exception =
650+
assertThrows(
651+
DatabricksTimeoutException.class,
652+
() ->
653+
databricksSdkClient.executeStatement(
654+
"SHOW SCHEMAS IN ALL CATALOGS",
655+
warehouse,
656+
new java.util.HashMap<>(),
657+
StatementType.METADATA,
658+
connection.getSession(),
659+
null, // parentStatement=null (metadata path)
660+
null));
661+
662+
assertTrue(exception.getMessage().contains("timed-out after 1 seconds"));
663+
verify(databricksSdkClient).cancelStatement(eq(STATEMENT_ID));
664+
}
665+
666+
@Test
667+
public void testNonMetadataWithNullParentHasNoTimeout() throws Exception {
668+
// Non-metadata with parentStatement=null should have timeout=0 (infinite)
669+
// Use a short poll interval so the test completes quickly
670+
IDatabricksConnectionContext connectionContext =
671+
DatabricksConnectionContext.parse(
672+
JDBC_URL,
673+
new Properties() {
674+
{
675+
setProperty("MetadataOperationTimeout", "1");
676+
}
677+
});
678+
DatabricksSdkClient databricksSdkClient =
679+
new DatabricksSdkClient(connectionContext, statementExecutionService, apiClient);
680+
DatabricksConnection connection =
681+
new DatabricksConnection(connectionContext, databricksSdkClient);
682+
683+
CreateSessionResponse sessionResponse = new CreateSessionResponse().setSessionId(SESSION_ID);
684+
when(apiClient.execute(any(Request.class), eq(CreateSessionResponse.class)))
685+
.thenReturn(sessionResponse);
686+
connection.open();
687+
688+
// Return SUCCEEDED immediately so the test completes
689+
ExecuteStatementResponse executeResponse =
690+
new ExecuteStatementResponse()
691+
.setStatementId(STATEMENT_ID.toSQLExecStatementId())
692+
.setStatus(new StatementStatus().setState(StatementState.SUCCEEDED));
693+
694+
when(apiClient.execute(
695+
argThat(req -> req != null && STATEMENT_PATH.equals(req.getUrl())),
696+
eq(ExecuteStatementResponse.class)))
697+
.thenReturn(executeResponse);
698+
699+
// Non-METADATA with parentStatement=null: no timeout applied, should succeed
700+
assertDoesNotThrow(
701+
() ->
702+
databricksSdkClient.executeStatement(
703+
"SELECT 1",
704+
warehouse,
705+
new java.util.HashMap<>(),
706+
StatementType.SQL,
707+
connection.getSession(),
708+
null,
709+
null));
710+
}
711+
605712
@Test
606713
public void testServerSideTimeoutThrowsTimeoutException() throws Exception {
607714

0 commit comments

Comments
 (0)