Skip to content

Commit 56d2b25

Browse files
authored
core: DelayedClientCall should include context in error
If the RPC is able to continue in time it can proceed to ClientCallImpl, which will describe the delay as coming from name resolution. But if it fails before then the message didn't give any hint what gRPC was waiting on. As noticed during b/526868988.
1 parent 71a10dc commit 56d2b25

6 files changed

Lines changed: 41 additions & 25 deletions

File tree

core/src/main/java/io/grpc/internal/DelayedClientCall.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,9 @@
4949
*/
5050
public class DelayedClientCall<ReqT, RespT> extends ClientCall<ReqT, RespT> {
5151
private static final Logger logger = Logger.getLogger(DelayedClientCall.class.getName());
52+
53+
/** A string describing what this call is waiting on. */
54+
private final String bufferContext;
5255
/**
5356
* A timer to monitor the initial deadline. The timer must be cancelled on transition to the real
5457
* call.
@@ -76,7 +79,11 @@ public class DelayedClientCall<ReqT, RespT> extends ClientCall<ReqT, RespT> {
7679
private DelayedListener<RespT> delayedListener;
7780

7881
protected DelayedClientCall(
79-
Executor callExecutor, ScheduledExecutorService scheduler, @Nullable Deadline deadline) {
82+
String bufferContext,
83+
Executor callExecutor,
84+
ScheduledExecutorService scheduler,
85+
@Nullable Deadline deadline) {
86+
this.bufferContext = checkNotNull(bufferContext, "bufferContext");
8087
this.callExecutor = checkNotNull(callExecutor, "callExecutor");
8188
checkNotNull(scheduler, "scheduler");
8289
context = Context.current();
@@ -143,7 +150,8 @@ public void run() {
143150
}
144151
buf.append(seconds);
145152
buf.append(String.format(Locale.US, ".%09d", nanos));
146-
buf.append("s");
153+
buf.append("s waiting for ");
154+
buf.append(bufferContext);
147155
cancel(
148156
Status.DEADLINE_EXCEEDED.withDescription(buf.toString()),
149157
// We should not cancel the call if the realCall is set because there could be a

core/src/main/java/io/grpc/internal/ManagedChannelImpl.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -986,9 +986,12 @@ private final class PendingCall<ReqT, RespT> extends DelayedClientCall<ReqT, Res
986986
final CallOptions callOptions;
987987
private final long callCreationTime;
988988

989-
PendingCall(
990-
Context context, MethodDescriptor<ReqT, RespT> method, CallOptions callOptions) {
991-
super(getCallExecutor(callOptions), scheduledExecutor, callOptions.getDeadline());
989+
PendingCall(Context context, MethodDescriptor<ReqT, RespT> method, CallOptions callOptions) {
990+
super(
991+
"name_resolver",
992+
getCallExecutor(callOptions),
993+
scheduledExecutor,
994+
callOptions.getDeadline());
992995
this.context = context;
993996
this.method = method;
994997
this.callOptions = callOptions;

core/src/test/java/io/grpc/internal/DelayedClientCallTest.java

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,8 @@ public class DelayedClientCallTest {
6666

6767
@Test
6868
public void allMethodsForwarded() throws Exception {
69-
DelayedClientCall<String, Integer> delayedClientCall =
70-
new DelayedClientCall<>(callExecutor, fakeClock.getScheduledExecutorService(), null);
69+
DelayedClientCall<String, Integer> delayedClientCall = new DelayedClientCall<>(
70+
"test", callExecutor, fakeClock.getScheduledExecutorService(), null);
7171
callMeMaybe(delayedClientCall.setCall(mockRealCall));
7272
ForwardingTestUtil.testMethodsForwarded(
7373
ClientCall.class,
@@ -94,18 +94,22 @@ public Object get(Method method, int argPos, Class<?> clazz) {
9494
@Test
9595
public void deadlineExceededWhileCallIsStartedButStillPending() {
9696
DelayedClientCall<String, Integer> delayedClientCall = new DelayedClientCall<>(
97-
callExecutor, fakeClock.getScheduledExecutorService(), Deadline.after(10, SECONDS));
97+
"tESt", callExecutor, fakeClock.getScheduledExecutorService(),
98+
Deadline.after(10, SECONDS, fakeClock.getDeadlineTicker()));
9899

99100
delayedClientCall.start(listener, new Metadata());
100101
fakeClock.forwardTime(10, SECONDS);
101102
verify(listener).onClose(statusCaptor.capture(), any(Metadata.class));
102103
assertThat(statusCaptor.getValue().getCode()).isEqualTo(Status.Code.DEADLINE_EXCEEDED);
104+
assertThat(statusCaptor.getValue().getDescription())
105+
.isEqualTo("Deadline CallOptions was exceeded after 10.000000000s waiting for tESt");
103106
}
104107

105108
@Test
106109
public void listenerEventsPropagated() {
107110
DelayedClientCall<String, Integer> delayedClientCall = new DelayedClientCall<>(
108-
callExecutor, fakeClock.getScheduledExecutorService(), Deadline.after(10, SECONDS));
111+
"test", callExecutor, fakeClock.getScheduledExecutorService(),
112+
Deadline.after(10, SECONDS, fakeClock.getDeadlineTicker()));
109113
delayedClientCall.start(listener, new Metadata());
110114
callMeMaybe(delayedClientCall.setCall(mockRealCall));
111115
@SuppressWarnings("unchecked")
@@ -130,7 +134,7 @@ public void listenerEventsPropagated() {
130134
@Test
131135
public void setCallThenStart() {
132136
DelayedClientCall<String, Integer> delayedClientCall = new DelayedClientCall<>(
133-
callExecutor, fakeClock.getScheduledExecutorService(), null);
137+
"test", callExecutor, fakeClock.getScheduledExecutorService(), null);
134138
callMeMaybe(delayedClientCall.setCall(mockRealCall));
135139
delayedClientCall.start(listener, new Metadata());
136140
delayedClientCall.request(1);
@@ -146,7 +150,7 @@ public void setCallThenStart() {
146150
@Test
147151
public void startThenSetCall() {
148152
DelayedClientCall<String, Integer> delayedClientCall = new DelayedClientCall<>(
149-
callExecutor, fakeClock.getScheduledExecutorService(), null);
153+
"test", callExecutor, fakeClock.getScheduledExecutorService(), null);
150154
delayedClientCall.start(listener, new Metadata());
151155
delayedClientCall.request(1);
152156
Runnable r = delayedClientCall.setCall(mockRealCall);
@@ -167,7 +171,7 @@ public void startThenSetCall() {
167171
@SuppressWarnings("unchecked")
168172
public void cancelThenSetCall() {
169173
DelayedClientCall<String, Integer> delayedClientCall = new DelayedClientCall<>(
170-
callExecutor, fakeClock.getScheduledExecutorService(), null);
174+
"test", callExecutor, fakeClock.getScheduledExecutorService(), null);
171175
delayedClientCall.start(listener, new Metadata());
172176
delayedClientCall.request(1);
173177
delayedClientCall.cancel("cancel", new StatusException(Status.CANCELLED));
@@ -183,7 +187,7 @@ public void cancelThenSetCall() {
183187
@SuppressWarnings("unchecked")
184188
public void setCallThenCancel() {
185189
DelayedClientCall<String, Integer> delayedClientCall = new DelayedClientCall<>(
186-
callExecutor, fakeClock.getScheduledExecutorService(), null);
190+
"test", callExecutor, fakeClock.getScheduledExecutorService(), null);
187191
delayedClientCall.start(listener, new Metadata());
188192
delayedClientCall.request(1);
189193
Runnable r = delayedClientCall.setCall(mockRealCall);
@@ -206,7 +210,8 @@ public void delayedCallsRunUnderContext() throws Exception {
206210
Object goldenValue = new Object();
207211
DelayedClientCall<String, Integer> delayedClientCall =
208212
Context.current().withValue(contextKey, goldenValue).call(() ->
209-
new DelayedClientCall<>(callExecutor, fakeClock.getScheduledExecutorService(), null));
213+
new DelayedClientCall<>(
214+
"test", callExecutor, fakeClock.getScheduledExecutorService(), null));
210215
AtomicReference<Context> readyContext = new AtomicReference<>();
211216
delayedClientCall.start(new ClientCall.Listener<Integer>() {
212217
@Override public void onReady() {
@@ -232,7 +237,7 @@ public void delayedCallsRunUnderContext() throws Exception {
232237
@Test
233238
public void listenerThrowsInPendingCallback_cancelsRealCall() {
234239
DelayedClientCall<String, Integer> delayedClientCall = new DelayedClientCall<>(
235-
callExecutor, fakeClock.getScheduledExecutorService(), null);
240+
"test", callExecutor, fakeClock.getScheduledExecutorService(), null);
236241
final RuntimeException boom = new RuntimeException("boom");
237242
ClientCall.Listener<Integer> throwingListener = new ClientCall.Listener<Integer>() {
238243
@Override
@@ -261,7 +266,7 @@ public void start(Listener<Integer> listener, Metadata metadata) {
261266
@Test
262267
public void listenerThrowsInPendingOnHeaders_cancelsRealCall() {
263268
DelayedClientCall<String, Integer> delayedClientCall = new DelayedClientCall<>(
264-
callExecutor, fakeClock.getScheduledExecutorService(), null);
269+
"test", callExecutor, fakeClock.getScheduledExecutorService(), null);
265270
final RuntimeException boom = new RuntimeException("boom");
266271
ClientCall.Listener<Integer> throwingListener = new ClientCall.Listener<Integer>() {
267272
@Override
@@ -286,7 +291,7 @@ public void start(Listener<Integer> listener, Metadata metadata) {
286291
@Test
287292
public void listenerThrowsInPendingOnReady_cancelsRealCall() {
288293
DelayedClientCall<String, Integer> delayedClientCall = new DelayedClientCall<>(
289-
callExecutor, fakeClock.getScheduledExecutorService(), null);
294+
"test", callExecutor, fakeClock.getScheduledExecutorService(), null);
290295
final RuntimeException boom = new RuntimeException("boom");
291296
ClientCall.Listener<Integer> throwingListener = new ClientCall.Listener<Integer>() {
292297
@Override
@@ -311,7 +316,7 @@ public void start(Listener<Integer> listener, Metadata metadata) {
311316
@Test
312317
public void onCloseExceptionCaughtAndLogged() {
313318
DelayedClientCall<String, Integer> delayedClientCall = new DelayedClientCall<>(
314-
callExecutor, fakeClock.getScheduledExecutorService(), null);
319+
"test", callExecutor, fakeClock.getScheduledExecutorService(), null);
315320
final RuntimeException boom = new RuntimeException("boom");
316321
final AtomicReference<Status> observed = new AtomicReference<>();
317322
ClientCall.Listener<Integer> throwingListener = new ClientCall.Listener<Integer>() {
@@ -339,7 +344,7 @@ public void start(Listener<Integer> listener, Metadata metadata) {
339344
@Test
340345
public void listenerThrowsInPassThroughOnMessage_cancelsRealCall() {
341346
DelayedClientCall<String, Integer> delayedClientCall = new DelayedClientCall<>(
342-
callExecutor, fakeClock.getScheduledExecutorService(), null);
347+
"test", callExecutor, fakeClock.getScheduledExecutorService(), null);
343348
final RuntimeException boom = new RuntimeException("boom");
344349
ClientCall.Listener<Integer> throwingListener = new ClientCall.Listener<Integer>() {
345350
@Override
@@ -362,7 +367,7 @@ public void onMessage(Integer msg) {
362367
@Test
363368
public void listenerThrowsInPassThroughOnHeaders_cancelsRealCall() {
364369
DelayedClientCall<String, Integer> delayedClientCall = new DelayedClientCall<>(
365-
callExecutor, fakeClock.getScheduledExecutorService(), null);
370+
"test", callExecutor, fakeClock.getScheduledExecutorService(), null);
366371
final RuntimeException boom = new RuntimeException("boom");
367372
ClientCall.Listener<Integer> throwingListener = new ClientCall.Listener<Integer>() {
368373
@Override
@@ -385,7 +390,7 @@ public void onHeaders(Metadata headers) {
385390
@Test
386391
public void listenerThrowsInPassThroughOnReady_cancelsRealCall() {
387392
DelayedClientCall<String, Integer> delayedClientCall = new DelayedClientCall<>(
388-
callExecutor, fakeClock.getScheduledExecutorService(), null);
393+
"test", callExecutor, fakeClock.getScheduledExecutorService(), null);
389394
final RuntimeException boom = new RuntimeException("boom");
390395
ClientCall.Listener<Integer> throwingListener = new ClientCall.Listener<Integer>() {
391396
@Override
@@ -408,7 +413,7 @@ public void onReady() {
408413
@Test
409414
public void listenerThrowsInPassThrough_subsequentCallbacksSwallowedAndOnCloseOverridden() {
410415
DelayedClientCall<String, Integer> delayedClientCall = new DelayedClientCall<>(
411-
callExecutor, fakeClock.getScheduledExecutorService(), null);
416+
"test", callExecutor, fakeClock.getScheduledExecutorService(), null);
412417
final RuntimeException boom = new RuntimeException("boom");
413418
final AtomicReference<Integer> lastMessage = new AtomicReference<>();
414419
final AtomicReference<Status> closeStatus = new AtomicReference<>();

xds/src/main/java/io/grpc/xds/ExternalProcessorClientInterceptor.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ private static String getHeaderValue(Metadata headers, String headerName) {
397397
private static class DataPlaneDelayedCall<ReqT, RespT> extends DelayedClientCall<ReqT, RespT> {
398398
DataPlaneDelayedCall(
399399
Executor executor, ScheduledExecutorService scheduler, @Nullable Deadline deadline) {
400-
super(executor, scheduler, deadline);
400+
super("ext_proc", executor, scheduler, deadline);
401401
}
402402
}
403403

xds/src/main/java/io/grpc/xds/FaultFilter.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,7 @@ private final class DelayInjectedCall<ReqT, RespT> extends DelayedClientCall<Req
414414
long delayNanos, Executor callExecutor, ScheduledExecutorService scheduler,
415415
@Nullable Deadline deadline,
416416
final Supplier<? extends ClientCall<ReqT, RespT>> callSupplier) {
417-
super(callExecutor, scheduler, deadline);
417+
super("httpfault_filter", callExecutor, scheduler, deadline);
418418
activeFaultCounter.incrementAndGet();
419419
ScheduledFuture<?> task = scheduler.schedule(
420420
new Runnable() {

xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2339,7 +2339,7 @@ public long nanoTime() {
23392339
assertThat(testCall).isNull();
23402340
verifyRpcDelayedThenAborted(observer, 4000L, Status.DEADLINE_EXCEEDED.withDescription(
23412341
"Deadline exceeded after up to 5000 ns of fault-injected delay:"
2342-
+ " Deadline CallOptions was exceeded after 0.000004000s"));
2342+
+ " Deadline CallOptions was exceeded after 0.000004000s waiting for httpfault_filter"));
23432343
}
23442344

23452345
@Test

0 commit comments

Comments
 (0)