- To make unit tests easier, added the BinaryLogSinkWriter abstract
class, which allows verifying high level arguments rather than low
level protobufs.
- remove InputStream based marshaller (IDENTITY_MARSHALLER) in favor
of byte[] because retries will need to call serialize multiple
times. This is achieved with a special InputStream that lets us
access the internal byte[] without copying.
- Change ByteBuffer based internal methods to use byte[]
- Update unit tests to reflect the above
Rather than querying the ServerImpl for its sockets, we register them
into channelz as with all the other entities, for consistency.
Error conditions are checked with `assert` statements, so we throw in
tests.
Changes:
- `ClientStreamListener.onClose(Status status, RpcProgress rpcProgress, Metadata trailers)` added.
- `AbstractClientStream.transportReportStatus(Status status, RpcProgress rpcProgress, boolean stopDelivery, Metadata trailers)` added
- `ClientCallImpl.ClientStreamListenerImpl` will ignore the arg `rpcProgress` (non retry)
- `RetriableStream.SubListener` will handle `rpcProgress` and decide if transparent retry.
- `NettyClientHandler` and `OkHttpClientTransport` will pass `RpcProgress.REFUSED` to client stream listener for later stream ids when received GOAWAY, or for stream received a RST_STREAM frame with REFUSED code.
- All other files are just a result of refactoring.
ProxyDetector is now responsible for resolving the proxy's
`InetSocketAddress`, and `ProxyParameters` asserts that the address is
resolved. The results are plumbed through using a `PairSocketAddress`,
which is a special `SocketAddress`.
If a proxy should be used but the proxy can not be resolved, we the
`DnsNameResolver` will re-attempt the resolution later.
Remove the unit test testing for unresolved proxy addresses, since
it's no longer applicable.
For channelz ref objects, we need the id as well as the name. By
returning a `WithLogId`, we can return the original object so that we
may call `toString` on it.
It is a channelz requirement that subchannels and channels are
separate, and we can not retrieve a root channel by asking for a
subchannel and vice versa.
Transport ststistics should really be a child member of SocketStats.
While we're at it, let's add the local and remote SocketAddress to
SocketStats, with a test.
With the advent of LoadBalancer2 (now just LoadBalancer) the need to use
internal APIs was removed; there's now first-class APIs exposed for
LoadBalancer use.
The old LoadBalancer API interacted with io.grpc.internal.ClientTransport
directly.
This partially reverts commit 48ca4527c1.
It leaves the changes to ServerCallImpl and test.
This also partially reverts "Lint fixes" commit
3002a23a0f which removed unused variables
which are now necessary again.
This is reverted for the combined result of two issues:
* Some users are testing that they get UNKNOWN when the service throws.
That's not unreasonable given the behavior was well-publicised when it
changed in v1.5. We should probably keep the UNKNOWN in some common
cases (like the service threw immediately, before sending anything).
* The client could see CANCELLED instead of INTERNAL as had been
intended. It's unclear as to why (I didn't investigate heavily). This
behavior is visible in MoreInProcessTest and was overlooked during
review.
This partially reverts commit c07ad68cbd.
The PARAMS_DEFAULT_PORT in NameResorverProvider is the preferred name.
The hope is still to kill NameResolver.Factory.
Channels, subchannels and oobchannels are added at creation and
removed at termination. Same goes for subchannels and
oobchannels. Everything is registered as a hard reference. This means
channels that are not properly cleaned up using `awaitTermination`
will remain visible in channelz.
In `FakeClock` we have `runDueTasks()` (without filter), `numPendingTasks(filter)`, and `getPendingTasks(filter)` which are useful. The `runDueTasks(filter)` method seems not a right way to test.
okhttp and netty libraries will retain hard references to
ManagedChannelImpl via its non static nested classes. Moving the
orphan check to a wrapper class solves this problem.
- Added ForwardingManagedChannel + test
Channel enters this mode whenever there is an uncaught throwable from
its ChannelExecutor, which is where most channel internal states are
mutated, such as load-balancing.
In panic mode, the channel will always report TRANSIENT_FAILURE as its
state, and will fail RPCs with an INTERNAL error code with the
uncaught throwable as the cause, which is helpful for investigating
bugs within gRPC and 3rd-party LoadBalancer implementations.
## Change to existing behavior
Previously if `LoadBalancer` throws in `handleResolvedAddressGroups()`, it would
be routed back to `LoadBalancer.handleNameResolutionError()`. Now it will make
the channel panic.
## Internal refactors
- Refactored out `shutdownNameResolverAndLoadBalancer()`, called from three code paths: `enterIdleMode()`, `delayedTransport.transportTerminated()`, and `panic()`.
- Refactored out `updateSubchannelPicker()`, called from both `updateBalancingState()` and `panic()`.
To avoid breakage, temporarily disable retry when stats or tracing is enabled.
This still agrees with the description of the javadoc of `ManagedChannelBuilder.enableRetry()`
```java
/**
* Enables the retry mechanism provided by the gRPC library.
*
* <p>This method may not work as expected for the current release because retry is not fully
* implemented yet.
*
* @return this
* @since 1.11.0
*/
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/3982")
public T enableRetry()
```
This removes the debug string added in 5a4794f2c because the issue was resolved
and the debug string is confusing for Netty, producing results like:
io.grpc.netty.NettyClientTransport$3: Frame size 216695976 exceeds maximum: 4194304
This makes it seem like it's a HTTP/2 frame problem and not a gRPC message
problem. Although it is clearer once you see the maximum is 4 MB, this already
bit me in #4086.
In general, we should talk about "messages" instead of "frames" when possible
as that's what is most comprehensible to the consumer of the description. I
also make sure to mention "gRPC" messages/frames because otherwise the message
is ambiguous.
It is the responsibility of the caller to close the `InputStream`
returned by `Marshaller`.
`Stream` takes ownership of any `InputStream`s passed in.
`DelayedStream` and the streams of `InProcessTransport` already
handle ownership transfer, but `AbstractStream` needs to call
`close`.
The `BinaryLog` will write `GrpcLogEntry`s to the sink, which is
intended as a pluggable interface. It is the sink's
responsibility to send the binlog protos to disk, to a remote
logging service, etc.
This change is being made in preparation for fixing a bug in the enforcement
of `@guardedby`.
Non-static inner classes can refer to locks declared by their enclosing
instances. Static member types (static classes, interfaces) cannot.
Since this address is never resolved gRPC fails to connect with an
UnresolvedAddressException when the env variable is specified. This
should resolve the address before we attempt to proxy to it.
This reverts commit 98aa69af72.
The GSON dependency is noticable on Android for no benefit (currently).
Reverting until we figure out what we want to do.
This reverts commit b01609572a.
The GSON dependency is noticable on Android for no benefit (currently).
Reverting until we figure out what we want to do.
Instead, pass a ServerCallInfo object containing the interesting bits
of info. This lets us modify the call handler for binary logging, but
still provide the original info to the StatsTraceContext API.
`@Internal` and `@ExperimentalApi` annotations are not visible at compile time
of application (not grpc-java) because their retention policies are `SOURCE`.
So, change retention policies to `CLASS`.
Use call executor and scheduled executor service to schedule and run `retry()`.
Backoff amount computation from retry policy and testcases during backoff will be added in future PRs.
It's more reasonable and convenient to return a `Runnable` for `commit()` method because we want to execute part of the commit under a lock and run part of it out of the lock.
I think the idle state transitions are correct. I have looked at them and tried tracing a few paths through. This doesn't go full idle because the name resolver doesn't need to be restarted. Also, the LB transition happens inside of a NR callback, so it would be odd to have the NR terminate itself upon successful resolution. (Might this cause recursion? I think it may).
Spies are really magical and easily produce unexpected results. Using them in
tests can easily yield tests that don't do what you think they do. Delegation
is much safer when possible.
Delegation doesn't work when methods `return true`, final methods, and with
restricted visibility, though. So CensusModulesTest and
MaxConnectionIdleManagerTest are left as-is.
As delayedTransport `shutdown` will make retry attempts that are in backoff unable to create new stream. So wrap delayedTransport with `retriableTransport`, which will keep uncommitted retriable streams alive when `shutdown`.
Using META-INF for loading the version is broken on Android and
frequently broken when gRPC is shaded. Having hard-coded strings be
replaced on version bumps has been working well.
Fixes#2098
`ManagedChannelImpl` and `OobChannelq now both have a package
private field `ChannelStats`. There will be an internal access
API coming in a follow up PR.
The channelz service must not live in io.grpc.internal, and channelz
needs to be able to get the identifier of the entities it
tracks. Since io.grpc can not refer to io.grpc.internal, the LogId
must be moved out of internal.
With an approach using a variant of `DelayedStream` - `RetriableStream`.
`RetriableStream` is a logical stream per call that can create a sequence of `substreams`, the physical streams for the retry attempts. It could also create a family of substreams in parallel in hedging case.
`RetriableStream` buffers, delays and executes stream operations before `passThrough`, where `passThrough` is meant by `commit` as well as the buffer `drained` by the winning substream.
This PR only implements buffering messages and replaying buffered messages when retry.
Retry policy, hedging, transparent retry, backoff, max buffer size and APIs that users can enable retry are not included in this PR.
Each MethodDescriptor will have a binary log, though the log may
be a noop log. The binary log class is a skeleton class at the
moment, but does contain the max header and message length
info. The limits are determined by parsing the shell variable
GRPC_BINARY_LOG_CONFIG.
Previously it was 20s. 20s isn't _bad_, but may be too aggressive.
Having it longer doesn't hurt us too much; we just need some bound.
Since the MAX_BACKOFF on client-side is 120s, this will have client and
server match, except for jitter. Although I will note that the
client-side value covers more work than the server-side, such as
attempting to connect to multiple hosts.
The class is still used internally, so we move it to context's tests for
it to be reused. To avoid a circular dependency with context's tests
depending on core's tests, StaticTestingClassLoader was also moved to
context's tests.
This is driven by a need to modernize DeadlineSubject for newer versions
of Truth, but the newer versions of Truth update Guava. To avoid leaking
the Guava update to all users of grpc-testing, we're removing the
Subject. In our internal tests we can update the Truth dependency with
less issue.
handshakeTimeout is unnecessary for InProcess, and the scheduling is causing
Thread creation that is breaking restrictive test environments. Those
environments are mostly broken already because client-side will try to create
Threads as well, but they are currently lucking out that the exception on
client-side doesn't break much.
RPC upstarts are counted into metrics
RPC_{CLIENT,SERVER}_STARTED_COUNT. In addition, RPC completions are
counted into metrics RPC_{CLIENT,SERVER}_FINISHED_COUNT. From these
metrics, users will be able to derive count of RPCs that are currently
active.
Eric Anderson
zpencer
Eric Gribkoff
Carl Mastrangelo
ZHANG Dapeng
Kun Zhang
Rama Chavali
Ryan Michela
Snow Pettersen
Bogdan Drutu
Shohei Kamimori
kenji yoshida
David Cowden