Commit Graph

114 Commits

Author SHA1 Message Date
Louis Ryan d54911713a Implement writes to the channel using a dedicated write queue which allows for efficient flush
coalescing
2015-05-15 09:20:13 -07:00
nmittler 8f537e3ec6 updates to test native epoll 2015-05-14 15:33:55 -07:00
nmittler 5efbb6fe73 Allow 100 streams initially rather than 10.
The HTTP/2 spec suggests 100 as the recommended minimum for SETTINGS_MAX_CONCURRENT_STREAMS (https://tools.ietf.org/html/draft-ietf-httpbis-http2-17#section-6.5.2).  We should use this value as our default.
2015-05-13 14:22:46 -07:00
Eric Anderson 1787106cc7 isReady() should return false until stream allocated
isReady() can provide pushback while the call is in progress, but it
can also provide the pushback necessary when the client creates more
streams than permitted by MAX_CONCURRENT_STREAMS.

As part of this commit, OkHttp is now calling onReady() after call
creation (it previously never called onReady()).
2015-05-12 16:41:50 -07:00
nmittler c3fa600eae Fixing compiler warning. 2015-05-12 16:23:39 -07:00
nmittler d5727c7fcd Deferring stream creation until receiving SETTINGS from server.
Additionally:
- Fixed bug where the decoder was given the incorrect encoder.
- Adding proper logging class for client/server.
2015-05-12 15:46:32 -07:00
nmittler b6407c4a10 Only complete graceful shutdown after buffered streams complete.
Upgrading to to the latest Netty as well.
2015-05-07 08:36:22 -07:00
nmittler b5443833d9 Fixing build issue. 2015-05-07 07:50:33 -07:00
nmittler d8d8ebd783 Adding logging to NettyClientHandler. 2015-05-06 14:38:22 -07:00
nmittler 64df428e36 Properly removing buffered streams after receiving goAway 2015-05-06 14:24:29 -07:00
nmittler 7f73189e56 Always checking MAX_CONCURRENT_STREAMS in
BufferingHttp2ConnectionEncoder.

Currently we don't check this setting when handling a streamClosed()
event. If the setting has lowered prior to this event, the stream
creation could fail.
2015-05-05 19:29:34 -07:00
nmittler a45e0a4767 Cleaning up closing for Netty client/server. 2015-05-05 15:43:22 -07:00
nmittler ef6a2f02d4 Updating to the latest Netty version.
Also re-enabling the gracefulShutdown test now that Netty has been
fixed.
2015-05-05 15:39:21 -07:00
Eric Anderson 10fb20650d Produce combined JavaDoc, add links, exclude internals 2015-05-05 11:32:15 -07:00
Eric Anderson c8c478ef81 netty: Remove goAwayStatus when client closing
Client closing doesn't really many anything special, since it is still
fully-operational. We don't want a later goAwayStatus() from getting
squelched because we were shutting down.
2015-05-05 08:38:38 -07:00
nmittler 68cba971af Do not fail pending streams when sending GOAWAY.
If the HEADERS have been written to the buffering encoder we should
allow their creation to complete since the HTTP/2 spec does not prohibit
this.
2015-05-04 13:00:11 -07:00
nmittler 64176d5560 Adding outbound flow control for Netty. 2015-05-04 12:19:57 -07:00
Jakob Buchgraber 1af367c786 Remove all blocking from the NettyClientTransport. Fixes #116
Motivation:

We are currently blocking in NettyClientTransport.newStream(...) until the channel is active and/or the TLS Handshake is complete.
In certain cases this may lead to deadlock of the eventloop, see #116 for details.

Modifications:

Remove all blocking by buffering writes until the channel is ready to receive those i.e. it is active, TLS is negotiated or the HTTP to HTTP/2 upgrade was sucessfull.

Result:

No more blocking parts when using Netty on the client side.
2015-05-04 11:50:53 -07:00
Eric Anderson b6eb9d763d Don't close streams when sending GOAWAY
This reverts a change introduced in f920bad which caused all streams to
be closed when sending GOAWAY as part of graceful shutdown. This is
because lastKnownId() returns -1 when a GOAWAY has not been received.

The test doesn't pass, but at least appears to revert to the old
behavior. It is unknown if the test fails to pass due to client or
server, but given reports that the old code was working, we are thinking
that server-side GOAWAY handling is what is preventing the test from
passing.
2015-05-01 20:05:18 -07:00
Louis Ryan f2cba89e48 Update Netty to 9d70cf3 to pick up https://github.com/netty/netty/commit/8271c8a which eliminates
explicit flushing from Nettys HTTP2 codec.
2015-05-01 13:00:04 -07:00
Eric Anderson e05885d2aa Swap to Netty's SslContextBuilder
We provide a utility to configure the SslContext for our usage, which we
can change as necessary.
2015-05-01 08:42:12 -07:00
nmittler 7779b4fddf Cleaning up some warnings. 2015-04-30 13:57:38 -07:00
Louis Ryan d8d7908109 Avoid flushing CreateStreamCommand for calls where the client is known to send a payload immediately afterward.
Added simple JMH benchmark for Netty so improvements can be measured
2015-04-29 13:41:15 -07:00
nmittler 29c43c1c2d Upgrading to the latest Netty version. 2015-04-28 10:31:38 -07:00
Louis Ryan f679edcfcc Fix memeory leak in MessageFramer caused by allocation of 'empty' buffers never being released.
The buffers are not empty in reality as the allocators enforce a minimum size
2015-04-28 10:10:00 -07:00
Jakob Buchgraber 3a4f4b5a06 Add support for setting the connection and stream flow control window to NettyServerBuilder.
- Updated related comments in NettyServerBuilder and NettyChannelBuilder.
- Fixed related tests in NettyChannelBuilder.
2015-04-24 10:46:50 -07:00
Xudong Ma 2f4aa5a490 Use Status "UNKNOWN" instead of "INTERNAL" when Status.fromThrowable can not find a cause with a status. 2015-04-24 10:52:27 +08:00
Louis Ryan f7010f2eac Use message size as a hint to the allocator for buffer capacity.
Set upper and lower bounds for Netty & OkHttp allocators based on transport limitations and benchmark results.
Fix OkHttp OutboundFlowController to chunk payloads larger than frameWriter.maxDataLength
Allow OkHttp to allocate buffers to FrameWriter larger than max DATA length
2015-04-23 16:58:43 -07:00
nmittler 3ecbd0456a Upgrading to latest Netty version.
This is to resolve the issue of receiving frames for closed (and
therefore missing) streams.
2015-04-22 10:46:07 -07:00
Eric Anderson fc3e41674b Propagate explicit flushes through MessageFramer
MessageFramer allows queing of data and explicit flushing. Sinks
generally can benefit from knowing when they are required to flush, so
we now tell them when MessageFramer received a flush so they only have
to flush when required.
2015-04-21 10:29:08 -07:00
Eric Anderson 23aac9e9ec Fix netty closure check
During Service removal a condition was inverted but incompletely, which
caused the Netty server to never shutdown.
2015-04-16 12:29:24 -07:00
Eric Anderson 4f4f8e40bf Remove Guava's Service from server transport
ServerImpl.start() now throws IOException to make the error explicit.
This was previously being papered over by wrapping the exception in
RuntimeException.
2015-04-16 11:28:04 -07:00
nmittler f920badc5e Upgrading to the latest Netty version. 2015-04-16 08:42:56 -07:00
Xudong Ma 883eb62de4 OkHttp: Temporally support multiple h2-xx protocol on client side.
Since the user provided SSLSocketFactory (especially in Android) may already do the handshake when creates the SSLSocket, and it may choose a different protocol name other than the one OkHttp is using.

Resolves #293
2015-04-16 09:04:19 +08:00
Eric Anderson 5b2e336b6f netty: Add option to set MAX_CONCURRENT_STREAMS 2015-04-13 13:46:36 -07:00
Eric Anderson 1e257c440e Remove unused local variable 2015-04-10 08:58:43 -07:00
Jakob Buchgraber e8afa3ca23 Fix bug where the stream id would not get incremented for buffered streams. 2015-04-09 13:04:39 -07:00
Xudong Ma 9d214637e6 Temporally support Http protocol name "h2-16" for OkHttp client.
Fixes #22 for real.

Manually tested with jdk8 by:
./run-test-server.sh
./run-test-client.sh --server_host_override=foo.test.google.fr --use_test_ca=true --use_okhttp=true
2015-04-09 13:50:18 +08:00
Eric Anderson fd7ba566aa netty: Use the bootstrap ClassLoader for ALPN/NPN
If ALPN.class isn't in the bootclasspath, we don't want to find it as it
won't work. Also, if someone has fiddled with ClassLoaders, we really
want to make sure we get the proper ALPN class.

Passing "null" to Class.forName() means "bootstrap class loader". In
fact, ClassLoader.getParent() says, "Some implementations may use null
to represent the bootstrap class loader."

We must load the Proxy in the bootstrap class loader as well, because
our class loader may not have access to ALPN.{Client,Server}Provider,
for the same reasons as above. Even if we have multiple instances of
gRPC (due to class loaders), combined they will only create two Proxy
classes: one for ALPN.ClientProvider and one for ALPN.ServerProvider.
2015-04-08 15:37:44 -07:00
Jakob Buchgraber 44f9904ab0 Replace 'internal' import with correct one.
The errornous import was introduced by my previous commit 5f70a12cfc
2015-04-08 12:54:30 -07:00
Xudong Ma f3ccdd99f1 Fixes Travis breakage 2015-04-08 13:00:09 +08:00
Jakob Buchgraber 5f70a12cfc Remove blocking on client stream creation and buffer RPC Calls for when the MAX_CONCURRENT_STREAMS limit is hit. Fixes #118 2015-04-07 17:06:21 -07:00
Jakob Buchgraber ce860f9003 fix compile error introduced by previous commit 2015-04-07 13:00:24 -07:00
Jakob Buchgraber 105964bfac Fix memory leak by adding the Http2StreamRemovalPolicy to the channel pipeline.
In benchmarks we would see grpc talking up tens of gigabytes of memory. A heap dump
revealed that streams would not get cleaned up and stick around in memory forever.
2015-04-07 12:56:27 -07:00
Xudong Ma 9aae6f65fa checkstyle change:
1. Adds <property name="separateLineBetweenGroups" value="true"/> to CustomImportOrder to enfore blank line between imports groups.
2. Uses checkstyle 6.5, which fixed a bug of "CustomImportOrder checks import sorting according to ASCII order instead of case-insensitive alphabetical order".
2015-04-07 08:27:48 +08:00
Eric Anderson 74c9b5ed02 Fix JavaDoc references to non-imported class
The ServerImpl import was removed because it wasn't used as far as
checkstyle was able to determine. However, it was being used to resolve
JavaDoc references. Instead of re-adding the import just make the
reference fully qualified to prevent the two systems from continuing to
disagree on whether it is needed.
2015-04-03 15:11:51 -07:00
Xudong Ma 2cad9e6000 Add "unsed import" check. 2015-04-03 16:55:01 +08:00
nmittler 11d0094e04 Upgrading to the latest Netty 4.1 branch. 2015-03-31 14:14:29 -07:00
Eric Anderson 9bd31daee6 Wait for handler registration 2015-03-26 12:35:31 -07:00
Eric Anderson e515c772cd netty: Status should be based on GOAWAY code
goingAway() is called before onGoAwayRead() in Netty:
b7f57223c1/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java (L521)

The test before checked that the stream went away, but not that the
GOAWAY code influenced our Status, as UNAVAILABLE is the default
internally.

The UNAVAILABLE default has also been changed to include a message so
that we can determine where the Status came from in case it is triggered
again in the future.
2015-03-25 11:04:52 -07:00