Skip to content
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

Blocking API over async API #415

Merged
merged 19 commits into from
Oct 10, 2017
Merged

Conversation

lutovich
Copy link
Contributor

PR makes all blocking APIs use async version underneath and block to get the result. Most of old blocking API infrastructure is removed. Added many TODOs mainly about missing tests but also some about shortcomings in the code. They all be fixed in subsequent PRs.

Zhen and others added 17 commits October 5, 2017 13:49
Made blocking methods always use async ones. Extracted convenience
methods on `QueryRunner` to execute queries in async way with and
without waiting for RUN response.
So that it's status is updated.
So that blocking API result reuses code of `StatementResultCursor`
and simply adds blocking to it.
Async APIs are used all the way to the top public layer where blocking occurs.
Last handler in the inbound pipeline is InboundMessageHandler. It
catches all unhandled exceptions and closes the network channel.
It also handles channel inactive events. This commit makes handler
ignore inactive event after exception was caught. Otherwise original
unhandled exception was overwritten by `ServiceUnavailableException`.
When using `CompletionStage#whenComplete()` and `CompletionStage#handle()`.
These methods get error wrapped in `CompletionException`. This commit
adds a utility method to extract the cause and makes all call sites use it.
To be used in `GraphDatabase` class and guarantee that created driver
is able to connect to single instance or fetch routing table from
a CC cluster.
This commit makes `NetworkSession` verify existence of a transaction
before running query or starting a new transaction. It will now chain
connection and transaction completion stages correctly. Previously it
tried to check connection and transaction for null, which was unsafe
because it was not properly chained with subsequent async operations.

`ExplicitTransaction` now tracks it's state more thoroughly and does
not allow running queries after commit & rollback.

Also fixed couple tests after sync over async.
Made `Session#run()` not wait for RUN response, so it conforms to
previous blocking behaviour.
`ChunkAwareByteBufOutput` encodes whole message into a byte buffer.
For each chunk it first reserves two bytes for headers, then writes
chunk body and then comes back to the beginning to white actual size.
Initially first reserved two bytes contain zeroes.

Previously output tried to "skip" two first header bytes by advancing
current writer index by two. This worked for most cases but failed
when current writer index was at buffers capacity. Advancing writer
index beyond the buffer capacity does not extend the buffer but
causes an exception.

This commit fixes the problem by making output actually write two zero
bytes, which expands buffer capacity, when necessary.
Following changes were made:
 * turned off channel health check on release so that pool behaves like
   in previous version of the driver
 * unwrap `DecoderException` in `HandshakeResponseHandler` and propagate
   it's cause, wrapping in `SecurityException` when necessary; this is
   needed to throw pretty errors when TLS handshake fails because of
   wrong certificates, etc.
 * `InternalStatementResultCursor` now throws with different messages
   based on the mode it's use in; this is needed to throw correct errors
   when it's used by blocking `StatementResult`
 * introduced a way for session to track latest result and await for it
   being finished (either fully received of failed); this is needed to
   guarantee that `Session#close()` does not RESET previous
   unconsumed queries

The only ignored tests right now are for `Session#reset()`. They will
be fixed subsequent commits.

Also many TODOs were added. They will also be addressed later.
Method `#messageBoundaryHook()` was only useful for blocking
connections is is unused for netty-based async IO.
Commit removes `Async` prefix from couple class names and moves some
interfaces and classes to more suitable packages.
@zhenlineo zhenlineo merged commit c6251ea into neo4j:1.5 Oct 10, 2017
@zhenlineo zhenlineo deleted the 1.5-sync-over-async branch October 10, 2017 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants