The Set from Collections.synchronizedSet() is not protected against
concurrent modification during iteration. We copy an ArrayList out of it
for iteration.
When triggered, it caused the ClientCall.Listener never to complete.
Fixes#1343
The new test doesn't actually fail on my machine with the old code, but
we would hope it would be flaky. Since a race is involved, I don't
expect a more reliable test.
This provides more structured data into the application for it to do
special handling.
In general, we would hope most people don't need this functionality, but
it is a good escape hatch to allow users to workaround infrastructure
problems.
This reverts commit eca1f7c1d6.
We want to preserve the status message identical to what the server
sent. We'll need a better way to communicate debugging details.
- Set knownAcceptEncodingRegistry in SingleTransportChannel
- Change the package name of load_balancer.proto to be consistent with
what is used internally.
Use LinkedHashSet in BlankFutureProvider so that it fulfills the futures
in the same order as they were created. This makes the behavior more
predictable, thus fixes the flakiness of GrpclbLoadBalancerTest and
simplifies BlankFutureProviderTest.
This LoadBalancer does round-robin on a address list received from a
separate "load-balancer service", via the protocol defined in
load_balancer.proto. Everything is put under a subproject `grpc-grpclb`,
because it has dependency to protobuf.
updateRetainedTransports() now accepts EquivalentAddressGroups. The
LoadBalancer merges the LB and normal server address groups when calling
it.
`TransportSet` won't connect/reconnect until a transport is requested
through `obtainActiveTransport()`.
Lazy connection is safer, and more desirable in mobile environments.
It's also what C core is doing.
To warm up connections, `LoadBalancer` can call
`TransportManager.getTransport()`, even periodically if it wants to
maintain live connections.
Locking is necessary to avoid race with setStream() since the
application may be calling cancel() in another thread, and we must not
call listener.close() multiple times.
"onHeaders always is called" is from the initial gRPC design where
0-many context frames were sent before the first message. That is why it
was possible for "no headers were received". That has long-since not
been true, but in the various refactorings this language was
accidentally left. The language "Headers always precede messages" is
correct since headers are only guaranteed if messages were sent.
TransportSet (as well as TransportManager) accepts a group of equivalent
addresses (EquivalentAddressGroup) instead of a single address.
TransportSet will move down the address list when reconnecting, and
applies back-off only after the entire list has been tried.
Main benefits:
- It will stop channel from trying to reconnect addresses that have been
failed to connect to and moved away from. (#1212)
- It will make future implementation of Happy Eyeballs possible, inside
TransportSet.
Tested: covered by TransportSetTest and
ManagedChannelImplTransportManagerTest.
See the javadocs of ManagedChannelBuilder.forTarget().
The most interesting case is passing an IPv6 address as target. It can
be either be passed as an authority, where brackets should not be
escaped ([::1]), or as a path of a full URI, where brackets must be
escaped (dns:///%5B::1%5D).
Previously, dns:///[::1], being an invalid URI (brackets not allowed in
path), would be converted to dns:////dns:///%5B::1%5D and passed to
DnsNameResolver. Though it would fail eventually, the error would be
very confusing to users. I changed the logic so that it would try with
dns:/// only if the target string doesn't look like an intended URI
target.
I have restricted the "URI target" to be absolute and hierarchical,
i.e., must start with scheme://. I couldn't find a way to better tell if
a string is intended to be a URI, but I am open to other options.
Refactored tests:
- Move the tests for getNameResolver() into a separate file
ManagedChannelImplGetNameResolverTest, because those tests are not
quite compatible with the facility provided by ManagedChannelImplTest.
- Create DnsNameResolverTest. Move DnsNameResolver out of the factory
class to accommodate for the test.
When using a direct executor we don't need to wrap calls in a
serializing executor and can thus also avoid the overhead that
comes with it.
Benchmarks show that throughput can be improved substantially.
On my MBP I get a 24% improvement in throughput with also
significantly better latency throughout all percentiles.
(running qps_client and qps_server with --address=localhost:1234 --directexecutor)
=== BEFORE ===
Channels: 4
Outstanding RPCs per Channel: 10
Server Payload Size: 0
Client Payload Size: 0
50%ile Latency (in micros): 452
90%ile Latency (in micros): 600
95%ile Latency (in micros): 726
99%ile Latency (in micros): 1314
99.9%ile Latency (in micros): 5663
Maximum Latency (in micros): 136447
QPS: 78498
=== AFTER ===
Channels: 4
Outstanding RPCs per Channel: 10
Server Payload Size: 0
Client Payload Size: 0
50%ile Latency (in micros): 399
90%ile Latency (in micros): 429
95%ile Latency (in micros): 453
99%ile Latency (in micros): 650
99.9%ile Latency (in micros): 1265
Maximum Latency (in micros): 33855
QPS: 97552
Eric Anderson
Carl Mastrangelo
Kun Zhang
nmittler
Xudong Ma
Louis Ryan
buchgr