Previously, if the content type was being ignored the error code would
have been UNKNOWN since there was no grpc-status. That seems very to
accidentally pass, so now if the content type is ignored it would get
OK.
ServerCall already had "headers must be sent before any messages, which
must be sent before closing," but the implementation did not enforce it
and our async server handler didn't obey.
The benefit of forcing sending headers first is that it removes the only
implicit call in our API and interceptors dealing just with metadata
don't need to override sendMessage. The implicit behavior was bug-prone
since it wasn't obvious you were forgetting that headers may not be
sent.
We want to allow overriding authority in the ManagedChannelBuilder for
testing. In doing that, we basically require that all Channels support
authority. In reality, this simplifies things and is already being done
by the C implementation, as their unix domain socket support uses
"localhost" just like our in-process transport now does.
We can debate some whether "localhost" is really the most appropriate
authority for the in-process transport, but that should probably happen
later since "localhost" is "good enough" for now.
Although the functionality is currently available by passing a
manually-created InetAddress, that requires that the user do I/O before
calling our API and does not work with naming in the future.
This provides an API for applications to use gRPC without using
ExperimentalApis. It also allows swapping out a transport implementation
in the future.
Resolves#926. Transports can still be alive when the Server shuts down,
but they are using the worker event loops. Only release the worker event
loops when all transport's channels are closed.
Client:
* New ManagedChannel abstract class.
* Adding ping to Channel.
* Moving builders and implementations to internal.
Server:
* Added lifecycle management API to Server (mirroring ManagedChannel).
* Moved ServerImpl, AbstractServerBuilder and handler registries to internal.
* New ServerBuilder abstract class (mirroring ManagedChannelBuilder).
Fixes#545
The URI no longer needs to be provided to the Credential explicitly,
which prevents needing to know a magic string and allows using the same
Credential with multiple services.
The current process of building a channel is a bit complicated in that transports have to provide a own shutdown hook to the channel builder in order to close shared executors. This somewhat entagled creation pattern makes it difficult to separate the process of channel building from transport building. Better separating these two should make the code more readable and maintainable moving forward.
This takes some steps towards #525, but it won't be fixed until we
officially support OpenSSL. This is due to the fact that ALPN->NPN fallback
isn't supported with Jetty (since only one of the bootstrap plugins can be provided).
Reserve io.grpc for public API only, and all internal stuff in core to
io.grpc.internal, including the non-stable transport API.
Raise the netty/okhttp/inprocess subpackages one level up to io.grpc,
because they are public API and entry points for most users.
Details:
- Rename io.grpc.transport to io.grpc.internal;
- Move SharedResourceHolder and SerializingExecutor to io.grpc.internal
- Rename io.grpc.transport.{netty|okhttp|inprocess} to
io.grpc.{netty|okhttp|inprocess}
ChannelInactive should be called in all cases of channel going down, so
we only need to cancel ping there. Use goAwayStatus for the error, since
we will be putting the most effort into making that status useful.
When connectionError was set, goAwayStatus was also set, so we shouldn't
lose any errors.
NettyClientTransport doesn't really need a Throwable, it just needs a
Status. Passing a Status out of NettyClientHandler reduces the number of
places that need to do transport-specific translation of Throwables into
Status codes.
As described in SslHandler's documentation, handshakeFuture() and
SslHandshakeCompletionEvent are equivalent forms of learning of
handshake completion. Watching both causes double-logging and serves no
purpose.
Otherwise new writes will be written to the channel and will fail in
some unhelpful way.
Logging was removed as we really want to propagate the failure back to
the application via Calls, which is done by failing the
CreateStreamCommand message. Propagating back to the application via
call removes uncontrollable log spam and is necessary anyway to inform
the application what sort of failure occurred in order to appropriately
to react.
Note that even more importantly, this translates a RST_STREAM error code
to a gRPC status code. This is generally useful, but also necessary for
DEADLINE_EXCEEDED to be more reliable in 0eae0d9.
Fixes#687
Improved some consistency. writeHeaders was the only non-final
implementation method of ServerStream, even though it is really no
different than the others.
Resolves#511.
- In generated code, make CONFIG private and METHOD_* fields public.
METHOD_* fields are MethodDescriptors now, users of the CONFIG field
should switch to using the METHOD_* fields.
- Move MethodType into MethodDescriptor (#529).
- Unify the fully qualified method name. It is fully qualified service
name + slash + short method name. It doesn't have the leading slash.
- HandlerRegistry switches the key from short method name to fully
qualified method name.
- Pass CallOptions to Channel.newCall() and
ClientInterceptor.interceptCall().
- Remove timeout from AbstractStub.StubConfigBuilder and add deadline,
which is stored in a CallOptions inside the stub.
- Deadline is in nanoseconds in the clock defined by System.nanoTime().
It is converted to timeout before transmitting on the wire. Fail the
call with DEADLINE_EXCEEDED if it's already expired.
The mapping is poorly suited for gRPC. C and Go don't even do any
mapping. We can improve the mapping in the future, but it is very
important that users don't start depending on the current mapping.
This change is "inspired by" the original code, but is even more
conservative.
Fixes#477
GCM is very slow, and doesn't provide any benefit in unit tests. Even if
we were using tcnative and GCM is fast, using more available ciphers in
tests still makes sense. With this change building with Java 7 works
again, although that isn't the reason for the change.
On my machine with parallel building, it cuts full build time from
92 seconds to 39 seconds. For an incremental build after only changing
an interop test, the build time is cut from 73 seconds to 15 seconds.
When shutting down the Netty event loop, we have already guaranteed that
all users of it are no longer running. Doing a shutdownGracefully is
just delaying graceful JVM termination by two seconds. This is very
noticeable for short-lived processes, like our integration tests.
We would actually also prefer to shutdown quickly and get a
RejectedExecutionException for any newly queued tasks, because that
would be a legitimate bug.
shutdown() is deprecated, thus we do shutdownGracefully with a timeout
of 0.
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()).
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.
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.
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.
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.
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
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.
ServerImpl.start() now throws IOException to make the error explicit.
This was previously being papered over by wrapping the exception in
RuntimeException.
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
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.
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.
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".
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.
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.
The checkstyle.xml is a slightly modified version of the upstream Google
checkstyle configuration. All changes have comment describing them.
Lots of warnings were corrected. Examples is the only project that has
warnings still, as the necessary changes require some thought.
The Http2ClientStream should not close the buffer in this case since
it's already been given to the deframer and potentially to the user.
Added cleanup code to MessageDeframer and AbstractClientStream to make
sure that we free the Buffer when appropriate.
As part of the effort to remove all blocking bits from the NettyClientStream with
this commit the SendGrpcFrameCommand now takes a stream object instead of the
stream id. This will be necessary as in a non blocking world the stream id might
not have yet been allocated when the SendGrpcFrameCommand gets instantiated.
- Renamed 'eventGroup' property to 'group' as this what's used elsewhere.
- Moved assignment of the channel before the listener is added as currently
there is a (theoretical) chance that the listener is executed before the assignment
to channel happens, namely in case the Future is already done when the listener
is added.
- Removed comment that seemed out of place / relict.
As part of the effort to remove all blocking bits from the NettyClientStream with
this commit the SendGrpcFrameCommand now takes a stream object instead of the
stream id. This will be necessary as in a non blocking world the stream id might
not have yet been allocated when the SendGrpcFrameCommand gets instantiated.
Xudong Ma
nmittler
Carl Mastrangelo
Eric Anderson
Kun Zhang
Jack Coughlin
Josh Humphries
Louis Ryan
Jakob Buchgraber