Add failure event for queries that were posted but not submitted #24205
Conversation
| queryMonitor.queryImmediateFailureEvent( | ||
| getBasicQueryInfoForFailure(USER_CANCELED.toErrorCode()), | ||
| toFailure(new TrinoException(USER_CANCELED, "Query was canceled"))); |
There was a problem hiding this comment.
I do not get the flow for cancelation.
Why do we need event code both here and in cancelInternal?
| @@ -177,6 +177,55 @@ public void queryCreatedEvent(BasicQueryInfo queryInfo) | |||
| Optional::empty))); | |||
| } | |||
|
|
|||
| private static QueryStatistics createEmptyStatistics(Duration queuedTime) | |||
There was a problem hiding this comment.
unrelated change. Fine on its own but move to separate commit
|
|
||
| private void cancelInternal() | ||
| { | ||
| if (!dispatchManager.cancelQuery(queryId)) { |
There was a problem hiding this comment.
why can't we have event logic in dispatchManager.cancelQuery?
Is that lack of session or the fact that is used from QueryResource too? Or ... ?
|
Looks generally good. @wendigo PTAL too. |
|
IMO @electrum should also take a look because from my discussion with him in the past this was intentional part of protocol. |
There was a problem hiding this comment.
This seems like a bad change, as it will cause bogus events for query resubmission. The query doesn't exist until the client fetches the first next URI.
What problem are you trying to solve? If this is for administrators to debug broken clients, you could add a debug log line for abandoned queries.
|
Closed due to conflicts with the Trino protocol |
Description
Additional context and related issues
Release notes
( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text: