Commit Graph

674 Commits

Author SHA1 Message Date
Eric Anderson 63661c7b70 netty: Add Bazel target similar to netty-shaded
See census-instrumentation/opencensus-java#1963
2019-08-21 09:48:20 -07:00
Carl Mastrangelo 21c34d7862
netty: check for null protocol negotiator in NettyChannelBuilder 2019-08-15 11:11:31 -07:00
Carl Mastrangelo 458f4533db
netty: migrate Server protocol negotiation to new style
* Revert "Revert "netty: change server to new protocol negotiator model" (#5798)"

This reverts commit 4e5e19f6fd.
2019-08-14 13:00:42 -07:00
Eric Anderson 9fcfb5b4f8
netty: Limit number of frames client can cause server to enqueue
Http2ControlFrameLimitEncoder is from Netty. It is copied here as a
temporary measure until we upgrade to the version of Netty that includes
the class.

See CVE-2019-9515
2019-08-13 12:24:15 -05:00
Laurent Le Brun 503696aa9a bazel: Remove missing pattern from glob
There's no third_party directory.
This change makes the repository compatible with
Bazel change --incompatible_disallow_empty_glob.
2019-08-08 16:41:17 -07:00
Jihun Cho 4faad27078
netty: record ByteBuf access in Netty{Client,Server}Stream before calling MessageDeframer (#6027) 2019-08-02 16:25:08 -07:00
Jihun Cho 65109e6738
netty: Netty{Server,Channel}Builder requires all or none of ELG and ChannelType (#6014) 2019-07-26 09:25:02 -07:00
Carl Mastrangelo cc524f10d3
netty,interop-testing: increase timeouts on tests for TSAN 2019-07-19 15:24:19 -07:00
Carl Mastrangelo 25a72e1f6d
netty: don't share transport tracers across transports
Found using TSAN, which shows the number of calls succeeded being incremented from multiple event loops
2019-07-15 11:18:35 -07:00
Carl Mastrangelo 855740622a
all: bump PerfMark to 0.17.0
* Bump PerfMark to 0.17.0

The main changes how linking is done.  Linking is now always done
through the `PerfMark` entry class.   This is for two reasons:

1.  It make instrumenting the linking calls *much* easier.
2.  It follows the API pattern of "verbNoun()".  Previous callsites
    would have `Link link = PerfMark.link(); link.link()`.  This
    stuttering is not quick to follow.

Generated using:

```
find -name \*.java -exec sed -i 's#link = PerfMark.link();#link = PerfMark.linkOut();#g' {} \;
find -name \*.java -exec sed -i 's#link.link();#PerfMark.linkIn(link);#g' {} \;
find -name \*.java -exec sed -i 's#command.getLink().link();#PerfMark.linkIn(command.getLink());#g' {} \;
find -name \*.java -exec sed -i 's#cmd.getLink().link();#PerfMark.linkIn(cmd.getLink());#g' {} \;
find -name \*.java -exec sed -i 's#msg.getLink().link();#PerfMark.linkIn(msg.getLink());#g' {} \;
```

Since the deprecated link methods are also `@DoNotCall`, the same
sed calls will need to be used on import.
2019-07-11 10:57:05 -07:00
Carl Mastrangelo 9aa3981ada
netty: use varargs logger in ProtocolNegotiators 2019-06-27 15:59:43 -07:00
Carl Mastrangelo 023b9a3f14
netty: don't use InternalProtocolNegotiators in ProtocolNegotiators 2019-06-27 15:07:43 -07:00
Carl Mastrangelo 0a1805db43
netty: include pipeline on exceptions too 2019-06-27 13:04:31 -07:00
Carl Mastrangelo 9e5f60b86a
netty: upstream ProtocolNegotiatiorHandler, and swap the appropriate classes to it
ALTS is not switched yet, since it is shared between client and server. Once the server is changed to use WBAEH, it can be moved too.
2019-06-26 18:23:12 -07:00
Carl Mastrangelo 7895c33efa
netty: make unexpected reads fail negotiation, and log close failures
In case a negotiating handler misses a read, and it reaches the WBAEH, it should cause a failure. Also, if closing the channel fails while handling another error, log the second failure.
2019-06-26 14:43:06 -07:00
ZHANG Dapeng 18645422c9
netty: Delete deprecated HandlerSettings 2019-06-20 11:34:19 -07:00
Eric Anderson 29cb7c4dd4 netty: Use await instead of sync
We only care about when closing is done, not whether it is successful or not.
If there's a failure, we're already going to log a warning. Use await to avoid
throwing unexpectedly.
2019-06-20 07:19:56 -07:00
Carl Mastrangelo 74e945ceb4
core,netty: block server shutdown until the socket is unbound 2019-06-19 17:23:08 -07:00
Eric Anderson 3d289213ce
netty-shaded: Use compile instead of runtime dependency in pom (#5901)
Maven does not include transitive runtime dependencies in the
compile-time classpath (testing shows Gradle 4 does; docs say
Gradle 5 doesn't). So if a user references the shaded
NettyServerBuilder without also depending on grpc-core directly,
compilation will fail because AbstractServerImplBuilder couldn't
be found.

This isn't technically a problem, since we're not wanting to encourage
users to reference the shaded classes directly. But some users will
certainly reference the classes anyway and the error is pretty confusing
while also being trivially worked around. In other words: it justs
wastes people's time and benefits nobody.

Fixes #5881
2019-06-19 09:51:00 -07:00
Carl Mastrangelo 9c9ca659d4
netty,alts: fire initial protocol negotiation event in WBAEH
This change is needed after trying to use the new style protocol negotiators internally.  The problem is that some handlers fire the event in handlerAdded, which is too early.  The followup PNE is fired after handlerAdded, which breaks the composibility of the negotiators.

To fix this, this change modifies the negotiation flow.  Specifically:

* Negotiators should NEVER fire a negotiation from handlerAdded, instead they should wait until userEventTriggered
* Negotiators now do state checking on the PNE.  If it is set twice, it fails.  If it has not been received when doing the next stage of negotiation, it fails.
* WBAEH now fires the initial, default event.  This is the only handler that can fire it from handlerAdded

The tests updated are ones not using WBAEH (which they probably should).  This change ensures attributes aren't lost when doing negotiation.
2019-06-18 09:33:40 -07:00
Carl Mastrangelo 40854dc9e1
core,netty: use PerfMark tags with the HTTP/2 stream ids
This change removes the WriteQueue linking and splits it out into each
of the commands, so that the trace is more precise, and the tag
information is correct.

It is still unclear what the initial Tag should be for ClientCallImpl,
since it should not access the TransportState to get the HTTP/2 stream id.
2019-06-17 15:25:39 -07:00
Carl Mastrangelo e5bd7f282c
Revert "Revert "core, netty: add io.perfmark Annotations" (#5853)" (#5884)
This reverts commit 2db3abc9ad.
2019-06-14 14:09:05 -07:00
Nick Travers 6aed34231f netty: refine filtering for benign transport level exceptions
Transport level exceptions (e.g. "Connection reset by peer") are not
useful and clutter the logs. `NettyServerTransport` contains logic to
log such exceptions at level `FINE`.

When running with epoll, transport level exceptions are prefixed with
additional contextual information (e.g. "syscall:read(..) failed:") that
causes the exceptions to be logged at level `INFO`.

Update the filtering logic to match on error messages _containing_ the
blacklisted messages, rather than using string equality.

Closes #5872.

Signed-off-by: Nick Travers <n.e.travers@gmail.com>
2019-06-13 09:24:36 -07:00
Carl Mastrangelo 2db3abc9ad
Revert "core, netty: add io.perfmark Annotations" (#5853)
This causes internal breakage which needs to be resolved before continuing.

This reverts commit 71967622d6.
2019-06-07 17:23:49 -07:00
Carl Mastrangelo 71967622d6
core, netty: add io.perfmark Annotations
This add perfmark annotations in some key places, notably on transport/application boundaries, and thread hop locations. Perfmark records to a thread-local buffer the events that happen in each thread. Perfmark is disabled by default, and will compile to a noop unless Perfmark.setEnabled is invoked. This should make it free when disable, and pretty fast when it is enabled.

It is important that started tasks are ended, so several places in our code are moved to either try-finally blocks, or moved into a private method. I realize this is ugly, but I think it is manageable. In the future, we can look at making an agent or compiler plugin that simplifies the recording.

Linking between threads is done with a Link object, which is created on the "outbound" task, and used on the "inbound" task. This is slightly more verbose, and does has a small amount of runtime overhead, even when disabled. (for null checks, slightly higher memory usage, etc.) I think this is okay to, because it makes other optimizations much easier.
2019-06-06 17:58:49 -07:00
Carl Mastrangelo 8536832232
core,netty: expose server stream id 2019-06-06 13:52:22 -07:00
Carl Mastrangelo 7657523b28
all: update to error prone 2.3.3 2019-06-05 15:28:43 -07:00
Carl Mastrangelo 4e5e19f6fd
Revert "netty: change server to new protocol negotiator model" (#5798)
This reverts commit f94b77c87f.
2019-05-28 21:12:25 -07:00
Eric Anderson 2cbc540cb3 Remove deprecated enableKeepAlive API from transports
As mentioned in 5188[1], the default used with the enableKeepAlive API
conflicted with the default server enforcement. Instead of fixing it,
remove it. These APIs were deprecated in v1.3.0 in April 2017.

1. https://github.com/grpc/grpc-java/issues/5188#issuecomment-482269303
2019-05-28 16:37:02 -07:00
Carl Mastrangelo 7834a50525
alts: use new ProtocolNegotiator style for ALTS
This change does a few core things, which result in a lot of churn in other parts.  It's not as bad as it seems.

Core things:

1.  AltsProtocolNegotiator is now a shim class, same as ProtocolNegotiators
2.  The protocol negotiators are now in the new style, where there is at most 1 negotiation handler in the pipe at a time.
3.  TsiHandshakeHandler is rewritten with respect to the above.  All errors and buffering are handled by the WBAEH.
4.  TsiFrameHandler is only installed once the negotiation is successful, eliminating the state handling.


The churn in mainly in GoogleDefaultChannel and the GCE channel, which now reuse the *handlers* rather than the negotiators.  This makes it significantly easier to reason about the pipeline state.  The tests are also a source of churn, which no long need to check for most buffering and error conditions.
2019-05-22 16:33:07 -07:00
Carl Mastrangelo f94b77c87f
netty: change server to new protocol negotiator model
Changes:

* PlaintextProtocolNegotiator is the same between client and server
* ServerTlsHandler is rewritten to not handle errors 
  * Also, it now sets the security level attribute, which I don't think it did previously
* NettyServerTransport now uses WBAEH, similar to the client. I don't think the buffer is needed, but it does  correctly handle errors during the startup
2019-05-16 18:15:47 -07:00
Carl Mastrangelo ae4d7a944a
netty: provide methods for logging protocol negotiation 2019-05-10 18:22:32 -07:00
NickUfer 6807f39155 Removes the class `io.grpc.internal.MoreThrowables` and replaces it with
`com.google.common.base.Throwables`
2019-05-02 14:23:09 -07:00
Jihun Cho f4fb7b40c5
netty: set NettyServer default socket options for all Channels (#5651) 2019-05-01 11:29:03 -07:00
Jihun Cho 6a32c508b8
netty: fix TCP_USER_TIMEOUT to use keepAliveTimeout instead of keepAliveTime (#5645) 2019-04-26 16:02:58 -07:00
Jihun Cho 67ef8c3466
netty: not using reflection to create NioEventLoopGroup (#5630) 2019-04-23 16:45:19 -07:00
Carl Mastrangelo 04e07034f3
all: update to truth 0.44 2019-04-23 10:50:49 -07:00
Jihun Cho dc0171839a
netty: add internal API to fall back to NIO transport (#5611) 2019-04-18 13:49:55 -07:00
Jihun Cho 2cdaac2adc
netty: maybe set TCP_USER_TIMEOUT when epoll and keepalive is enabled (#5599) 2019-04-18 10:12:36 -07:00
Eric Anderson 80c3c992a6 core: Move io.grpc to grpc-api
io.grpc has fewer dependencies than io.grpc.internal. Moving it to a
separate artifact lets users use the API without bringing in the deps.
If the library has an optional dependency on grpc, that can be quite
convenient.

We now version-pin both grpc-api and grpc-core, since both contain
internal APIs.

I had to change a few tests in grpc-api to avoid FakeClock. Moving
FakeClock to grpc-api was difficult because it uses
io.grpc.internal.TimeProvider, which can't be moved since it is a
production class. Having grpc-api's tests depend on grpc-core's test
classes would be weird and cause a circular dependincy. Having
grpc-api's tests depend on grpc-core is likely possible, but weird and
fairly unnecessary at this point. So instead I rewrote the tests to
avoid FakeClock.

Fixes #1447
2019-04-16 21:45:40 -07:00
Carl Mastrangelo f3731eabb3
netty: deflake ping flow control logic 2019-04-16 13:17:59 -07:00
Jihun Cho a48ebb1616
netty: change default transport to Epoll if available, otherwise using Nio (#5581)
Motivation:
To support TCP_USER_TIMEOUT(proposal). Nio doesn't support TCP_USER_TIMEOUT while Epoll and KQueue supports TCP_USER_TIME. Since most users/servers are using linux based system, adding Epoll is necessary. KQueue maybe supported later, but not in this PR.

To make it backward compatible for cases where channelType and eventLoop is mixed in with default and user provided object(s), we will fallback to Nio (NioSocketChannel, NioEventLoop). This ensures not breaking existing code (same as existing behavior). Users not specified both channelType and EventLoops will be affect by this Epoll change if netty-epoll is available.
In later version (possibly 1.22.0), the backward compatible behavior will be removed and it will start to throw exception since this is error prone.
2019-04-15 17:53:14 -07:00
Carl Mastrangelo 21141cc837
netty: make default number of event loops defer to netty 2019-04-12 12:42:18 -07:00
Jihun Cho 3b8088833c
netty, alts: expose ProtectedPromise, and writeBufferedAndRemove methods (#5542) 2019-04-04 19:01:54 -07:00
Eric Anderson 73bade418b netty: ALPN negotiation failure should be UNAVAILABLE, not UNKNOWN 2019-04-04 15:33:06 -07:00
Tim van der Lippe d35fbd7eee all: Update to Mockito 2
This is the public port of cl/238445847

Fixes #5319
2019-03-19 14:17:52 -07:00
Fabio Kung 7df2d5feeb netty: default grace time for RPCs can leak memory
Using Long.MAX_VALUE as the delay for Netty's NioEventLoop#schedule can
cause (long) deadlines for scheduled tasks to wrap into negative values,
which is unspecified behavior. Recent versions of netty are guarding
against overflows, but not all versions of grpc-java are using a recent
enough netty.

When connections are gracefully closed, Netty's Http2ConnectionHandler
sets up a timeout task forcing resources to be freed after a grace
period. When their deadlines wrap into negative values, a race was
observed in production systems (with grpc-java <= 1.12.1) causing Netty
to never properly release buffers associated with active streams in the
connection being (gracefully) closed.
2019-03-18 12:38:54 -07:00
Carl Mastrangelo 5cc71f1de9
netty, core: pass log-only channel logger into transport 2019-03-06 11:49:42 -08:00
Carl Mastrangelo 07d7b99e31
netty: expose methods of ProtocolNegotiationEvent through accessor 2019-03-06 10:59:41 -08:00
Carl Mastrangelo d833767037
netty: expose some of the reusable handlers in ProtocolNegotiators 2019-03-05 11:29:32 -08:00