-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve handling of interrupts in TrinoResultSet #14187
Conversation
|
Differentiate between interrupts happening in the calling thread and interrupts of the background thread. - use different message - the interrupts happening in the calling thread should result in full cancellation of the background thread. Previously they would close client, but wouldn't set `cancelled` flag.
19ba980
to
3e01a3e
Compare
@xiacongling please take a look |
@@ -183,7 +183,9 @@ public AsyncIterator(Iterator<T> dataIterator, StatementClient client) | |||
} | |||
} | |||
catch (InterruptedException e) { | |||
handleInterrupt(e); | |||
client.close(); | |||
rowQueue.clear(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why does cleaning rowQueue here matter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here it is probably not important. it's more like "why not?"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
discussed offline and agreed to keep
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
LGTM. It is much clearer than before. |
Differentiate between interrupts happening in the calling thread and
interrupts of the background thread.
cancellation of the background thread. Previously they would close
client, but wouldn't set
cancelled
flag.