Handle thread interruption by closing the channel #441
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ali-ince
approved these changes
Dec 7, 2017
|
Added a commit with some more tests. @ali-ince could you please take a look? |
ali-ince
reviewed
Dec 13, 2017
| } | ||
| catch ( Exception ignore ) | ||
| { | ||
| // expected |
There was a problem hiding this comment.
@lutovich Would it be better to expect a ServiceUnavailableException here instead of a IllegalStateException?
There was a problem hiding this comment.
Yeah, that's true. What do you think about always wrapping exceptions from connectivity verification in ServiceUnavailableException? (unless it's already a ServiceUnavailableException)
|
@ali-ince exceptions should now be fixed |
lutovich
force-pushed
the
1.5-thread-interrupt
branch
2 times, most recently
from
ae0ac74
to
7b0c9ef
Compare
Blocking API like `Session#run()` is now implemented on top of async API, i.e. `Session#runAsync()`. Blocking calls simply invoke async ones and uninterruptibly wait for them to complete. Previously, with blocking IO, thread interruptions resulted in `ClosedByInterruptException` which terminated query/transaction in a harsh way. Right now interrupts will do nothing. Handling them is not strictly required because `Thread#interrupt()` is not a nice thing to do. However, interrupts are still used in some environments as a last resort thing. This commit makes driver react on thread interrupts by closing the underlying Netty channel. It's only done in places where channel is actually available. Closing of channel results in exception and the waiting thread fails. It will also make database reset state for this connection, terminate transaction and query. Such aproach seems very similar to the previous one except it will result in `ServiceUnavailableException` instead of `ClosedByInterruptException`.
Added couple more integrations tests to verify that `Thread#interrupt()` terminates active connection and "unblocks" the blocking thread: * when driver is created towards unresponsive server * when transaction blocks to read back the result * when transaction blocks to commit Also `NettyConnection` will now fail provided handlers except throwing exception directly. Increased test coverage for future helpers.
Driver verifies connectivity on creation. It closes the connection pool when thread is interrupted. Previously original `IllegalStateException` from the closed pool has been propagated to the user. This commit makes connectivity verification always throw `ServiceUnavailableException`.
lutovich
force-pushed
the
1.5-thread-interrupt
branch
from
7b0c9ef
to
cc8fc7e
Compare
Also added unit tests for `Futures#asCompletionException()`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Blocking API like
Session#run()is now implemented on top of async API, i.e.Session#runAsync(). Blocking calls simply invoke async ones and uninterruptibly wait for them to complete. Previously, with blocking IO, thread interruptions resulted inClosedByInterruptExceptionwhich terminated query/transaction in a harsh way. Right now interrupts will do nothing. Handling them is not strictly required becauseThread#interrupt()is not a nice thing to do. However, interrupts are still used in some environments as a last resort thing.This commit makes driver react on thread interrupts by closing the underlying Netty channel. It's only done in places where channel is actually available. Closing of channel results in exception and the waiting thread fails. It will also make database reset state for this connection, terminate transaction and query.
Such approach seems very similar to the previous one except it will result in
ServiceUnavailableExceptioninstead ofClosedByInterruptException.