Both Netty and OkHttp had local and remote windows flipped. Also, the
comment in OkHttp about "not tracked" wasn't true, so make it more
accurate and provide a _hint_ of what the window may be instead of just
-1.
* api: add support for SocketAddress types in ManagedChannelProvider
also add support for SocketAddress types in NameResolverProvider
Use scheme in target URI to select a NameRseolverProvider and get
that provider's supported SocketAddress types.
implement selection in ManagedChannelRegistry of appropriate
ManagedChannelProvider based on NameResolver's SocketAddress types
Users should be able to inject all executors. The transport shouldn't be
hard-coded to create the TIMER_SERVICE, especially since a scheduler is
already available to the builder.
This matches what we do in ManagedChannelImplBuilder and
NettyChannelBuilder. It also fixes a (probably unimportant) bug where
the factory returned from swapChannelCredentials() didn't have its
references to the executors so could not outlive the parent factory.
In the olden days, before LB policies, transports had to accept RPCs as
soon as they were created. This hasn't been true for a very long time,
so remove the tests.
Since a978c9ed we're using real, legit code flows in the tests. This
allowed TSAN to discover that `attributes` is racy when read when
creating a new stream before the transport is ready. We could use a lock
or volatile, but the value of the attributes would still be incorrect
for any RPCs that are created before the transport is ready.
Since there's now only one test that delays the connection, I inline the
support code.
This greatly reduces the number of arguments passed to the constructor
and allows using the builder in tests to change specific arguments
without having to pass all the other arguments. It also makes it easier
to see where tests are doing something special.
While it is weird to expose fields as package-private for digging-into
in the constructor, it's actually very similar to the pattern of passing
the builder instance into the constuctor. In this case, the weirdness is
because the builder isn't a nested class of the transport and there is
an additional level of building going on (Builder and TransportFactory).
We do this pattern already in ManagedChannelImpl which only has the one
level of building.
With the completely different constructor it was hard to track which
fields were different during the test and reduced confidence. Now the
test code flows are much closer to the real-life code flows.
* okhttp: forked required files to make okhttp dep compile only
* okhttp: forked missing file to make okhttp dep compile only
* okhttp: moved url and request files to proxy packge
* okhttp: removed unused methods from forked files; fixed build
This introduces new TLS 1.2 cipher suites (#8610) and prepares the
internal okhttp implementation for TLS1.3. A new method for creating
internal ConnectionSpec was added to be able to use the newly introduced
cipher suites in the OkHttpChannelBuilder. Okhttp cipher suites
synchronized with the ones from netty.
In refactoring described in #7211, the implementation of #maxInboundMessageSize(int)
(and its corresponding field) were pulled down from internal AbstractManagedChannelImplBuilder
to concrete classes that actually enforce this setting. For the same reason, it wasn't ported
to ManagedChannelImplBuilder (the #delegate()).
Then AbstractManagedChannelImplBuilder was brought back to fix ABI backward compatibility,
and temporarily turned into a ForwardingChannelBuilder, ref PR #7564. Eventually it will
be deleted, after a period with "bridge" ABI solution introduced in #7834.
However, restoring AbstractManagedChannelImplBuilder unintentionally made ABI of
pre-refactoring builds expect it to be a method of AbstractManagedChannelImplBuilder,
and not concrete classes, ref #8313.
The end goal is to keep #maxInboundMessageSize(int) only in concrete classes that enforce it.
To fix method's ABI, we temporary reintroduce it to the original layer it was removed from:
AbstractManagedChannelImplBuilder. This class' only intention is to provide short-term
ABI compatibility. Once we move forward with dropping the ABI, both fixes are no longer
necessary, and both will perish with removing AbstractManagedChannelImplBuilder.
Rebased PR #8343 into the first commit of this PR, then (the 2nd commit) reverted the part for metric recording of retry attempts. The PR as a whole is mechanical refactoring. No behavior change (except that some of the old code path when tracer is created is moved into the new method `streamCreated()`).
The API change is documented in go/grpc-stats-api-change-for-retry-java
We used to have two ClientStreamListener.closed() methods. One is simply calling the other with default arg. This doubles debugging (e.g. #7921) and sometimes unit testing work. Deleting the 2-arg method to cleanup.
This PR is purely refactoring.
Enables a codepath for zero-copy protobuf deserialization. Two new InputStream extension interfaces are added:
- HasByteBuffer: allows access to the underlying buffers containing inbound bytes directly without copying
- Detachable: allows customer marshaller to keep the buffers around until the application code is done with using the protobuf messages
Applications can implement a custom marshaller that takes over the ownership of ByteBuffers and wrap them into ByteStrings with protobuf's UnsafeByteOperations support. Then a RopeByteString, which is a in-place composite of ByteStrings can be created. This enables using the zero-copy codepath (requires immutable ByteBuffer indication) of CodedInputStream for deserialization.
`OkHttpClientTransport.ClientFrameHandler` will fail a stream with `Status.UNAVAILABLE.withDescription("End of stream or IOException")` when socket is closed with an error. However, it does not include any more error detail. This PR provides more error detail in case there is an existing goaway status, e.g. netty server can send goaway with lastKnownStreamId=MAX_INT when header size exceeded max allowed size netty/netty/pull/10775 and shutdown the connection.
Test: `io.grpc.okhttp.OkHttpTransportTest.serverChecksInboundMetadataSize` with `netty-4.1.54.Final`
Fixes#8080. The address 0.0.0.0 (that comes from new Socket(0).
.getLocalSocketAddress()) is for listening with a server, but it
is not meant to be used as the destination address as per
"3.2.1.3 Addressing" in RFC 1122
okio 2.x is ABI compatible with 1.x but not API compatible. This hasn't
been a problem as users use binaries from Maven Central so the ABI
compatibility is the important part. However, when building with Bazel
the API compatibily is the important part.
Tested with okio 2.10.0
Fixes#8004
- Add APIs to `ClientTransportFactory`:
```java
public interface ClientTransportFactory {
/**
* Swaps to a new ChannelCredentials with all other settings unchanged. Returns null if the
* ChannelCredentials is not supported by the current ClientTransportFactory settings.
*/
SwapChannelCredentialsResult swapChannelCredentials(ChannelCredentials channelCreds);
final class SwapChannelCredentialsResult {
final ClientTransportFactory transportFactory;
@Nullable final CallCredentials callCredentials;
}
}
```
- Add `ChannelCredentials` to constructor args of `ManagedChannelImplBuilder`:
```java
public ManagedChannelImplBuilder(
String target, @Nullable ChannelCredentials channelCreds, @Nullable CallCredentials callCreds, ...)
```
Change InternalServer to handle multiple addresses and implemented in NettyServer.
It makes ServerImpl to have a single transport server, and this single transport server (NettyServer) will bind to all listening addresses during bootstrap. (#7674)
API change (See go/grpc-rls-callcreds-to-server):
- Add `ChannelCredentials.withoutBearerTokens()`
- Add `createResolvingOobChannelBuilder(String, ChannelCredentials)`, `getChannelCredentials()` and `getUnsafeChannelCredentials()` for `LoadBalancer.Helper`
This PR does not include the implementation of `createResolvingOobChannelBuilder(String, ChannelCredentials)`.
I didn't touch Protobuf and Netty; we upgrade those individually. Below
are issues I encountered that caused me to not upgrade (further).
Guava 30.1-android fails to build with Android without enabling
desugaring. https://github.com/google/guava/issues/5358
Robolectric 4.4 breaks AndroidChannelBuilderTest.
https://github.com/grpc/grpc-java/issues/7731
Opencensus 0.28.1+ is incompatible with gRPC.
https://github.com/census-instrumentation/opencensus-java/issues/2069https://github.com/grpc/grpc-java/issues/7732
Truth now defines the asm dependency as "compile" although it is still
optional. But asm appears to have accidentally included incorrect gradle
module metadata in their release (I see they've disabled the metadata on
master) which make gradle think it requires Java 8. We could asm
everywhere, but that's is annoying. It seems likely this will resolve
itself.
Mockito can be upgraded to 3.4.0, but it deprecates initMocks, which
causes more code churn than I wanted in this commit. I still
synchronized the example versions on 3.4.0, though, as it was already
being used in some examples and the examples don't use initMocks.
* fix channel builders ABI backward compatibility broken in v1.33.0
* fix server builders ABI backward compatibility broken in v1.33.0
* makes ForwardingServerBuilder package-private
It deprecates ExpectedException and Assert.assertThat(T, org.hamcrest.Matcher).
Without Java 8 we don't want to migrate away from ExpectedException at
this time. We tend to prefer Truth over Hamcrest, so I swapped the one
instance of Assert.assertThat() to use Truth. With this change we get a
warning-less build with JUnit 4.13. We don't yet upgrade because we
still need to support JUnit 4.12 for some use-cases, but will be able to
upgrade to 4.13 soon when they upgrade.
verifyZeroInteractions has the same behavior as verifyNoMoreInteractions. It
was deprecated in Mockito 3.0.1 and replaced with verifyNoInteractions, which
does not change behavior depending on previous verify() calls. All instances
were replaced with verifyNoInteractions, except those in
ApplicationThreadDeframerTest which were replaced with verifyNoMoreInteractions
since there is a verify() call in `@Before`.
This provides a substantial ~3x performance increase to Netty async
streaming with small messages. It also increases OkHttp performance for
the same benchmark 40% and decreases unary latency by 3µs for Netty and
10µs for OkHttp.
We avoid calling listener after closure because the Executor used for
RPC callbacks may no longer be available. This issue was already
present in the ApplicationThreadDeframer, but full-stream compression is
not really deployed so was unnoticed.
DirectExecutor saw a 5-6µs latency increase via MigratingDeframer.
DirectExecutor usages should see no benefit from MigratingDeframer, so
disable it in that case.
* Revert "okhttp: revert changes for using new APIs to configure TLS in Android (#6959)"
This reverts commit ee8b395f79.
Roll forward with adding manual check if hostname contains underscore, as Android's URI implementation allows underscore in hostsname.
* Revert "okhttp: Skip enabling SNI and session ticket for fake/test host names (#6949)"
This reverts commit eb8e31409e.
* Revert "okhttp: use new APIs for configuring TLS whenever possible (Android Q+) (#6912)"
This reverts commit 5803dfd9dc.
Use new APIs for configuring TLS in Android environment. Starting from Android 29, there is a new set of public APIs for configuring ALPN (and starting from Android 24, there is API for enabling SNI). This change migrates to use these new APIs whenever possible. Only fallback to call the old hidden APIs if new ones do not exist (or do not work).
Specifically, this addresses bugs that occur when the `OkHttpChannelBuilder.flowControlWindow(int)` setting is increased from its default value.
Two changes:
1. On starting a connection, ensure the value of `OkHttpChannelBuilder.flowControlWindow(int)` is sent via Settings.INITIAL_WINDOW_SIZE. Also send a WINDOW_UPDATE after Settings to update the connection-level window.
2. Always initialize the `OutboundFlowController` with an initialWindowSize of 65335 bytes per the [http2 spec](https://http2.github.io/http2-spec/#InitialWindowSize) instead of using the inbound window size.
Fixes#6685
This change adds two booleans to the ChannelBuilders to
allow transports to use get and put. These are currently defaulted to
on, but unset on the method descriptors. This change is 1/2 that will
allow the safe / idempotent bits to be set on generated proto code.
Part 2/2 will actually enable it.
The use case for this is for interceptors that implement caching logic.
They need to be able to access the safe/idempotent bits on the MD in
order to decide to how to handle the request, even if gRPC doesn't use
GET / PUT HTTP methods.
* okhttp: add PerfMark tracing for okhttp transport
* change task for AsyncSink to give more accurate task description
* add final to field tag
* add PerfMark dependency in okhttp package bazel
The former is deprecated and replaced by the latter in Mockito 2.
However, there is a functional difference: ArgumentMatchers will reject
`null` and check the type if the matcher specified a type (e.g.
`any(Class)` or `anyInt()`). `any()` will remain to accept anything.
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
* okhttp: add verbose logging for OkHttp HTTP/2 frames to include brief frame content.
* fix small details and typo issues
* fix branch condition (it's a typo) for logging the whole buffer
* remove redudent log level checking, always not log entire buffer
* convert ByteString to String to avoid printing 'hex=' prefix
* add test for OkHttpFrameLogger for inbound http frames
* refactor OkHttpFrameLogger's help method
* add Constructor for testing and missing logHeaders
* add http/2 framelogger test for outbound side
* use ArgumentMatchers instead of Matchers
* replace assertTrue with Truth.assertThat
* assert buffer log with exact length match instead of <=
This is needed for GRPCLB pick_first support, which needs to attach
tokens to headers, and the tokens are per server. In pick_first, all
addresses are in a single Subchannel, thus the LoadBalancer needs to
know which backend is used for a new stream.
This will be a new override. The old override is now deprecated.
In order to pass new information without adding new overrides, I shoved most information
into an object called StreamInfo. The Metadata is left out to draw attention because
it's mutable.
Motivation: this is needed for correctly supporting pick_first in GRPCLB. GRPCLB needs to
add a token to the headers, and the token varies among servers. With round_robin, GRPCLB
create a Subchannel for each server, thus can attach the token when the Subchannel is picked.
To implement pick_first, all server addresses will be put in a single Subchannel, we will
need to add the header in newClientStreamTracer(), by looking up the server address from
the transport attributes and deciding which token to add.
Context: [#4159 (comment)](https://github.com/grpc/grpc-java/issues/4159#issuecomment-415827641)
`Attributes` is appropriate for plumbing optional objects, especially useful for a long plumbing path where components in the middle may not care or see all objects in the container. It's not the case for the `params` on `newNewResolver()`. Both the default port and the proxy detector are guaranteed to be there and the plumbing path is very short. In this case, a first-class object is more appropriate and easier to use.
The `Helper` will also have `getSynchronizationContext()` (#2649) and a method to parse and validate service config. We we also considering merging `Listener` into the `Helper`, to make `NameResolver` match the `LoadBalancer` API.
Combine the public `ProxyParameters` and the internal `ProxySocketAddress` as `HttpConnectProxiedSocketAddress`. The more specific name signifies the type of the proxy we currently support, and makes room for other proxy types (e.g., SOCKS) in the future. The combination simplifies NameResolver implementation.
Introduce `ProxiedSocketAddress` as the base class that is returned by `ProxyDetector`, mainly for clarification and documentation. Added documentation about proxy in general on `ProxyDetector`.
This will reduce the method count, as okhttp's copy of ConnectionSpec will no
longer be retained when ProGuarded. It was deprecated in 25f357699, 10 months
ago.
We've been on newer versions of Guava for a while now; these no longer
do anything.
Reworded the comment for Stopwatch.createUnstarted(), because it is not
safe (it doesn't matter if the method isn't marked Beta; you have to use
Ticker), except for the fact it is only used in our tests.
Optimize OkHttp transport's memory use by getting rid of queuing writes in
AsyncFrameWriter. If any write is pending due to connection issue or by flow
control, AsyncFrameWriter can use at least 8K per each task (task includes
buffer) even if the actual payload is very small. To merge pending writes,
Async mechanism is moved from AsyncFrameWriter to AsyncSink (AsyncSink
is used by okio's FrameWriter). AsyncSink is still relying on okio's buffer to
decide merging writes or not.
Resolves#4860
Following the [spec](https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md) on duplicate header names:
**Custom-Metadata** header order is not guaranteed to be preserved except for values with duplicate header names. Duplicate header names may have their values joined with "," as the delimiter and be considered semantically equivalent. Implementations must split Binary-Headers on "," before decoding the Base64-encoded values.
This will allow enabling Error Prone on JDK 10+ (after
updating the net.ltgt.errorprone plugin), and is also a
prerequisite to that plugin update.
Also remove net.ltgt.apt plugin, as Gradle has native
support for annotationProcessor.
This is a rename of the pre-existing Netty builder method, so aliases
were added to the Netty builders.
Fixes#4050. This API was a minor rename to the pre-existing Netty API,
so has already undergone API review and thus is not ExperimentalApi.
Returns a Channel that allows a LoadBalancer to make auxiliary RPCs on already-established application connections. We need this to implement client-side health-checking (#4932)
See comments on the API for its semantics.
Notable changes:
- Transports are modified to use InUseStateAggregator so that they can exclude RPCs made on Subchannel.asChannel() when reporting in-use state for idle mode.
- OobChannel shares the same Executor as Subchannel.asChannel(). Because the latter is not a ManagedChannel and doesn't have life-cycle, thus can't determine when to return the Executor to a pool, the Executor is now returned only when ManagedChannelImpl is terminated.
This is the first step of smoothly changing the CallCredentials API.
Security level and authority are parameters required to be passed to
applyRequestMetadata(). This change wraps them, along with
MethodDescriptor and the transport attributes to RequestInfo, which is
more clear to the implementers.
ATTR_SECURITY_LEVEL is moved to the internal GrpcAttributes and
annotated as TransportAttr, because transports are required to set it,
but no user is actually reading them from
{Client,Server}Call.getAttributes().
ATTR_AUTHORITY is removed, because no transport is overriding it.
All involved interfaces are changed to abstract classes, as this will
make further API changes smoother.
The CallCredentials name is stabilized, thus we first introduce
CallCredentials2, ask CallCredentials implementations to migrate to
it, while GRPC accepting both at the same time, then replace
CallCredentials with CallCredentials2.
Eric Anderson
sanjaypujare
Terry Wilson
beatrausch
ZHANG Dapeng
Jintao
Sergii Tkachenko
Chengyuan Zhang
Tomo Suzuki
Yifei Zhuang
Jihun Cho
chrisschek
Graeme Morgan
Carl Mastrangelo
Ran
Carl Mastrangelo
康智冬
Dharmesh Jogadia
Tim van der Lippe
Tim van der Lippe
Carl Mastrangelo
Kun Zhang
Eric Gribkoff
Ignacio del Valle Alles
Thomas Broyer