Commit Graph

1461 Commits

Author SHA1 Message Date
Carl Mastrangelo a395eec4a3
core: update LB and NR API names
Updates #1770
2019-04-17 12:45:29 -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
Jihun Cho 71f32bb700
core: fix typo in comment (#5597) 2019-04-15 17:29:45 -07:00
Chengyuan Zhang 636161712d
core/util: create a ForwardingClientStreamTracer class for delegation use (#5589)
* core/util: create a ForwardingClientStreamTracer class for delegation use

* add ExperimentalApi annotation
2019-04-12 18:06:17 -07:00
Kun Zhang 62b03fd7e6
core: pass Subchannel state updates to SubchannelStateListener rather than LoadBalancer (#5503)
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`.
2019-04-12 10:58:09 -07:00
Kun Zhang 0244418d2d
core: Move ConfigOrError up level up. (#5578)
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.
2019-04-10 16:28:23 -07:00
Kun Zhang 880cfd09cd
core: make the NameResolver.start() change backward compatible for forwarding callers. (#5561)
This will give time for pre-existing external callers (e.g.,
forwarding NameResolvers) to migrate to the new start().
2019-04-08 16:09:31 -07:00
Kun Zhang a157052117
core: make the LoadBalancer.handleResolvedAddressGroups() change backward compatible. (#5563)
This will give time for pre-existing external callers (e.g.,
forwarding LoadBalancers) to migrate to the new handleResolvedAddresses().
2019-04-08 15:34:52 -07:00
Kun Zhang 1e901d3b8c
core: make newNameResolver() change backward compatible for callers. (#5560)
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
2019-04-08 12:02:54 -07:00
Carl Mastrangelo 483697f754
core: change service config to ConfigOrError
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.
2019-04-08 11:06:13 -07:00
Chengyuan Zhang 9a2ef07b71
core: use FakeClock.ScheduledExecutorService for KeepAliveManagerTest (#5501)
* core: use FakeClock.ScheduledExecutorService for KeepAliveManagerTest

* add annotation for accessing stopwatch in synchronized section only
2019-04-05 12:49:04 -07:00
ZHANG Dapeng 1c276a823b
core: refactor lookUpServiceConfig(boolean) to disableServiceConfigLookUp()
Refactor as discussed in #5189. Will backport to v1.20.0 to reduce a cycle of deprecation process.
2019-04-04 14:01:19 -07:00
Carl Mastrangelo faf6ff9f98
core: add client and server call perfmark annotations
This code is highly experimental, so we can change it at will later. This PR is to go ahead and try it out.

Perfmark task calls are added to the client and server calls public APIs, recording when the calls begin and end. They use separate scope IDs (roughly these are Thread IDs) for the listener and the call due to no synchronization between them. However, they both use the same PerfTags, which allows them to be associated.

In the future, we can plumb the tag down into the stream to include deeper information about whats going on in a call.
2019-04-03 18:16:22 -07:00
Carl Mastrangelo d8b5d84ce7
perfmark: add a perf annotation package 2019-03-29 11:29:36 -07:00
Carl Mastrangelo ecccdbb3b0
core: update since javadoc, and add toBuilder for NameResolver results 2019-03-29 10:43:08 -07:00
Carl Mastrangelo 17d67f17fa
all: add LoadBalancer overload for Resolution results 2019-03-29 09:31:24 -07:00
Kun Zhang 026e4c53bd
Start 1.21.0 development cycle (#5515) 2019-03-28 13:27:45 -07:00
Carl Mastrangelo 5ef8377efa
core: remove Type from ConfigOrError 2019-03-28 09:40:50 -07:00
Nick Hill 8e41f6e43b core: Avoid locks in SynchronizationContext (#5504)
Similar to what's done in SerializingExecutor, it's easy to make SynchronizationContext non-blocking.
2019-03-26 13:54:01 -07:00
Carl Mastrangelo 53f4ad21b4
core: fix DNS JNDI not working if there is an unavailability cause 2019-03-25 16:10:13 -07:00
Carl Mastrangelo f4a31ec62d
core: deprecate NR.L and add NR.Results 2019-03-22 16:38:28 -07:00
ZHANG Dapeng a2cda8d15d
all: fix lint 2019-03-20 09:01:25 -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
Carl Mastrangelo c6b505229c
all: move LB parsing logic into LB.Factory 2019-03-19 13:33:13 -07:00
Carl Mastrangelo 6b4a3796a6
core: change DNS error handling to fail on all invalid configs 2019-03-19 09:22:02 -07:00
ZHANG Dapeng 1735adc4c0
core: channelBuilder.defaulServiceConfig() and lookUpServiceConfig() 2019-03-14 16:57:16 -07:00
ZHANG Dapeng 283f04983b
core: reorder code in NameResolver.Listener.onAddress() 2019-03-13 16:54:10 -07:00
ZHANG Dapeng bd77e886d5
doc: s/<string>/<strong>/ in javadoc 2019-03-12 13:46:13 -07:00
Eric Anderson d7e53e871b
Merge pull request #5454 from ejona86/protobuf-3.7.0
Upgrade to Protobuf 3.7.0
2019-03-11 15:39:50 -06:00
Eric Anderson 57e94d19ea core: Document grpc does not perform compression nego on client 2019-03-11 15:36:21 -06:00
Eric Anderson 656dcc1c7c core,stub: Document grpc performs compression nego on server 2019-03-11 15:36:21 -06:00
ZHANG Dapeng 97ff7fe50a
core: fix thread safety in NameResolver.Listener 2019-03-11 14:35:49 -07:00
Jihun Cho 5ba6619ce5
all: fix lint error (#5464) 2019-03-11 13:58:37 -07:00
Jihun Cho b6d7f6e84f
all: fix lint errors (#5462) 2019-03-11 10:52:12 -07:00
Carl Mastrangelo 0c23735cfc
core: separate service config parsing and add NR.Helper method 2019-03-10 15:36:34 -07:00
Kun Zhang f095926d2c
core: stop catching exceptions from NameResolver.start() (#5452)
It's not supposed to throw. Any exception from it should be considered
a bug and deserves a panic.
2019-03-08 15:21:36 -08:00
Carl Mastrangelo e5e01b5169
core,grpclb: use better generics on service config 2019-03-08 14:11:13 -08:00
Carl Mastrangelo 6b0325c84f
core: normalize log statement callsite
Make the "logged" method be consistent, and refer to the public logging class name and method.  This makes the log statements return the same class name used to set the log level.  

Before:

```
190306 13:29:39.290:D 1 [io.grpc.internal.ChannelTracer.logOnly] [Channel<1>: (localhost:10000)] Channel for 'localhost:10000' created
190306 13:29:39.414:D 1 [io.grpc.internal.ChannelTracer.logOnly] [Channel<1>: (localhost:10000)] Exiting idle mode
190306 13:29:39.622:D 17 [io.grpc.internal.ChannelTracer.logOnly] [Channel<1>: (localhost:10000)] Resolved address: [[[/127.0.0.1:10000]/{}]], config={}
190306 13:29:39.623:D 17 [io.grpc.internal.ChannelTracer.logOnly] [Channel<1>: (localhost:10000)] Address resolved: [[[/127.0.0.1:10000]/{}]]
190306 13:29:39.624:D 17 [io.grpc.internal.ChannelTracer.logOnly] [Subchannel<3>] Subchannel for [[[/127.0.0.1:10000]/{}]] created
```

After:

```
190306 13:49:15.654:D 1 [io.grpc.ChannelLogger.log] [Channel<1>: (localhost:10000)] Channel for 'localhost:10000' created
190306 13:49:15.772:D 1 [io.grpc.ChannelLogger.log] [Channel<1>: (localhost:10000)] Exiting idle mode
190306 13:49:15.995:D 18 [io.grpc.ChannelLogger.log] [Channel<1>: (localhost:10000)] Resolved address: [[[/127.0.0.1:10000]/{}]], config={}
190306 13:49:15.995:D 18 [io.grpc.ChannelLogger.log] [Channel<1>: (localhost:10000)] Address resolved: [[[/127.0.0.1:10000]/{}]]
190306 13:49:15.997:D 18 [io.grpc.ChannelLogger.log] [Subchannel<3>] Subchannel for [[[/127.0.0.1:10000]/{}]] created
190306 13:49:15.999:D 18 [io.grpc.ChannelLogger.log] [Channel<1>: (localhost:10000)] Child Subchannel created
```
2019-03-06 15:29:46 -08:00
Carl Mastrangelo 5cc71f1de9
netty, core: pass log-only channel logger into transport 2019-03-06 11:49:42 -08:00
Kun Zhang b0bbd7537b
core/grpclb: propagation and parsing of grpclb config (#5419)
Make sure the config for grpclb is passed to the GrpclbLoadBalancer, which will support two child policies -- "round_robin" (default) and "pick_first".

Previously the presence of balancer addresses would dictate "grpclb" policy, despite of the service config. Service config will now take precedence instead.

Implement config parsing logic in GrpclbLoadBalancer. Per offline discussions with @markdroth and @ejona86, we will ignore configuration errors for now. The more appropriate config error handling is upcoming.
2019-03-05 15:39:48 -08:00
ZHANG Dapeng b7dd92e7bb
core: suppress android lint error for javax.naming.*
Resolves #5422
2019-03-05 14:46:38 -08:00
Carl Mastrangelo 801cc5c189
core,netty,okhttp: propagate the subchannel logger to the transport 2019-03-04 15:16:53 -08:00
Kun Zhang 2336eb6f57
test: test the header mutation inside newClientStreamTracer() (#5421)
newClientStreamTracer() is called from transport implementations, thus
needs to be tested in AbstractTransportTest.
2019-03-04 13:53:03 -08:00
Kun Zhang a15a3117de
core: deprecate LoadBalancer.Helper#getNameResolverFactory (#5418)
This was added for the potential use case of needing to resolve target
names (of the same scheme as the top-level channel's target's) in the
LoadBalancer.  Now actual use cases come up in xDS that we need to
resolve fully-qualified target strings with arbitrary schemes.  This
method has never been used and won't fit future uses because it's too
restrictive.
2019-03-04 13:37:49 -08:00
Kun Zhang 59a336c3ae
core: add internal transport attribute ATTR_CLIENT_EAG_ATTRS (#5420)
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.
2019-03-04 12:38:26 -08:00
Kun Zhang 0e7b8fd9ef
core: add LoadBalancer.Helper#createResolvingOobChannel() (#5415)
This can be used by xds LoadBalancer to create a channel to the XDS
traffic director, as the service config will only specify the target
name of the balancer.

This PR only adds the interface to unblock the xds work.
Implementation would take some time thus will come later.
2019-03-04 09:42:17 -08:00
Kun Zhang 02f55189aa
core: refactor load-balancing config handling (#5397)
The LoadBalancingConfig message, which looks like
```json
{
  "policy_name" : {
    "config_key1" : "config_value1",
    "config_key2" : "config_value2"
   }
}
```
appears multiple times. It gets super tedious and confusing to handle, because both the whole config and the value (in the above example is `{ "config_key1" : "config_value1" }`) are just `Map<String, Object>`, and each user needs to do the following validation:
 1. The whole config must have exactly one key
 2. The value must be a map

Here I define `LbConfig` that holds the policy name and the config value, and a method in `ServiceConfigUtil` that converts the parsed JSON format into `LbConfig`.

There is also multiple cases where you need to handle a list of configs (top-level balancing policy, child and fallback policies in xds, grpclb child policies). I also made another helper method in `ServiceConfigUtil` to convert them into `List<LbConfig>`.

Found and fixed a bug in the xds code, where the top-level balancer should pass the config value (excluding the policy name), not the whole config to the child balancers. Search for "supported_1_option" in the diff to see it in the tests.
2019-03-01 19:05:33 -08:00
Kun Zhang 8806403b3a
core: remove unnecessary private method (#5417) 2019-03-01 17:28:30 -08:00
Eric Anderson a818c81a71 testing: Move AbstractTransportTest to core to avoid Truth dep
Fixes #5301
2019-02-28 21:33:19 -07:00
Carl Mastrangelo c5d2d483e2
all: try out mockito rule 2019-02-27 16:40:47 -08:00