Commit Graph

124 Commits

Author SHA1 Message Date
Scott Mitchell
edb91afcd6 Http2LifecycleManager.onException rename
Motivation:
Http2LifecycleManager.onException takes a Throwable as a paramter and not an Exception. There are also onConnectionError and onStreamError methods in the codec. We should rename this method to onError for consistency and clarity.

Modifications:
- Rename Http2LifecycleManager.onException to Http2LifecycleManager.onError

Result:
More consistent and clarified interface.
2015-09-23 16:48:36 -07:00
Scott Mitchell
24c9407080 DefaultHttp2RemoteFlowController may not write all pending bytes
Motivation:
DefaultHttp2RemoteFlowController attempts to write as many bytes as possible to transition the channel to not writable, and then relies on notification of channelWritabilityChange to continue writing. However the amount of bytes written by DefaultHttp2RemoteFlowController may not be the same number of bytes that is actually written to the channel due to other ChannelHandlers (SslHandler, compression, etc...) in the pipeline. This means there is a potential for the DefaultHttp2RemoteFlowController to be waiting for a channel writaiblity change event that will never come, and thus not write all queued data.

Modifications:
- DefaultHttp2RemoteFlowController should write pending bytes until there are no more, or until the channel is not writable.

Result:
DefaultHttp2RemoteFlowController will write all pending data.
Fixes https://github.com/netty/netty/issues/4242
2015-09-23 16:39:24 -07:00
nmittler
ec20902613 Don't set HTTP/2 flow controller ctx to null
Motivation:

We currently set the flow controller ChannelHandlerContexts to null when the channel becomes inactive. This is bad :)

Modifications:

Just remove that code in Http2ConnectionHandler

Result:

Fixes #4240
2015-09-22 07:25:42 -07:00
fratboy
6241bb059c [#4244] Convert urlencoded uri to http2 path correctly
Motivation:

HttpConversionUtil.toHttp2Headers does not convert urlencoded uri to http2 path properly.

Modifications:

Use getRawPath(), getRawQuery(), getRawFragment() in java.net.URI when converts to http2 path

Result:

HttpConversionUtil.toHttp2Headers does not urldecode uri unproperly.
2015-09-21 16:30:59 -07:00
Scott Mitchell
c7e3f6c6fd HTTP/2 defines using String instead of CharSequence
Motivation:
Http2CodecUtils has some static variables which are defined as Strings instead of CharSequence. One of these defines is used as a header name and should be AsciiString.

Modifications:
- Change the String defines in Http2CodecUtils to CharSequence

Result:
Types are more consistently using CharSequence and adding the upgrade header will require less work.
2015-09-16 14:55:33 -07:00
Scott Mitchell
1d4d5fe312 DefaultHttp2Headers should throw exception of type Http2Exception
Motivation:
The DefaultHttp2Headers code is throwing a IllegalArgumentException if an invalid character is detected. This is being ignored by the HTTP/2 codec instead of generating a GOAWAY.

Modifications:
- Throw a Http2Exception of type PROTOCOL_ERROR in accordance with https://tools.ietf.org/html/rfc7540#section-8.1.2.6
- Update examples which were building invalid headers

Result:
More compliant with https://tools.ietf.org/html/rfc7540#section-8.1.2.6
2015-09-16 13:47:05 -07:00
Scott Mitchell
15450af2e7 DefaultHttp2FrameWriter ping payload size check
Motivation:
The HTTP/2 spec states that the ping frame length must be 8 and is otherwise an error https://tools.ietf.org/html/rfc7540#section-6.7. The DefaultHttp2FrameReader enforces this, but the DefaultHttp2FrameWriter allows invalid frames to be written. We should not allow invalid ping frames to be written to the network.

Modifications:
- DefaultHttp2FrameWriter checks the frame size to be 8, or throws an exception

Result:
Fixes https://github.com/netty/netty/issues/3721
2015-09-16 10:25:07 -07:00
Scott Mitchell
59600f1812 HTTP/2 to HTTP/1.x headers conversion more accessible
Motivation:
Currently there is a HttpConversionUtil.addHttp2ToHttpHeaders which requires a FullHttpMessage, but this may not always be available. There is no interface that can be used with just Http2Headers and HttpHeaders.

Modifications:
- add an overload for HttpConversionUtil.addHttp2ToHttpHeaders which does not take FullHttpMessage

Result:
An overload for HttpConversionUtil.addHttp2ToHttpHeaders exists which does not require FullHttpMessage.
2015-09-16 10:02:48 -07:00
Scott Mitchell
ba11879c9f HTTP/2 codec heap buffer usage
Motivation:
The HTTP/2 codec has a few static buffers sent over the network which are allocated on the heap. This results in a copy operation when the buffer is sent out on the network.

Modifications:
- Ensure these static buffers are allocated using direct memory.

Result:
No copy operation necessary when writing static buffers to network.
2015-09-14 13:12:02 -07:00
Scott Mitchell
f89dfb0bd5 Deprecation cleanup for HTTP headers
Motivaion:
The HttpHeaders and DefaultHttpHeaders have methods deprecated due to being removed in future releases, but no replacement method to use in the current release. The deprecation policy should not be so aggressive as to not provide any non-deprecated method to use.

Modifications:
- Remove deprecated annotations and javadocs from methods which are the best we can do in terms of matching the master's api for 4.1

Result:
There should be non-deprecated methods available for HttpHeaders in 4.1.
2015-09-09 14:30:21 -07:00
Scott Mitchell
47726991b2 HTTP/2 Header Name Validation
Motivation:
The HTTP/2 header name validation was removed, and does not currently exist.

Modifications:
- Header name validation for HTTP/2 should be restored and set to the default mode of operation.

Result:
HTTP/2 header names are validated according to https://tools.ietf.org/html/rfc7540
2015-09-09 13:59:08 -07:00
Scott Mitchell
50d1f0a680 Http2Headers.iterator() comment correction
Motivation:
The javadoc comments on Http2Headers.iterator() are incorrect.

Modifications:
- Correct and clarify the javadoc for Http2Headers.iterator()

Result:
Javadoc for Http2Headers.iterator() is more correct.
2015-09-04 12:43:26 -07:00
Norman Maurer
34de2667c7 [maven-release-plugin] prepare for next development iteration 2015-09-02 11:45:20 +02:00
Norman Maurer
2eb444ec1d [maven-release-plugin] prepare release netty-4.1.0.Beta6 2015-09-02 11:36:11 +02:00
Scott Mitchell
41ee9148e5 HTTP/2 InboundHttp2ToHttpAdapterTest serverChannel NPE
Motivation:
InboundHttp2ToHttpAdapterTest.bootstrapEnv does not wait for the serverConnectedChannel to be initialized before returning. Some methods rely only this behavior and throw a NPE because it may not be set.

Modifications:
- Add a CountDownLatch to ensure the serverConnectedChannel is initialized

Result:
No more NPE.
2015-09-01 10:38:04 -07:00
Scott Mitchell
0736a3bc35 HTTP/2 SimplePromiseAggregator tryFailure not consistent with setFailure
Motivation:
The SimplePromiseAggregator.setFailure allows a failure to occur before newPromise is called, but tryFailure doesn't. These methods should be consistent.

Modifications:
- tryFailure should use the same logic as setFailure

Result:
Consistent failure routines.
2015-09-01 10:35:10 -07:00
Scott Mitchell
50cc647804 DefaultPropertyKey private member variable accessed outside scope
Motivation:
DefaultPropertyKey.index is currently private and accessed outside the class's scope.

Modifications:
- Change access level to package private

Result:
No chance of synthetic method generation for accessing this field
2015-08-28 08:54:11 -07:00
Scott Mitchell
0365927951 HTTP/2 InboundHttp2ToHttpAdapterTest race condition
Motivation:
The latches in InboundHttp2ToHttpAdapterTest were volatile and reset during the tests. This resulted in race conditions and sometimes the tests would be waiting on old latches that were not the same latches being counted down when messages were received.

Modifications:
- Remove volatile latches from tests

Result:
More reliable tests with less race conditions.
2015-08-28 08:50:08 -07:00
Scott Mitchell
b6a4f5de9d Refactor of HttpUtil and HttpHeaderUtil
Motivation:
There currently exists http.HttpUtil, http2.HttpUtil, and http.HttpHeaderUtil. Having 2 HttpUtil methods can be confusing and the utilty methods in the http package could be consolidated.

Modifications:
- Rename http2.HttpUtil to http2.HttpConversionUtil
- Move http.HttpHeaderUtil methods into http.HttpUtil

Result:
Consolidated utilities whose names don't overlap.
Fixes https://github.com/netty/netty/issues/4120
2015-08-27 08:49:58 -07:00
Scott Mitchell
14dc571956 Http2ConnectionHandler channelInactive sequencing
Motivation:
ByteToMessageDecoder may call decode after channelInactive is called. This will lead to a NPE.

Modifications:
- Call super.channelInactive() before we process the event in Http2ConnectionHandler

Result:
No more NPE in decode.
2015-08-26 15:31:47 -07:00
Scott Mitchell
99d6a97b4a HTTP to HTTP/2 translation errors (round 2)
Motivation:
Commit 0d8ce23c83 failed to fix the Host header processing. Host is not a URI but is instead defined in https://tools.ietf.org/html/rfc3986#section-3.2.2 as host        = IP-literal / IPv4address / reg-name

Modifications:
- Host should not be treated as a URI.
- We should be more explicit about required fields, and unexpected input by throwing exceptions.

Result:
Translation from HTTP/1.x to HTTP/2 is more correct.
2015-08-24 20:37:58 -07:00
Scott Mitchell
85c79dbbe4 HTTP to HTTP/2 tranlation errors
Motivation:
HttpUtil.toHttp2Headers is currently not translating HTTP request headers to HTTP/2 request headers correctly.  The path, scheme, and authority are tranlation process are not respecting the HTTP/2 RFC https://tools.ietf.org/html/rfc7540#section-8.1.2.3 and HTTP RFC https://tools.ietf.org/html/rfc7230#section-5.3.

Modifications:
- path, scheme, authority must be set according to rules defined in https://tools.ietf.org/html/rfc7540#section-8.1.2.3
- HTTP/1.x URIs must be handled as defined in https://tools.ietf.org/html/rfc7230#section-5.3

Result:
More correct translation from HTTP/1.x requests to HTTP/2 requests.
2015-08-21 11:33:10 -07:00
Scott Mitchell
ac9ae14bd9 HTTP/2 SimpleChannelPromiseAggregator failure condition
Motivation:
If a SimpleChannelPromiseAggregator is failed before any new promises are generated, the failure is not propegated through to the aggregated promise.

Modifications:
- Failures should be allowed to occur even if no new promises have been generated

Result:
Failures are always allowed.
2015-08-21 11:26:31 -07:00
Scott Mitchell
08477eaf03 HTTP/2 Graceful Shutdown Timeout
Motivation:
If any streams are still active the graceful shutdown code will wait until they are all closed before the connection is closed. In some situations this event may never occur, and thus a timeout should be supported so the socket can be closed even if all streams haven't been closed.

Modifications:
- Add a configurable timeout for when the graceful shutdown process is attempted.
- Update unit tests to be faster, and use this graceful timeout

Result:
Local endpoint can protect from local or remote issues which prevent the channel from being closed during the graceful shutdown process.
2015-08-20 13:27:18 -07:00
Scott Mitchell
34dfa7a2d8 DefaultHttp2ConnectionEncoder private constructors on inner classes
Motivation:
DefaultHttp2ConnectionEncoder.FlowControlledHeaders and DefaultHttp2ConnectionEncoder.FlowControlledData have private constructors which may result in static factory methods being generated to construct instances of these classes.

Modifications:
- Make constructors public for these private classes

Result:
Accessor for inner class constructor more correct and no possibiliy of synthetic method generation.
2015-08-19 13:30:24 -07:00
Scott Mitchell
ba6ce5449e Headers Performance Boost and Interface Simplification
Motivation:
A degradation in performance has been observed from the 4.0 branch as documented in https://github.com/netty/netty/issues/3962.

Modifications:
- Simplify Headers class hierarchy.
- Restore the DefaultHeaders to be based upon DefaultHttpHeaders from 4.0.
- Make various other modifications that are causing hot spots.

Result:
Performance is now on par with 4.0.
2015-08-17 08:50:11 -07:00
Scott Mitchell
2d4bef9b18 Http2ConnectionHandler not flushing on writabilityChange
Motivation:
The Http2ConnectionHandler was writing pending bytes, but was not flushing. This may result in deadlock.

Modifications:
- Http2ConnectionHandler must writePendingBytes and also flush.

Result:
Data is now flushed after writabilityChange writes more data to underlying layers.
2015-08-13 09:43:00 -07:00
Jakob Buchgraber
6fd0a0c55f Faster and more memory efficient headers for HTTP, HTTP/2, STOMP and SPYD. Fixes #3600
Motivation:

We noticed that the headers implementation in Netty for HTTP/2 uses quite a lot of memory
and that also at least the performance of randomly accessing a header is quite poor. The main
concern however was memory usage, as profiling has shown that a DefaultHttp2Headers
not only use a lot of memory it also wastes a lot due to the underlying hashmaps having
to be resized potentially several times as new headers are being inserted.

This is tracked as issue #3600.

Modifications:
We redesigned the DefaultHeaders to simply take a Map object in its constructor and
reimplemented the class using only the Map primitives. That way the implementation
is very concise and hopefully easy to understand and it allows each concrete headers
implementation to provide its own map or to even use a different headers implementation
for processing requests and writing responses i.e. incoming headers need to provide
fast random access while outgoing headers need fast insertion and fast iteration. The
new implementation can support this with hardly any code changes. It also comes
with the advantage that if the Netty project decides to add a third party collections library
as a dependency, one can simply plug in one of those very fast and memory efficient map
implementations and get faster and smaller headers for free.

For now, we are using the JDK's TreeMap for HTTP and HTTP/2 default headers.

Result:

- Significantly fewer lines of code in the implementation. While the total commit is still
  roughly 400 lines less, the actual implementation is a lot less. I just added some more
  tests and microbenchmarks.

- Overall performance is up. The current implementation should be significantly faster
  for insertion and retrieval. However, it is slower when it comes to iteration. There is simply
  no way a TreeMap can have the same iteration performance as a linked list (as used in the
  current headers implementation). That's totally fine though, because when looking at the
  benchmark results @ejona86 pointed out that the performance of the headers is completely
  dominated by insertion, that is insertion is so significantly faster in the new implementation
  that it does make up for several times the iteration speed. You can't iterate what you haven't
  inserted. I am demonstrating that in this spreadsheet [1]. (Actually, iteration performance is
  only down for HTTP, it's significantly improved for HTTP/2).

- Memory is down. The implementation with TreeMap uses on avg ~30% less memory. It also does not
  produce any garbage while being resized. In load tests for GRPC we have seen a memory reduction
  of up to 1.2KB per RPC. I summarized the memory improvements in this spreadsheet [1]. The data
  was generated by [2] using JOL.

- While it was my original intend to only improve the memory usage for HTTP/2, it should be similarly
  improved for HTTP, SPDY and STOMP as they all share a common implementation.

[1] https://docs.google.com/spreadsheets/d/1ck3RQklyzEcCLlyJoqDXPCWRGVUuS-ArZf0etSXLVDQ/edit#gid=0
[2] https://gist.github.com/buchgr/4458a8bdb51dd58c82b4
2015-08-04 17:12:24 -07:00
Scott Mitchell
7bd6472804 HTTP/2 DataCompressionHttp2Test test complete race condition
Motivation:
The DataCompressionHttp2Test was exiting prematurely leading to unit test failures.

Modifications:
- Fix the race condition so the test does not evaluate final conditions until all expected events occur

Result:
Unit test no longer fails
2015-08-04 13:57:40 -07:00
nmittler
94f65ed7ff Use standard syntax for logging HTTP/2 stream ID.
Motivation:

When looking through the logs for entries pertaining to a specific stream, it's difficult because header entries use the syntax "streamId:<id>" but all other entries use "streamId=<id>". We should make all of the entries consistent.

Modifications:

Changed header entries to use "streamId=<id>" to match the other entries.

Result:

Easier HTTP/2 log navigation.
2015-07-28 09:54:14 -07:00
nmittler
296649cfc8 Make PrimitiveCollections generated for all primitive maps.
Motivation:

We should support XXXCollections methods for all primitive map types.

Modifications:

Removed PrimitiveCollections and added a template for XXXCollections.

Result:

Fixes #4001
2015-07-27 06:59:23 -07:00
nmittler
93fc3c6e45 Make IntObjectHashMap extend Map
Motivation:

It would be useful to support the Java `Map` interface in our primitive maps.

Modifications:

Renamed current methods to "pXXX", where p is short for "primitive". Made the template for all primitive maps extend the appropriate Map interface.

Result:

Fixes #3970
2015-07-22 15:52:27 -07:00
Brendt Lucas
57d28dd421 Support conversion of HttpMessage and HttpContent to HTTP/2 Frames
Motivation:

HttpToHttp2ConnectionHandler only converts FullHttpMessage to HTTP/2 Frames. This does not support other use cases such as adding a HttpContentCompressor to the pipeline, which writes HttpMessage and HttpContent.

Additionally HttpToHttp2ConnectionHandler ignores converting and sending HTTP trailing headers, which is a bug as the HTTP/2 spec states that they should be sent.

Modifications:

Update HttpToHttp2ConnectionHandler to support converting HttpMessage and HttpContent to HTTP/2 Frames.
Additionally, include an extra call to writeHeaders if the message includes trailing headers

Result:

One can now write HttpMessage and HttpContent (http chunking) down the pipeline and they will be converted to HTTP/2 Frames.  If any trailing headers exist, they will be converted and sent as well.
2015-07-21 20:17:27 +02:00
nmittler
0d73907c58 Fixing NPE in StreamBufferingEncoderTest
Motivation:

The bufferingNewStreamFailsAfterGoAwayReceived method currently causes an NPE.

Modifications:

Fixed the test so that a valid ByteBuf is passed in.

Result:

The test no longer throws an NPE.
2015-07-20 13:47:26 -07:00
Scott Mitchell
74dd7f85ca HTTP/2 Thread Context Interface Clarifications
Motivation:
It is currently assumed that all usages of the HTTP/2 codec will be from the same event loop context. If the methods are used outside of the assumed thread context then unexpected behavior is observed. This assumption should be more clearly communicated and enforced in key areas.

Modifications:
- The flow controller interfaces have assert statements and updated javadocs indicating the assumptions.

Result:
Interfaces more clearly indicate thread context limitations.
2015-07-17 12:40:36 -07:00
Scott Mitchell
d7cdc469bc HTTP/2 CompressorHttp2ConnectionEncoder bug
Motivation:
The CompressorHttp2ConnectionEncoder is attempting to attach a property to streams before the exist.

Modifications:
- Allow the super class to create the streams before attempting to attach a property to the stream.

Result:
CompressorHttp2ConnectionEncoder is able to set the property and access the compressor.
2015-07-17 09:59:55 -07:00
Scott Mitchell
9747ffe5fc HTTP/2 Flow Controller should use Channel.isWritable()
Motivation:
See #3783

Modifications:
- The DefaultHttp2RemoteFlowController should use Channel.isWritable() before attempting to do any write operations.
- The Flow controller methods should no longer take ChannelHandlerContext. The concept of flow control is tied to a connection and we do not support 1 flow controller keeping track of multiple ChannelHandlerContext.

Result:
Writes are delayed until isWritable() is true. Flow controller interface methods are more clear as to ChannelHandlerContext restrictions.
2015-07-16 14:38:48 -07:00
Ryo Okubo
aaba1b9ed5 Accept over 2^31-1 MAX_HEADER_LIST_SIZE
Motivation:

The MAX_HEADER_LIST_SIZE of SETTINGS is represented by
unsigned 32-bit value and this value isn't limited in RFC7540.
But in current implementation, its stored to int variable so
over 2^31-1 value is recognized as minus and handled as
PROTOCOL_ERROR.

Modifications:

If a value of MAX_HEADER_LIST_SIZE is larger than 2^31-1, its
handled as 2^31-1

Result:

Over 2^31-1 MAX_HEADER_LIST_SIZE is became acceptable
2015-07-10 10:58:41 -07:00
Louis Ryan
60c59f39af Use CoalescingBufferQueue to merge data writes on a stream in HTTP2 instead of CompositeByteBuf
Motivation:

Slicing a mutable CompositeByteBuf is not the appropriate mechanism to use to track and release buffers that have been written to a channel.
In particular buffers passed over an Embedded or LocalChannel are retained after the ChannelPromise is completed and listening to the
promise to consolidate a CompositeBuffer breaks slices taken from the composite as the offset indices have changed.

In addition CoalescingBufferQueue handles taking arbitrarily sized slices of a sequence of buffers more efficiently.

Modifications:

Convert FlowControlledData to use a CoalescingBufferQueue to handle merging data writes.

Result:

HTTP2 works over LocalChannel and code is considerably simpler.
2015-07-09 10:47:18 -07:00
zhangduo
e949dcd94f Allow numBytes == 0 when calling Http2LocalFlowController.consumeBytes.
Motivation:

Sometimes people use a data frame with length 0 to end a stream(such as jetty http2-server). So it is possible that data.readableBytes and padding are all 0 for a data frame, and cause an IllegalArgumentException when calling flowController.consumeBytes.

Modifications:

Return false when numBytes == 0 instead of throwing IllegalArgumentException.

Result:

Fix IllegalArgumentException.
2015-07-09 08:43:52 -07:00
nmittler
6e044b082c Proper shutdown of HTTP2 encoder when channelInactive
Motivation:

The problem is described in https://github.com/grpc/grpc-java/issues/605. Basically, when using `StreamBufferingEncoder` there is a chance of creating zombie streams that never get closed.

Modifications:

Change `Http2ConnectionHandler`'s `channelInactive` handling logic to shutdown the encoder/decoder before shutting down the active streams.

Result:

Fixes https://github.com/grpc/grpc-java/issues/605
2015-07-09 07:36:42 -07:00
Norman Maurer
61b9da470a [#3945] Http2ConnectionHandler breaks channelReadComplete pipeline notification
Motivation:

Http2ConnectionHandler missed to forward channelReadComplete(...) events.

Modifications:

Ensure we notify the next handler in the pipeline via ctx.fireChannelReadComplete().

Result:

Correctly forwarding of event.
2015-07-08 08:04:02 +02:00
Scott Mitchell
ecacd11b06 HTTP/2 limit header accumulated size
Motivation:
The Http2FrameListener requires that the Http2FrameReader accumulate ByteBuf objects. There is currently no way to limit the amount of bytes that is accumulated.

Motiviation:
- DefaultHttp2FrameReader supports maxHeaderSize which will fail fast as soon as the maximum size is exceeded.
- DefaultHttp2HeadersDecoder will respect the return value of endHeaderBlock() and fail if the max size is exceeded.

Result:
Frames which carry header data now respect a maximum number of bytes that can be accumulated.
2015-07-07 13:25:03 -07:00
Ryo Okubo
ba84a596e2 Allow servers to specify ENABLE_PUSH to 0 explicitly
Motivation:

If server sends SETTINGS with ENABLE_PUSH, its handled as
PROTOCOL_ERROR in spite of the value. But the value specified to
0 may be allowed in RFC7540.

Modifications:

Check whether ENABLE_PUSH sent from a server is 0 or not.

Result:

When server specifies ENABLE_PUSH to 0 explicitly, client doesn't
handle it as PROTOCOL_ERROR.
2015-07-07 10:00:58 -07:00
nmittler
b39223e295 Fixing exception in StreamBufferingEncoderTest.
Motivation:

NPE doesn't cause the tests to fail but should get fixed.

Modifications:

Modified the StreamBufferingEncoderTest to mock the ctx to return a promise.

Result:

Fixes #3905
2015-06-24 12:10:09 -07:00
nmittler
391df0547b Fixing broken build in the 4.1 branch. 2015-06-19 16:08:37 -07:00
Louis Ryan
05ce33f5ca Make the flow-controllers write fewer, fatter frames to improve throughput.
Motivation:

Coalescing many small writes into a larger DATA frame reduces framing overheads on the wire and reduces the number of calls to Http2FrameListeners on the remote side.
Delaying the write of WINDOW_UPDATE until flush allows for more consumed bytes to be returned as the aggregate of consumed bytes is returned and not the amount consumed when the threshold was crossed.

Modifications:
- Remote flow controller no longer immediately writes bytes when a flow-controlled payload is enqueued. Sequential data payloads are now merged into a single CompositeByteBuf which are written when 'writePendingBytes' is called.
- Listener added to remote flow-controller which observes written bytes per stream.
- Local flow-controller no longer immediately writes WINDOW_UPDATE when the ratio threshold is crossed. Now an explicit call to 'writeWindowUpdates' triggers the WINDOW_UPDATE for all streams who's ratio is exceeded at that time. This results in
  fewer window updates being sent and more bytes being returned.
- Http2ConnectionHandler.flush triggers 'writeWindowUpdates' on the local flow-controller followed by 'writePendingBytes' on the remote flow-controller so WINDOW_UPDATES preceed DATA frames on the wire.

Result:
- Better throughput for writing many small DATA chunks followed by a flush, saving 9-bytes per coalesced frame.
- Fewer WINDOW_UPDATES being written and more flow-control bytes returned to remote side more quickly, thereby improving throughput.
2015-06-19 15:20:31 -07:00
nmittler
1ecc37fbb2 Better error when first HTTP/2 frame is not SETTINGS
Motivation:

Bootstrap of the HTTP/2 can take a lot of paths and a lot of things can go wrong in the initial handshakes leading up to establishment of HTTP/2 between client and server. There have been many times where handshakes have failed silently, leading to very cryptic errors that are hard to debug.

Modifications:

Changed the HTTP/2 handler and decoder to ensure that the very first data on the wire (WRT HTTP/2) is SETTINGS/preface+SETTINGS. When this is not the case, a connection error is thrown with the bytes that were found instead.

Result:

Fixes #3880
2015-06-18 15:58:42 -07:00
Trustin Lee
0ca65f1373 Lazily instantiate HttpServerUpgradeHandler.UpgradeCodec
Related: #3814

Motivation:

To implement the support for an upgrade from cleartext HTTP/1.1
connection to cleartext HTTP/2 (h2c) connection, a user usually uses
HttpServerUpgradeHandler.

It does its job, but it requires a user to instantiate the UpgradeCodecs
for all supported protocols upfront. It means redundancy for the
connections that are not upgraded.

Modifications:

- Change the constructor of HttpServerUpgradeHandler
  - Accept UpgraceCodecFactory instead of UpgradeCodecs
- The default constructor of HttpServerUpgradeHandler sets the
  maxContentLength to 0 now, which shouldn't be a problem because a
  usual upgrade request is a GET.
- Update the examples accordingly

Result:

A user can instantiate Http2ServerUpgradeCodec and its related objects
(Http2Connection, Http2FrameReader/Writer, Http2FrameListener, etc) only
when necessary.
2015-06-10 12:06:27 +09:00
Trustin Lee
73d79a4b3b Do not use hard-coded handler names in HTTP/2
Motivation:

Our HTTP/2 implementation sometimes uses hard-coded handler names when
adding/removing a handler to/from a pipeline. It's not really a good
idea because it can easily result in name clashes. Unless there is a
good reason, we need to use the reference to the handlers

Modifications:

- Allow null as a handler name for Http2Client/ServerUpgradeCodec
  - Use null as the default upgrade handler name
- Do not use handler name strings in some test cases and examples

Result:

Fixes #3815
2015-06-10 11:46:02 +09:00