Previously, AutoConfiguredLoadBalancer was also handling it, but it
doesn't trigger retries. By returning true for
canHandleEmptyAddressListFromNameResolution(),
AutoConfiguredLoadBalancer effectively by-passed the empty-result
handling logic in ManagedChannelImpl, thus resolution retries were
never triggered.
This change requires AutoConfiguredLoadBalancer to stop being a
LoadBalancer, for its tryHandleResolvedAddresses(). It doesn't
cause any trouble because AutoConfiguredLoadBalancer has become
less and less like a LoadBalancer during the service config changes.
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
The `method` and `status` shouldn't be propagated in the first place,
but in previous OpenCensus implementation all tags are propagating by
default. Now with the TagMetadata it may make sense to change them to
local tags.
This will be a breaking change to users who depend on the behavior that
these tags propagate through process boundaries.
The old createSubchannel() doesn't require being called from
sync-context, thus its implementation schedules start() on
sync-context because start() has such requirement. However, if a
LoadBalancer call createSubchannel() in sync-context, start() is
queued and only called after the control exits the sync-context. If
createSubchannel() is immediately followed by other Subchannel methods
that requires start(), such as requestConnection(), they will fail.
This will be a very common issue as most LoadBalancers call
createSubchannel() in the sync-context.
This fix splits out the real work of start() into internalStart() which
is called by the old createSubchannel().
Previously PickResult's Subchannel must be the actual implementation
returned from the Channel's Helper, and Channel would cast it to the
implementation class in order to use it. This will be broken if
Subchannel is wrapped in the case of hierarchical LoadBalancers.
getInternalSubchannel() is the guaranteed path for the Channel to get
the InternalSubchannel implementation. It is friendly for wrapping.
Background: #5676
The pick_first policies in core and grpclb previously would call
Subchannel.requestConnection() from data-path. They now will schedule
that call in the sync-context to avoid the warning. They will only
call it for the first pick of each picker, to prevent storming the
sync-context.
This is a revised version of #5503 (62b03fd), which was rolled back in f8d0868. The newer version passes SubchannelStateListener to Subchannel.start() instead of SubchannelCreationArgs, which allows us to remove the Subchannel argument from the listener, which works as a solution for #5676.
LoadBalancers that call the old createSubchannel() will get start() implicitly called with a listener that passes updates to the deprecated LoadBalancer.handleSubchannelState(). Those who call the new createSubchannel() will have to call start() explicitly.
GRPCLB code is still using the old API, because it's a pain to migrate the SubchannelPool to the new API. Since CachedSubchannelHelper is on the way, it's easier to switch to it when it's ready. Keeping
GRPCLB with the old API would also confirm the backward compatibility.
Contrary to #5736, we will still keep the sync-context requirement of
requestConnection(), because it prevents API fragmentation.
PickFirstLoadBalancer is the only known violator. We will fix it on
master, but we don't want to make that change on 1.21.x because the
release is soon. We simply remove the warning in this release so that
users won't be annoyed.
This supersedes #5736
We will require Subchannel.requestConnection() to be called from
sync-context (#5722), but SubchannelPicker.requestConnection() is
currently calling it with the assumption of thread-safety. Actually
SubchannelPicker.requestConnection() is called already from
sync-context by ChannelImpl, it makes more sense to move this method
to LoadBalancer where all other methods are sync-context'ed, rather than
making SubchannelPicker.requestConnection() sync-context'ed and fragmenting
the SubchannelPicker API because pickSubchannel() is thread-safe.
C++ also has the requestConnection() equivalent on their LoadBalancer
interface.
We check for idle mode the first time we try newStream(), but failed to when
newStream races with reprocess(). This would normally be a very rare race,
except when you consider that AbstractChannelBuilder will call
managedChannel.enterIdle() when the network changes.
Fixes#5729
I see more cases of wrapping Helper and Subchannel during the work of
XdsLoadBalancer, we will require that all methods that involve mutable
state to be called from the Synchronization Context. We will start
logging warnings first, and make them throw in a future release.
Helper.createSubchannel() is already doing so. This change adds
warnings to the other eligible methods.
https://github.com/grpc/grpc-java/issues/5015
This follows the sort of changes we've done in the past, where the '2'
implies "version 2." We can end up reclaiming the original name if we
wish in the future.
The main reason for this change is to avoid changing to Observer since
the rest of io.grpc consistently uses Listener.
Also updated CensusModule to use the new helper methods ContextUtils.withValue() instead of directly manipulating the context keys. See census-instrumentation/opencensus-java#1864.
Having two Helpers (the other being LoadBalancer.Helper) is proven to
be confusing, and the one in NameResolver is fundamentally different
from the one from LoadBalancer, as the latter mutates the states while
the former doesn't. Renaming it to Args make it less awkward to
expose it to LoadBalancer, which is needed for creating NameResolvers
for per-locality routing in the XdsLoadBalancer.
As we are now endorsing the wrapping of ClientStreamTracers by
providing ForwardingClientStreamTracer, there is a need for altering
StreamInfo, especially CallOptions before it's passed onto the
delegate. A Builder class and a toBuilder() provides a robust way
to copy the rest of the fields.
This is a breaking change for anybody who creates StreamInfo, which is
unlikely in non-test code, because StreamInfo was added as late as
1.20.0.
NameResolverRegistry takes on all the logic previously in
NameResolverProvider. But it also allows manual registration of
NameResolvers, which is useful when the providers have complex
construction or need objects injected into them.
This also avoids a circular dependency during class loading since
previously loading any Provider searched for all Providers via
ClassLoader since ClassLoader handling was static within the parent
class.
Fixes#5562
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
Resolves#5497
## Motivation
In hierarchical `LoadBalancer`s (e.g., `XdsLoadBalancer`) or wrapped `LoadBalancer`s (e.g., `HealthCheckingLoadBalancerFactory`, the top-level `LoadBalancer` receives `Subchannel` state updates from the Channel impl, and they almost always pass it down to its children `LoadBalancer`s.
Sometimes the children `LoadBalancer`s are not directly created by the parent, thus requires whatever API in the middle to also pass Subchannel state updates, complicating that API. For example, the proposed [`RequestDirector`](https://github.com/grpc/grpc-java/issues/5496#issuecomment-476008051) includes `handleSubchannelState()` solely to plumb state updates to where they are used. We also see this pattern in `HealthCheckingLoadBalancerFactory`, `GrpclbState` and `SubchannelPool`.
Another minor issue is, the parent `LoadBalancer` would need to intercept the `Helper` passed to its children to map Subchannels to the children `LoadBalancer`s, so that it pass updates about relevant Subchannels to the children. Otherwise, a child `LoadBalancer` may be surprised by seeing Subchannel not created by it, and it's not efficient to broadcast Subchannel updates to all children.
## API Proposal
We will pass a `SubchannelStateListener` when creating a `Subchannel` to accept state updates, those updates could be directly passed to where the `Subchannel` is created, skipping the explicit chaining in the middle.
Also define a first-class object `CreateSubchannelArgs` to pass arguments for the reasons below:
1. It may avoid breakages when we add new arguments to `createSubchannel()`. For example, a `LoadBalancer` may wrap `Helper` and intercept `createSubchannel()` in a hierarchical case. It may not be interested in all arguments. Passing a single `CreateSubchannelArgs` will not break the parent `LoadBalancer` if we add new fields later.
2. This also reduces the eventual size of Helper interface, as the convenience `createSubchannel()` that accepts one EAG instead of a List is no longer necessary, since that convenience is moved into `CreateSubchannelArgs`.
```java
interface SubchannelStateListener {
void onSubchannelState(Subchannel subchannel, ConnectivityStateInfo newState);
}
abstract class LoadBalancer.Helper {
abstract Subchannel createSubchannel(CreateSubchannelArgs args);
}
final class CreateSubchannelArgs {
final List<EquivalentAddressGroup> getAddresses();
final Attributes getAttributes();
final SubchannelStateListener getStateListener();
final class Builder () {
...
}
}
```
The new `createSubchannel()` must be called from synchronization context, as a step towards #5015.
## How the new API helps
Most hierarchical `LoadBalancer`s would just let the listener from the child `LoadBalancer`s directly pass through to gRPC core, which is less boilerplate than before.
Without any effort by the parent, each child will only see updates for the Subchannels it has created, which is clearer and more efficient.
If a parent `LoadBalancer` does want to look at or alter the Subchannel state updates for its delegate (like in `HealthCheckingLoadBalancerFactory`), it can still do so in the wrapping `LoadBalancer.Helper` passed to the delegate by intercepting the `SubchannelStateListener`.
## Migration implications
Existing `LoadBalancer` implementations will continue to work, while they will see deprecation warnings when compiled:
- The old `LoadBalancer.Helper#createSubchannel` variants are now deprecated, but will work until they are deleted. They create a `SubchannelStateListener` that delegates to `LoadBalancer#handleSubchannelState`.
- `LoadBalancer#handleSubchannelState` is now deprecated, and will throw if called and the implementation doesn't override it. It will be deleted in a future release.
The migration for most `LoadBalancer` implementation would be moving the logic from `LoadBalancer#handleSubchannelState` into a `SubchannelStateListener`.
This class is used in other places than just NameResolver.Helper. It
should not be an inner class of Helper.
Strictly speaking this is an API-breaking change. However, this is
part of the service config error handling API that hasn't been done
yet. Nobody has a legitimate reason to use it.
We assumed that we were the only caller. Turns out there are
forwarding NameResolver too. Implementing the old override will give
them time to migrate to the new API.
Resolves#5556
Both the config and the error need to be passed to the managed channel so it can decide to keep or reject the config. Passing only the parsed object means the managed channel cannot implement the error handling gRFC.
ZHANG Dapeng
Carl Mastrangelo
Ran
Eric Anderson
Jihun Cho
Kun Zhang
Yang Song
Chengyuan Zhang
NickUfer