toddmeng-db commented on code in PR #3217:
URL: https://github.com/apache/arrow-adbc/pull/3217#discussion_r2241477161
##########
csharp/src/Drivers/Apache/Hive2/HiveServer2Statement.cs:
##########
@@ -394,6 +404,10 @@ public override void Dispose()
HiveServer2Connection.HandleThriftResponse(resp.Status,
activity);
OperationHandle = null;
}
+ // For DatabricksReader, must be called after CloseOperation,
otherwise it may cancel a
+ // request when the input buffer of Thrift client has not been
consumed/cleaned-up
+ _currentReader?.Dispose();
+ _currentReader = null;
Review Comment:
Encountered during testing:
When this is done before CloseOperation in this Dispose method,
in DatabricksOperationStatusPoller, the Thrift Client may have received a
full or partial GetOperationStatusResp in the input-buffer. However, if
cancellation occurs before this method (recv_GetOperationStatus) completes, it
would leave stranded response data in the input buffer.
Changing to the ordering above, it means that CloseOperation happens after
GetOperationStatus (which is guarded by a lock on the ThreadSafeClient) call
completing
##########
csharp/src/Drivers/Apache/Hive2/HiveServer2Statement.cs:
##########
@@ -394,6 +404,10 @@ public override void Dispose()
HiveServer2Connection.HandleThriftResponse(resp.Status,
activity);
OperationHandle = null;
}
+ // For DatabricksReader, must be called after CloseOperation,
otherwise it may cancel a
+ // request when the input buffer of Thrift client has not been
consumed/cleaned-up
+ _currentReader?.Dispose();
+ _currentReader = null;
Review Comment:
Encountered during testing:
When this is done before CloseOperation in this Dispose method,
in DatabricksOperationStatusPoller, the Thrift Client may have received a
full or partial GetOperationStatusResp in the input-buffer. However, if
cancellation occurs before this method (recv_GetOperationStatus) completes, it
would leave stranded response data in the input buffer.
Changing to the ordering above, it means that CloseOperation happens after
GetOperationStatus (which is guarded by a lock on the ThreadSafeClient) call
completes
--
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]
