For broken queries EventListenerMangager.queryCompleted() is called too early

[dominikzalewski]

Summary

The majority of the queries are processed in such a way that we first call QueryTracker.addQuery() and only later EventListenerManager.queryCompleted(). For badly broken failed queries, for instance ones that have invalid SQL syntax, the order of the calls is inverted. As a result, EventListeners that want to access query details, fail in this edge case.

Current Workaround

EventListener is coded using a separate thread that waits (usually 100 milliseconds) so that the call to QueryTracker.addQuery() has a chance of completing in the main thread before EventListener.queryCompleted() is finished.

This mechanism does not always give good results, especially on overloaded systems where 100 milliseconds is not enough wait time.

Code details

DispatchManager.createQueryInternal():

• failedDispatchQueryFactory.createFailedDispatchQuery() calls EventListenerManager.queryCreated()
• queryCreated() calls QueryTracker.addQuery()

[hashhar]

cc: @dain @sopel39

An example would be an event listener that tries to collect query JSONs for all queries - currently it can only do so reliably for non-failed queries. For failed queries the query JSON isn't available timely and if the listener waits then it's available.

What can be done to make sure that the events sent to listeners don't depend on when the event listener executes? There is a comment about changing the order might impact event listeners registered via SystemAccessControl. Would it make sense to decouple engine provided functionality (audit logging SAC) vs user provided code (e.g. EventListener)?

What alternative options are there?

[dominikzalewski]

See this PR: https://github.com/trinodb/trino/pull/20231