This should avoid messages being leaked when a Listener throws an exception and
the executor is shut down immediately after the call completes. This is related
to #7105 but a different scenario and we aren't aware of any user having
observed the previous behavior.
Note also this does _not_ fix the similar case of reordering caused by
delayedCancelOnDeadlineExceeded().
It has been our intention for years to remove nameResolverFactory. We should
make it clear to users to avoid new code depending on it and so they can tell
us why they need it so we can provide replacements.
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.
This fixes#6817 for the normal retry case, although it makes the hedging issue #7089 more broken, and there is still space of optimization for normal retry.
The original service config error handling design was unclear about the case when an updated resolution result with valid service config and empty address is returned to Channel, and the selected LB policy does not accept empty addresses. Existing implementation silently triggers resolver backoff after LB policy is changed, while leaving Channel being CONNECTING state, if there was an old service config resolved. This change converge the behaviors of whether the received service config is the first one or an update to a previous config, in both cases, Channel goes into TRANSIENT_FAILURE if the selected LB policy cannot handle empty addresses.
- Use gradle configuration `api` for dependencies that are part of grpc public api signatures.
- Replace deprecated gradle configurations `compile`, `testCompile`, `runtime` and `testRuntime`.
- With minimal change in dependencies: If we need dep X and Y to compile our code, and if X transitively depends on Y, then our build would still pass even if we only include X as `compile`/`implementation` dependency for our project. Ideally we should include both X and Y explicitly as `implementation` dependency for our project, but in this PR we don't add the missing Y if it is previously missing.
Define util function to exclude guava's transitive dependencies jsr305 and animal-sniffer-annotations, and always manually add them as runtimeOnly dependency. error_prone_annotations is an exception: It is also excluded but manually added not as runtimeOnly. It must always compile with guava, otherwise users will see warning spams if guava is in the compile classpath but error_prone_annotations is not.
Eliminate the hack of InternalNotifyOnBuild mechanism for letting ProtoReflectionService get access to the Sever instance, which makes ProtoReflectionService incompatible with server interceptors. This change put the Server instance into the Context and let the ProtoReflectionService RPC obtain it in its RPC Context. Also enhanced ProtoReflectionService so that one service instance can be used across multiple servers.
A user has been seeing "InternalSubchannel closed transport due to address
change" errors (b/153064566). It is unclear if they are predomenent, but they
are at least adding noise. Since #2562 is still far from being generally
solved, we delay the shutdown a while to side-step the race.
Make each subchannel created by RR stay in TRANSIENT_FAILURE state until READY. That is, each subchannel ignores consequent non-READY states after TRANSIENT_FAILURE.
Previously AbstractServerStream would throw an exception which would kill the
RPC with a RST_STREAM. Now the server actually responds with a clean error
message and avoids spamming the logs.
WARNING: Exception in onHeadersRead()
java.lang.IllegalStateException
at com.google.common.base.Preconditions.checkState(Preconditions.java:495)
at io.grpc.internal.AbstractStream$TransportState.onStreamAllocated(AbstractStream.java:232)
at io.grpc.internal.AbstractServerStream$TransportState.onStreamAllocated(AbstractServerStream.java:224)
at io.grpc.netty.NettyServerHandler.onHeadersRead(NettyServerHandler.java:451)
at io.grpc.netty.NettyServerHandler.access$900(NettyServerHandler.java:101)
at io.grpc.netty.NettyServerHandler$FrameListener.onHeadersRead(NettyServerHandler.java:807)
at io.netty.handler.codec.http2.DefaultHttp2ConnectionDecoder$FrameReadListener.onHeadersRead(DefaultHttp2ConnectionDecoder.java:373)
The condition "effectiveServiceConfig != validServiceConfig" should
have been deleted in commit 2162ad0436.
The condition was there before that commit because
NAME_RESOLVER_SERVICE_CONFIG was already in "attrs", thus it needed to
be re-added only if "effectiveServiceConfig" differs from the original
"validServiceConfig".
In contrast, ATTR_HEALTH_CHECKING_CONFIG is not in the original
"attrs" and always needs to be added.
This allows an InProcessTransport instance to be created via a new
internal accessor class InternalInProcess. We effectively just expose a
method to create an InProcessTransport with a existing ServerListener
instance.
This will be used for in-process channels to an under-development
on-device server.
Move ATTR_LB_ADDR_AUTHORITY and ATTR_LB_PROVIDED_BACKEND attributes definition in GrpcAttributes to GrpclbConstants. grpc-alts will have a compile dependency on grpc-grpclb.
Eliminated the code path of resolving Grpclb balancer addresses in grpc-core and moved it into GrpclbNameResolver, which is a subclass of DnsNameResolver. Main changes:
- Slightly changed ResourceResolver and its JNDI implementation. ResourceResolver#resolveSrv(String) returns a list of SrvRecord so that it only parse SRV records and does nothing more. It's gRPC's name resolver's logic to use information parsed from SRV records.
- Created a GrpclbNameResolver class that extends DnsNameResolver. Logic of using information from SRV records to set balancer addresses as ResolutionResult attributes is implemented in GrpclbNameResolver only.
- Refactored DnsNameResolver, mainly the resolveAll(...) method. Logics for resolving backend addresses and service config are modularized into resolveAddresses() and resolveServiceConfig() methods respectively. They are shared implementation for subclasses (i.e., GrpclbNameResolver).
Add "envoy.lb.does_not_support_overprovisioning" to xDS node identifier client features. Set the new user_agent_name and user_agent_version fields for build version.
First take for grpclb selection stabilization:
1. Changed DnsNameResolver to return balancer addresses as a GrpcAttributes.ATTR_LB_ADDRS attribute in ResolutionResult, instead of among the addresses.
2. AutoConfiguredLoadBalancerFactory decides LB policy solely based on parsed service config without looking at resolved addresses. Behavior changes:
- If no LB policy is specified in service config, default to pick_first, even if there exist balancer addresses (in attributes).
- If grpclb specified but not available and no other specified policies available, it will fail without fallback to round_robin.
3. GrpclbLoadBalancer populates balancer addresses from ResolvedAddresses's attribute (GrpclbConstants.ATTR_LB_ADDRS) instead of sieving from addresses.
ZHANG Dapeng
Eric Gribkoff
Eric Anderson
Jihun Cho
Anar Sultanov
Elliotte Rusty Harold
Chengyuan Zhang
Reginald McDonald
Kun Zhang
markb74