After debugging #2153, it would have been nice to know what the exact
parameter was that was null. This change adds a name for each
checkNotNull (and tries to normalized on static imports in order to
shorten lines)
780b2696 caused all failures for blocked unary stubs to have a
StatusRuntimeException as the cause of the StatusRuntimeException, with
the two exceptions having almost the same status.
It was withCallCredentials on the stub to avoid confusion with channel
credentials (which don't exist in Java at this time, but do in other
languages), and having the method names be different doesn't add value.
onReady/isReady previously could disagree causing a sort of deadlock
where the application isn't sending because grpc said not to, but won't
be informed to send via onReady later.
This is a stack trace from inprocessTransportOutboundFlowControl. The
line numbers are from this commit but with the changes to DelayedStream
disabled:
at io.grpc.internal.DelayedStream.isReady(DelayedStream.java:306)
(That is isReady returning false because fallThrough == false)
at io.grpc.internal.ClientCallImpl.isReady(ClientCallImpl.java:382)
at io.grpc.stub.ClientCalls$CallToStreamObserverAdapter.isReady(ClientCalls.java:289)
at io.grpc.stub.ClientCallsTest$8$1.run(ClientCallsTest.java:403)
(And yet that was the onReady callback, and it won't be called again)
at io.grpc.stub.ClientCalls$StreamObserverToCallListenerAdapter.onReady(ClientCalls.java:377)
at io.grpc.internal.ClientCallImpl$ClientStreamListenerImpl$4.runInContext(ClientCallImpl.java:481)
at io.grpc.internal.ContextRunnable.run(ContextRunnable.java:52)
at io.grpc.internal.SerializeReentrantCallsDirectExecutor.execute(SerializeReentrantCallsDirectExecutor.java:65)
at io.grpc.internal.ClientCallImpl$ClientStreamListenerImpl.onReady(ClientCallImpl.java:478)
at io.grpc.internal.DelayedStream$DelayedStreamListener.onReady(DelayedStream.java:366)
at io.grpc.inprocess.InProcessTransport$InProcessStream$InProcessServerStream.request(InProcessTransport.java:284)
at io.grpc.internal.ServerCallImpl.request(ServerCallImpl.java:99)
at io.grpc.stub.ServerCalls$ServerCallStreamObserverImpl.request(ServerCalls.java:345)
at io.grpc.stub.ClientCallsTest.inprocessTransportOutboundFlowControl(ClientCallsTest.java:432)
Fixes#1932
This reverts commit 3df1446deb.
The commit was adding to the difficulty of integration for testing. By
itself it isn't bad, so this is a temporary revert until the many other
commits are absorbed and then it will be reapplied.
This does have a manual edit for ClientCallsTest.
The test has found a legitimate flake caused by a race in DelayedStream
related to onReady. Disable it until the fix arrives to prevent CI
failure noise.
If the timoutout expires, the test will almost certainly fail now due to
responses not matching expected responses, but it isn't as obvious why
the test fails. Swapping to ArrayList prints the entire list when they
don't match, which also makes it easier to diagnose.
This is, in part, to help diagnose #1932
Introduce CallCredentials as a first-class option to allow applications
to set per-call credentials into headers for outgoing RPCs. This will
supersede ClientAuthInterceptor. It has access to more
information (e.g., transport attributes, MethodDescriptor) and allow
results to be returned asynchronously, e.g., from a blocking I/O, which
was problemantic with ClientAuthInterceptor.
This change updates the behavior of the core compression semantics. Previously,
if the codec was "identity", nothing was set on the wire. This is allowed by
the spec, but doesn't match what wrapped languages do.
Additionally, the interop tests will now attempt to honor the requested
compression.
Resolves#1221
Add ClientCall.cancel(String, Throwable) and deprecate
ClientCall.cancel(). Will delete cancel() after all known third-party
overriders have switched to overriding the new one.
- Made CallOptions use the Deadline type instead of a
long to represent a deadline.
- Added new methods CallOptions.withDeadline(Deadline) and
AbstractStub.withDeadline(Deadline). The methods are
marked experimental, as the Deadline class is marked
experimental. These methods are meant to replace
CallOptions.withDeadlineNanoTime(Long) and
AbstractStub.withDeadlineNanoTime(Long), which have
been deprecated.
- Updated CallOptions.toString() to include all fields.
while still allowing stubs to be used.
Currently these APIs are only useful on the server side, a separate proposal will be made for adding client support
An example of usage is in TestServiceImpl.streamingOutputCallManualFlowControl which listens for flow-control 'onReady' events and produces messages to the client.
Added some unit testing for ServerCalls
Although the changes were determined automatically, they were manually
applied to the codebase.
ClientCalls actually has a bug fix, since the suggestion to add
interrupt() made it obvious that interrupted() was inappropriate.
This was already being done in one case, but should have been done for
the other occurrances of InterruptedException. Before the
RuntimeException is just asking to be a bug since application code will
commonly only catch StatusRuntimeException.
This reduces the necessary number of threads in the application executor
and provides a small improvement in latency (~15μs, which is normally in
the noise, but would be a 5% improvement).
Benchmark (direct) (transport) Mode Cnt Score Error Units
Before:
TransportBenchmark.unaryCall1024 true INPROCESS avgt 10 1566.168 ± 13.677 ns/op
TransportBenchmark.unaryCall1024 false INPROCESS avgt 10 35769.532 ± 2358.967 ns/op
After:
TransportBenchmark.unaryCall1024 true INPROCESS avgt 10 1813.778 ± 19.995 ns/op
TransportBenchmark.unaryCall1024 false INPROCESS avgt 10 18568.223 ± 1679.306 ns/op
The benchmark results are exactly what we would expect, assuming that
half of the benefit of direct is on server and half on client:
1566 + (35769 - 1566) / 2 = 18668 ns --vs-- 18568 ns
It is expected that direct=true would get worse, because
SerializingExecutor is now used instead of
SerializeReentrantCallsDirectExecutor plus the additional cost of
ThreadlessExecutor.
In the future we could try to detect the ThreadlessExecutor and ellide
Serializ*Executor completely (as is possible for any single-threaded
executor). We could also optimize the queue used in ThreadlessExecutor
to be single-producer, single-consumer. I don't expect to do those
optimizations soon, however.
ServerCall already had "headers must be sent before any messages, which
must be sent before closing," but the implementation did not enforce it
and our async server handler didn't obey.
The benefit of forcing sending headers first is that it removes the only
implicit call in our API and interceptors dealing just with metadata
don't need to override sendMessage. The implicit behavior was bug-prone
since it wasn't obvious you were forgetting that headers may not be
sent.
Carl Mastrangelo
Eric Anderson
nmittler
Louis Ryan
Kun Zhang
elandau
Lukasz Strzalkowski
Łukasz Strzałkowski
Steven Parkes
buchgr
Xudong Ma