jadewang-db commented on code in PR #3217:
URL: https://github.com/apache/arrow-adbc/pull/3217#discussion_r2248524315
##########
csharp/src/Drivers/Databricks/CloudFetch/CloudFetchResultFetcher.cs:
##########
@@ -257,7 +258,7 @@ private async Task FetchResultsAsync(CancellationToken
cancellationToken)
// Add the end of results guard to the queue even in case of
error
try
{
- _downloadQueue.Add(EndOfResultsGuard.Instance,
CancellationToken.None);
+ _downloadQueue.TryAdd(EndOfResultsGuard.Instance, 0);
Review Comment:
why don't use cancellation token here?
##########
csharp/src/Drivers/Databricks/DatabricksOperationStatusPoller.cs:
##########
@@ -69,13 +72,18 @@ private async Task PollOperationStatus(CancellationToken
cancellationToken)
var operationHandle = _statement.OperationHandle;
if (operationHandle == null) break;
Review Comment:
are you able to repro this? should we do this to all the thrift rpc calls in
the driver?
##########
csharp/src/Drivers/Databricks/DatabricksCompositeReader.cs:
##########
@@ -108,5 +111,41 @@ private BaseDatabricksReader
DetermineReader(TFetchResultsResp initialResults)
return await
_activeReader.ReadNextRecordBatchAsync(cancellationToken);
}
+
+ public override async ValueTask<RecordBatch?>
ReadNextRecordBatchAsync(CancellationToken cancellationToken = default)
+ {
+ var result = await
ReadNextRecordBatchInternalAsync(cancellationToken);
+ // Stop the poller when we've reached the end of results
+ if (result == null)
+ {
+ StopOperationStatusPoller();
+ }
+ return result;
+ }
+
+ protected override void Dispose(bool disposing)
+ {
+ if (disposing)
+ {
+ _activeReader?.Dispose();
Review Comment:
set to null after dispose?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
