korlov42 commented on code in PR #2965:
URL: https://github.com/apache/ignite-3/pull/2965#discussion_r1433841576
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/AsyncSqlCursorImpl.java:
##########
@@ -158,17 +131,6 @@ public CompletableFuture<Void> closeAsync() {
dataCursor.closeAsync()
.thenCompose(ignored -> txWrapper.commitImplicit())
.whenComplete((r, e) -> {
Review Comment:
> Should we complete it in a separate thread?
I would say "no". Query processor is an internal API and doesn't expose any
results directly to the user. Depending on a particular public API
implementation, an internal results follow by different paths:
- in case of thin clients, the results are serialised and send over the
network. To me, it's perfectly fine to use SQL thread pool for that
- in case of embedded sync API, user will wait till the future completion,
and then will proceed on its own thread
- in case of embedded async API there is a chance that user's workload will
be done on SQL thread pool, but this is a problem of a particular API and
should be addressed accordingly (by introducing continuation executor, for
instance, as it done in thin clients; BTW, not only sql is affected, by
KV/Record views as well)
in other word, the problem exists, but it's subject to separate JIRA
--
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]
