You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "ptupitsyn (via GitHub)" <gi...@apache.org> on 2025/12/24 16:35:57 UTC
[PR] IGNITE-25836 Suppress client handler logs for cancellation errors [ignite-3]
ptupitsyn opened a new pull request, #7306:
URL: https://github.com/apache/ignite-3/pull/7306
Cancellation is triggered by the user and should not produce errors in the log.
https://issues.apache.org/jira/browse/IGNITE-25836
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25836 Suppress client handler logs for cancellation errors [ignite-3]
Posted by "ptupitsyn (via GitHub)" <gi...@apache.org>.
ptupitsyn merged PR #7306:
URL: https://github.com/apache/ignite-3/pull/7306
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25836 Suppress client handler logs for cancellation errors [ignite-3]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #7306:
URL: https://github.com/apache/ignite-3/pull/7306#discussion_r2646031517
##########
modules/client-handler/src/main/java/org/apache/ignite/client/handler/ClientInboundMessageHandler.java:
##########
@@ -738,6 +741,26 @@ private void writeErrorCore(Throwable err, ClientMessagePacker packer) {
}
}
+ private static boolean shouldLogError(Throwable e) {
+ Throwable cause = ExceptionUtils.unwrapRootCause(e);
+
+ // Do not log errors caused by query cancellation (triggered by user action).
Review Comment:
The comment "Do not log errors caused by query cancellation (triggered by user action)" is incomplete as it only mentions query cancellation, but the method also suppresses logging for compute job cancellation errors (COMPUTE_JOB_CANCELLED_ERR and CANCELLING_ERR). Consider updating the comment to reflect all the cancellation types being suppressed, for example: "Do not log errors caused by user-triggered cancellations (query or compute job)."
```suggestion
// Do not log errors caused by user-triggered cancellations (query or compute job).
```
##########
modules/client-handler/src/main/java/org/apache/ignite/client/handler/ClientInboundMessageHandler.java:
##########
@@ -738,6 +741,26 @@ private void writeErrorCore(Throwable err, ClientMessagePacker packer) {
}
}
+ private static boolean shouldLogError(Throwable e) {
+ Throwable cause = ExceptionUtils.unwrapRootCause(e);
+
+ // Do not log errors caused by query cancellation (triggered by user action).
+ if (cause instanceof QueryCancelledException) {
+ return false;
+ }
Review Comment:
The check for `QueryCancelledException` is redundant. Since `QueryCancelledException` extends `IgniteException` (which implements `TraceableException`) and has error code `EXECUTION_CANCELLED_ERR`, it will already be handled by the `TraceableException` check at lines 752-759 where the code checks for `Sql.EXECUTION_CANCELLED_ERR`. Consider removing this redundant check to simplify the logic.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org