If a panic is followed a panic, we'd ignore the second. But if an
exception happens while entering panic mode we may fail to update the
picker with the first error. This is "fine" from a correctness
standpoint; all bets are off when panicking and we've already logged the
first error. But failing RPCs can often be more easily seen than just
the log.
Noticed because of http://yaqs/8493785598685872128
Combined success / error status passed via ResolutionResult to the NameResolver.Listener2 interface's onResult2 method - Addresses in the success case or address resolution error in the failure case now get set in ResolutionResult::addressesOrError by the internal name resolvers.
* Change PickFirstLeafLoadBalancer to only have 1 subchannel at a time if environment variable GRPC_SERIALIZE_RETRIES == true.
Cache serializingRetries value so that it doesn't have to look up the flag every time.
Clear the correct task when READY in processSubchannelState and move the logic to cancelScheduledTasks
Cleanup based on PR review
remove unneeded checks for shutdown.
* Fix previously broken tests
* Shutdown previous subchannel when run off end of index.
* Provide option to disable subchannel retries to let PFLeafLB take control of retries.
* InternalSubchannel internally goes to IDLE when sees TF when reconnect is disabled.
Remove an extra index.increment in LeafLB
* use an attribute from resolved addresses IS_PETIOLE_POLICY to control whether or not health checking is supported so that top level versions can't do any health checking, while those under petiole policies can.
Fixes#11413
Detachable lets a buffer outlive its original lifetime. The new lifetime
is application-controlled. If the application fails to read/close the
stream, then the leak detector wouldn't make clear what code was
responsible for the buffer's lifetime. With this touch, we'll be able to
see detach() was called and thus know the application needs debugging.
Realized when looking at b/364531464, although I think the issue is
unrelated.
It doesn't do anything.
Call scheduleNextConnection() unconditionally since it is responsible
for checking if `enableHappyEyeballs == true`. It's also surprising to
check in the CONNECTING case but not the IDLE case.
It is trivial to avoid the exception from
addressIndex.getCurrentAddress(). The log message was inaccurate, as the
subchannel might have been TRANSIENT_FAILURE. The only important part of
the condition was whether the subchannel was the current subchannel.
It will never throw, because it would only throw if helper is null, but
helper is checkNotNull()ed in the constructor. It could have checked for
a null return value instead; since it hasn't been, it is clear we don't
need this check.
Bazel had the dependency added because of #5046, where Guava was
depending on it as compile-only and Bazel build have "unknown enum
constant" warnings. Guava now has a compile dependency on j2objc, so
this workaround is no longer needed. There are currently no version skew
issues in Gradle, which was the only usage.
This reverts commit 9ba2f9dec5.
It causes a channel panic due to unimplemented onResult2().
```
java.lang.UnsupportedOperationException: Not implemented.
at io.grpc.NameResolver$Listener2.onResult2(NameResolver.java:257)
at io.grpc.internal.DnsNameResolver$Resolve.lambda$run$0(DnsNameResolver.java:334)
at io.grpc.SynchronizationContext.drain(SynchronizationContext.java:94)
at io.grpc.SynchronizationContext.execute(SynchronizationContext.java:126)
at io.grpc.internal.DnsNameResolver$Resolve.run(DnsNameResolver.java:333)
```
b/356669977
Introducing NameResolver listener method "Status Listener2::onResult2(ResolutionResult)" that returns Status of the acceptance of the name resolution by the load balancer, and the Name Resolver will call this method for both success and error cases.
* Eliminate NPE after recovering from a temporary name resolution failure.
* Add test case for 2 failing subchannels to make sure it causes channel to go into TF.
The test was added in e4e7f3a06 when InProcess stopped returning a
Runnable from start(). In c5a63a1 we realized (indirectly) that there's
no point in using the Runnable any more.
This test failed with Binder (which seems to have been using the
Runnable unnecessarily), and InProcess, Netty, and OkHttp don't use the
Runnable. Instead of fixing it, we'll just move toward stopping using
Runnable.
I'm not removing the Runnable usage from Binder in this commit because
this test is currently causing CI failures and I don't want to do a
behavior change when fixing it.
This hasn't been needed since f8f569e07, when InternalSubchannel stopped
calling start() with a lock held. Note that also means no transport
needs to return a Runnable (but some still are).
I had noticed in e4e7f3a06 that it was safe for InProcess to call the
listener directly within start(), but I didn't notice this Javadoc that
said it wasn't allowed.