eric-wang-1990 commented on code in PR #3302:
URL: https://github.com/apache/arrow-adbc/pull/3302#discussion_r2283561022
##########
csharp/src/Drivers/Apache/Hive2/HiveServer2Statement.cs:
##########
@@ -1006,5 +1038,97 @@ public bool TryGetDirectResults(IResponse response, out
TSparkDirectResults? dir
directResults = null;
return false;
}
+
+ /// <inheritdoc/>
+ public override void Cancel()
+ {
+ this.TraceActivity(activity =>
+ {
+ using CancellationTokenSource cancellationTokenSource =
ApacheUtility.GetCancellationTokenSource(QueryTimeoutSeconds,
ApacheUtility.TimeUnit.Seconds);
+ try
+ {
+ // This will cancel any operation using the current token
source
+ CancelTokenSource();
+
+ // Clone the operation handle so it doesn't get changed
while we make our call
Review Comment:
Why do we need to clone the operation handle? IMO we can only cancel the
most recent statement, and that is stored in the current
_executeOperationHandle variable.
If no new statement gets executed, we will keep using that old one.
If there is a new statement get executed, that means statement complete and
that variable will get overridden anyways.
##########
csharp/src/Drivers/Apache/Hive2/HiveServer2Statement.cs:
##########
@@ -1006,5 +1038,97 @@ public bool TryGetDirectResults(IResponse response, out
TSparkDirectResults? dir
directResults = null;
return false;
}
+
+ /// <inheritdoc/>
+ public override void Cancel()
+ {
+ this.TraceActivity(activity =>
+ {
+ using CancellationTokenSource cancellationTokenSource =
ApacheUtility.GetCancellationTokenSource(QueryTimeoutSeconds,
ApacheUtility.TimeUnit.Seconds);
+ try
+ {
+ // This will cancel any operation using the current token
source
+ CancelTokenSource();
+
+ // Clone the operation handle so it doesn't get changed
while we make our call
+ TOperationHandle? operationHandle = CloneOperationHandle();
+ if (operationHandle != null)
+ {
+ TCancelOperationReq req = new
TCancelOperationReq(operationHandle);
+ TCancelOperationResp resp =
Client.CancelOperation(req, cancellationTokenSource.Token)
+ .ConfigureAwait(false).GetAwaiter().GetResult();
+
HiveServer2Connection.HandleThriftResponse(resp.Status, activity);
+ }
+ }
+ catch (Exception ex)
+ when (ApacheUtility.ContainsException(ex, out
OperationCanceledException? _) ||
+ (ApacheUtility.ContainsException(ex, out
TTransportException? _) && cancellationTokenSource.IsCancellationRequested))
+ {
+ throw new TimeoutException("The cancel operation timed
out. Consider increasing the query timeout value.", ex);
Review Comment:
Does the exception get surfaced to PBI? Ideally cancel should be best effort
and we should not block user flow if exceptions are thrown.
--
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-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
