You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "jackyhu-db (via GitHub)" <gi...@apache.org> on 2025/07/16 17:55:31 UTC
[PR] feat(csharp/test/Drivers/Databricks): Enable RunAsync option in TExecuteStatementReq [arrow-adbc]
jackyhu-db opened a new pull request, #3171:
URL: https://github.com/apache/arrow-adbc/pull/3171
## Motivation
Databricks adds `RunAsync` (default is false) option for all the thrift operations, when it is set as `true`, the operation runs async at the backend and the status can be polled by calling getStatus, it helps Databricks backend to better manage capacity and load on client requests. Furthermore, it can avoid the unnecessary retry on thrift operation (e.g. TExecuteStatementReq) when the warehouse is stopped or unavailable, the backend will returns 503 error when `RunAsync` is false and client has to retry on this thrift operation till the warehouse is up, this generates lots of queries with 503 errors in the Databricks query history and consume more resources. When `RunAsync=true`, server will return 200 with query state 'PENDING`, the client will poll the status till the warehouse is up, this only generates one query in the query history.
## Change
- Add a connection parameter `adbc.databricks.enable_run_async_thrift`, default is `false` (it will be changed to `true` later)
- Set `RunAsync` of `TExecuteStatementReq` with above connection parameter (`RunAsyncInThrift`) in `DatabricksStatement:SetStatementProperties`
- Fix a bug in `BaseDatabricksReader` by adding `null` check on `statement.DirectResults.ResultSet`
## Test
- Run all the E2E tests under `csharp/test/Drivers/Databricks/E2E` when `Connection.RunAsyncInThrift` is both on and off
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/test/Drivers/Databricks): Enable RunAsync option in TExecuteStatementReq [arrow-adbc]
Posted by "birschick-bq (via GitHub)" <gi...@apache.org>.
birschick-bq commented on code in PR #3171:
URL: https://github.com/apache/arrow-adbc/pull/3171#discussion_r2211730549
##########
csharp/src/Drivers/Databricks/BaseDatabricksReader.cs:
##########
@@ -42,7 +42,7 @@ protected BaseDatabricksReader(DatabricksStatement statement, Schema schema, boo
this.schema = schema;
this.isLz4Compressed = isLz4Compressed;
this.statement = statement;
- if (statement.DirectResults != null && !statement.DirectResults.ResultSet.HasMoreRows)
+ if (statement.DirectResults?.ResultSet?.HasMoreRows == true)
Review Comment:
@jackyhu-db Sorry. My mistake!
```suggestion
if (statement.DirectResults?.ResultSet != null && !statement.DirectResults.ResultSet.HasMoreRows)
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/test/Drivers/Databricks): Enable RunAsync option in TExecuteStatementReq [arrow-adbc]
Posted by "birschick-bq (via GitHub)" <gi...@apache.org>.
birschick-bq commented on code in PR #3171:
URL: https://github.com/apache/arrow-adbc/pull/3171#discussion_r2211737529
##########
csharp/src/Drivers/Databricks/BaseDatabricksReader.cs:
##########
@@ -42,7 +42,7 @@ protected BaseDatabricksReader(DatabricksStatement statement, Schema schema, boo
this.schema = schema;
this.isLz4Compressed = isLz4Compressed;
this.statement = statement;
- if (statement.DirectResults != null && !statement.DirectResults.ResultSet.HasMoreRows)
+ if (statement.DirectResults?.ResultSet? != null && !statement.DirectResults.ResultSet.HasMoreRows)
Review Comment:
```suggestion
if (statement.DirectResults?.ResultSet != null && !statement.DirectResults.ResultSet.HasMoreRows)
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/test/Drivers/Databricks): Enable RunAsync option in TExecuteStatementReq [arrow-adbc]
Posted by "CurtHagenlocher (via GitHub)" <gi...@apache.org>.
CurtHagenlocher merged PR #3171:
URL: https://github.com/apache/arrow-adbc/pull/3171
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/test/Drivers/Databricks): Enable RunAsync option in TExecuteStatementReq [arrow-adbc]
Posted by "birschick-bq (via GitHub)" <gi...@apache.org>.
birschick-bq commented on code in PR #3171:
URL: https://github.com/apache/arrow-adbc/pull/3171#discussion_r2211633003
##########
csharp/src/Drivers/Databricks/BaseDatabricksReader.cs:
##########
@@ -42,7 +42,7 @@ protected BaseDatabricksReader(DatabricksStatement statement, Schema schema, boo
this.schema = schema;
this.isLz4Compressed = isLz4Compressed;
this.statement = statement;
- if (statement.DirectResults != null && !statement.DirectResults.ResultSet.HasMoreRows)
+ if (statement.DirectResults != null && statement.DirectResults.ResultSet != null && !statement.DirectResults.ResultSet.HasMoreRows)
Review Comment:
More concise?
```suggestion
if (statement.DirectResults?.ResultSet?.HasMoreRows == true)
```
##########
csharp/src/Drivers/Databricks/DatabricksStatement.cs:
##########
@@ -73,6 +76,8 @@ protected override void SetStatementProperties(TExecuteStatementReq statement)
statement.CanDecompressLZ4Result = canDecompressLz4;
statement.MaxBytesPerFile = maxBytesPerFile;
Review Comment:
nit: extra line
```suggestion
```
##########
csharp/src/Drivers/Databricks/DatabricksParameters.cs:
##########
@@ -180,6 +180,13 @@ public class DatabricksParameters : SparkParameters
/// </summary>
public const string UseDescTableExtended = "adbc.databricks.use_desc_table_extended";
+
Review Comment:
nit: extra line
```suggestion
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/test/Drivers/Databricks): Enable RunAsync option in TExecuteStatementReq [arrow-adbc]
Posted by "jackyhu-db (via GitHub)" <gi...@apache.org>.
jackyhu-db commented on PR #3171:
URL: https://github.com/apache/arrow-adbc/pull/3171#issuecomment-3079737503
@CurtHagenlocher can you add @jadewang-db as the reviewer before merging this.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/test/Drivers/Databricks): Enable RunAsync option in TExecuteStatementReq [arrow-adbc]
Posted by "birschick-bq (via GitHub)" <gi...@apache.org>.
birschick-bq commented on code in PR #3171:
URL: https://github.com/apache/arrow-adbc/pull/3171#discussion_r2211730549
##########
csharp/src/Drivers/Databricks/BaseDatabricksReader.cs:
##########
@@ -42,7 +42,7 @@ protected BaseDatabricksReader(DatabricksStatement statement, Schema schema, boo
this.schema = schema;
this.isLz4Compressed = isLz4Compressed;
this.statement = statement;
- if (statement.DirectResults != null && !statement.DirectResults.ResultSet.HasMoreRows)
+ if (statement.DirectResults?.ResultSet?.HasMoreRows == true)
Review Comment:
@jackyhu-db Sorry. My mistake!
```suggestion
if (statement.DirectResults?.ResultSet? != null && !statement.DirectResults.ResultSet.HasMoreRows)
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/test/Drivers/Databricks): Enable RunAsync option in TExecuteStatementReq [arrow-adbc]
Posted by "CurtHagenlocher (via GitHub)" <gi...@apache.org>.
CurtHagenlocher commented on code in PR #3171:
URL: https://github.com/apache/arrow-adbc/pull/3171#discussion_r2211471895
##########
csharp/src/Drivers/Databricks/DatabricksConnection.cs:
##########
@@ -164,6 +165,19 @@ private void ValidateProperties()
}
}
+ if (Properties.TryGetValue(DatabricksParameters.EnableRunAsyncInThriftOp, out string? enableRunAsyncInThriftStr))
+ {
+ if (bool.TryParse(enableRunAsyncInThriftStr, out bool enableRunAsyncInThrift))
+ {
+ _runAsyncInThrift = enableRunAsyncInThrift;
+ }
+ else
+ {
+ throw new ArgumentException($"Parameter '{DatabricksParameters.EnableRunAsyncInThriftOp}' value '{enableRunAsyncInThriftStr}' could not be parsed. Valid values are 'true' and 'false'.");
+ }
+ }
+
+
Review Comment:
nit: extra blank line
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/test/Drivers/Databricks): Enable RunAsync option in TExecuteStatementReq [arrow-adbc]
Posted by "CurtHagenlocher (via GitHub)" <gi...@apache.org>.
CurtHagenlocher commented on PR #3171:
URL: https://github.com/apache/arrow-adbc/pull/3171#issuecomment-3080197473
> @CurtHagenlocher can you add @jadewang-db as the reviewer before merging this.
Only official Arrow committers can be added as reviewers, I'm afraid, but I can certainly wait until @jadewang-db signs off.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org