This removes a needless warning, and isn't much slower. Also this
includes a benchmark for StatsTraceContext to measure the overhead
for creation. It adds about 40ns per RPC. Optimization will come
after structural changes are made to break the dependency on
Census.
Resolves#2716
- Add attributes to EquivalentAddressGroup
- Deprecate ResolvedServerInfoGroup by EquivalentAddressGroup
- Deprecate ResolvedServerInfo, because attributes for a single address
with an address group is not found to be useful.
- The changes on the NameResolver and LoadBalancer interfaces are backward-compatible
in the next release, with which implementors can switch to the new API smoothly.
As a related change, redefine the semantics of DnsNameResolver and
RoundRobinLoadBalancer:
- Before: DnsNameResolver returns all addresses in one address group.
RoundRobinLoadBalancer ignores the grouping of addresses and
round-robin on every single addresses. It doesn't work well with the
one-server-multiple-address setup, e.g., both IPv4 and IPv6 addresses
are returned for a single serve, even if they are put in the same
address group by the NameResolver.
- After: DnsNameResolver returns every address in its own
EAG. RoundRobinLoadBalancer takes an EAG as a whole, and only
round-robin on the list of EAGs. The new behavior is a better
interpretation of the EAGs, and really allows the case where one
server has more than one addresses (e.g., IPv4 and IPv6).
This change will affect users that use custom LoadBalancer with the
stock DnsNameResolver, and those who use custom NameResolver with the
stock RoundRobinLoadBalancer.
Users who use both the stock DnsNameResolver and RoundRobinLoadBalancer
or PickFirstBalancer will see no behavioral change. Because they will
still round-robin on individual addresses from DNS, or do pick-first on
all addresses from DNS (PickFirstBalancer flattens all addresses).
The result is a simpler API and reduction of boilderplates.
`keepAlivedManager#onTransportshutdown` should not be called in `transport.shutdown()` because it is possible that there are still open RPC streams, and maybe inactive, so keepalive is still needed.
Preparing to support server side keepalive.
For the convience on server side, not to use Ping `onSuccess()` callback to cancle shutdownFuture any more, instead, regard `onDataReceived()` as ping Ack and cancel shutdownFuture in it.
The balancer service attaches a token string for each Server entry it
sends to the client. The client has to set the token to the "lb-token"
header when assigning that Server entry to an RPC.
For convenience of testing, also implemented hashCode() and equals() for
PickResult.
The build (jdk version jdk1.8.0_91) produces some noise
```
/usr/local/google/home/zdapeng/git/grpc-java/core/src/main/java/io/grpc/LoadBalancer.java:196: warning - Tag @link: can't find pickSubchannel(PickSubchannelArgs) in io.grpc.LoadBalancer.SubchannelPicker
/usr/local/google/home/zdapeng/git/grpc-java/core/src/main/java/io/grpc/LoadBalancer.java:388: warning - Tag @link: can't find pickSubchannel(
PickSubchannelArgs) in io.grpc.LoadBalancer.SubchannelPicker
/usr/local/google/home/zdapeng/git/grpc-java/core/src/main/java/io/grpc/LoadBalancer.java:178: warning - Tag @link: can't find pickSubchannel(PickSubchannelArgs) in io.grpc.LoadBalancer.SubchannelPicker
3 warnings
:grpc-core:javadocJar
```
It seems nothing wrong with the javadoc and it could be a javac's bug, but here's a workaround.
fix JavaStyle and ErrorProne warnings found in internal weekly import:
- Calls to ExpectedException#expect should always be followed by exactly one statement.
- Do not mock 'java.util.concurrent.Future'
handleResolvedAddresses constructs subchannels for each address in
an EquivalentAddressGroup, sharing a reference to an immutable
Atrributes. However, as noted, this Attributes instance contains
a mutable AtomicReference, eventually causing subchannel state
changes to be improperly reflected in *all* subchannels of an
EquivalentAddressGroup.
Guava 20 introduced some overloading optimizations for Preconditions
that require using Guava 20+ at runtime. Unfortunately, Guava 20 removes
some things that is causing incompatibilities with other libraries, like
Cassandra. While the incompatibility did trigger some of those libraries
to improve compatibility for newer Guavas, we'd like to give the
community more time to work through it. See #2688
At this commit, we appear to be compatible with Guava 18+. It's not
clear if we want to actually "support" 18, but it did compile. Guava 17
doesn't have at least MoreObjects, directExecutor, and firstNotNull.
Guava 21 compiles without warnings, so it should be compatible with
Guava 22 when it is released.
One test method will fail with the upcoming Guava 22, but this won't
impact applications. I made MoreThrowables to avoid using any
known-deprecated Guava methods in our JARs, to reduce pain for those
stuck with old versions of gRPC in the future (July 2018).
In the stand-alone Android apps I removed unnecessary explicit deps
instead of syncing the version used.
ErrorProne provides static analysis for common issues, including
misused variables GuardedBy locks.
This increases build time by 60% for parallel builds and 30% for
non-parallel, so I've provided a way to disable the check. It is on by
default though and will be run in our CI environments.
Futures almost universally should be handled in some way when being
returned, either to receive the value or to cancel scheduled tasks to
prevent leaks.
Netty is a bit of a special case though, since it constantly returns
futures that you ignore (even adding a listener returns the "this"
future). So we want to suppress the warning for code using Netty instead
of trying to fix it. When we enable ErrorProne in the build, we should
start passing -Xep:FutureReturnValueIgnored:OFF in the compilerArgs.
The new plugin uses a newer version of animalsniffer, allows overriding
the animalsniffer version used, and has up-to-date handling. The
up-to-date handling cuts fully incremental parallel build times in half,
from 5.5s to 2.7s.
The previous plugin was supposed to be verifying tests. However, either
it wasn't verifying them or its verification was broken.
The LBv2 setter was added in #2658 as an abstract method, which breaks
anyone who implements ManagedChannelBuilder. Here we add a default
implementation that throws.
This is needed because in interceptor tests, often the types cannot
be changed. The void methods stay for users who are writing tests
where they actually don't care about types. The noop methods
require types to be specified. This is for users who don't care
about the implementation. These represent different levels of
commitment.
This eases the transition of code Mocking MethodDescriptor, which
breaks in this release.
In some environments DNS is not available and is performed by the
CONNECT proxy. Nothing "special" should need to be done for these
environments, but the previous support took shortcuts which knowingly
would not support such environments.
This change should fix both OkHttp and Netty. Netty's
Bootstrap.connect() resolved the name immediately whereas using
ChannelPipeline.connect() waits until the address reaches the end of the
pipeline. Netty uses NetUtil.toSocketAddressString() to get the name of
the address, which uses InetSocketAddress.getHostString() when
available.
OkHttp is still using InetSocketAddress.getHostName() which may issue
reverse DNS lookups. However, if the reverse DNS lookup fails, it should
convert the IP to a textual string like getHostString(). So as long as
the reverse DNS maps to the same machine as the IP, there should only be
performance concerns, not correctness issues. Since the DnsNameResolver
is creating unresolved addresses, the reverse DNS lookups shouldn't
occur in the common case.
Because "core"'s test source already depends on "testing", e.g.,
`core/src/test/java/io/grpc/internal/ServerCallImplTest.java` uses
`testing/src/main/java/io/grpc/internal/testing/StatsTestUtils.java`,
which forms a circular dependency.
This change moves the StatsContext setter accessors from "core"'s test
source to "testing".
Resolves#2651
This removes an abuse of scheduled executor in ManagedChannelImpl. The executor
was used to avoid deadlocking. Now we run the work on the same thread, but
delay it until locks have been released.
There is no need to fix ManagedChannelImpl2. Due to its different
threading model it didn't have need to abuse the scheduledExecutor.
Fixes#2444
These are only used in internal tests. In production,
StatsContextFactory is loaded by the "Instrumentation" library and must
be one per process, thus we won't allow setting it on a per-channel or
per-server basis.
* Remove else, make it easier to read
* Ready subchanns list will be at max the same as the input list
* Make status private, add pkg-private getter for tests
* Remove empty field, cache size, simplify logic
Make list unmodifiable. Document index with GuardedBy (see #nextSubchannel())
add `getAttributes()` to `ClientStream` and `ClientCall` to be able to share clientTransport
information such as socket TOS with higher lever API's, once the RPC picks up an active transport that is ready to use.
Previously it does it at shutdown, which was wrong because executor may
still be used before the server is terminated.
Resolves#2034
Uses ObjectPool to make this change testable. Cleans up test and makes
it mostly single-threaded, except for two deadlock tests that have to be
multi-threaded.
If a LoadBalancer2 is passed in, the builder will create ManagedChannelImpl2 instead of ManagedChannelImpl. This allows us to test the LBv2 classes on a large scale.
1. Adapt to LoadBalancer2 interface. Channel holds on to a single
DelayedClientTransport2.
2. Lock-free: every channel state mutation, including Subchannel
mutations, calling into LoadBalancer, idleness and shutdown, is made
from channelExecutor.
3. Idleness grace period is no longer needed.
In upstream, Census is renamed to "Instrumentation". `com.google.census` is renamed to `com.google.instrumentation.stats`.
In gRPC, "census" in every name is replaced by "stats".
Besides API changes, this implementation is also up-to-date with the
latest design:
1. Delegate to round-robin and pick-first policies if requested by
the naming system.
2. OOB channels to LoadBalancer always use the LB authority provided by
the naming system.
3. Never send application RPCs to balancer addresses, even if the
address returns UNIMPLEMENTED error.
"Because of spurious wakeups, wait() must always be called in a loop".
Got rid of wait().
"If you return or throw from a finally, then values returned or thrown from the try-catch block will be ignored. Consider using try-with-resources instead."
Ignored the exception thrown from finally.
1. Use ChannelExecutor to run callbacks. Now callbacks are no longer
run under the delayed transport lock.
2. Use explicit picker version instead of relying on identity equality
to detect new pickers. Rationale: if reprocess() is called again with
the same picker, all pending streams will be reprocessed by this picker
again, thus there is no reason to leave out the racing new stream.
* Fork DelayedClientTransport into DelayedClientTransport2, and fix a race
on it. Consider the following sequence:
1. Channel is created. Picker is initially null
2. Channel has a new RPC. Because picker is null, the delayed transport
is picked, but newStream() is not called yet.
3. LoadBalancer updates a new picker to Channel. Channel runs
reprocess() but does nothing because no pending stream is there.
4. newStream() called on the delayed transport.
In previous implementation, newStream() on step 4 will not see the
picker. It will only use the next picker.
After this change, delayed transport would save the latest picker and
use it on newStream(), which is the desired behavior.
Also deleted all the code that will not be used after the LB refactor.
* Also fixed a bug: newStream() should always return a failing stream if it's shutdown. Previously it's not doing so if picker is not null.
OutputStreamAdapter is a private class, and is only ever called by two
places: ByteStreams.copy, which never calls the single byte method, and
DrainTo, which potentially can. There are two classes that implement
DrainTo, which is primarily ProtoInputStream. It calls
MessageLite.writeTo(OutputStream) or back again to ByteStreams.copy.
MessageLite.writeTo in turn wraps the OutputStream in a
CodedOutputStream.OutputStreamEncoder, which then never calls the single
byte version. Thus, all know implementations never call the single
byte override.
Additionally, it is well known that the single byte write is slow, and
is expected to be wrapped in a BufferedOutputStream if there are many
small writes.
Carl Mastrangelo
ZHANG Dapeng
Kun Zhang
Eric Anderson
Łukasz Strzałkowski
Jack Amadeo
Lukasz Strzalkowski
Kun Zhang
Ryan Michela
Eric Gribkoff
Anuraag Agrawal
Kenji Kaneda