Skip to content

Commit 6ff8eca

Browse files
committed
core: Don't pre-compute DEADLINE_EXCEEDED message for delayed calls
The main reason I made a change here was to fix the tense from the deadline "will be exceeded in" to "was exceeded after". But we really don't want to be doing the string formatting unless the deadline is actually exceeded. There were a few more changes to make some variables effectively final.
1 parent 8021727 commit 6ff8eca

File tree

2 files changed

+23
-25
lines changed

2 files changed

+23
-25
lines changed

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

Lines changed: 22 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -96,15 +96,13 @@ private boolean isAbeforeB(@Nullable Deadline a, @Nullable Deadline b) {
9696
private ScheduledFuture<?> scheduleDeadlineIfNeeded(
9797
ScheduledExecutorService scheduler, @Nullable Deadline deadline) {
9898
Deadline contextDeadline = context.getDeadline();
99-
if (deadline == null && contextDeadline == null) {
100-
return null;
101-
}
102-
long remainingNanos = Long.MAX_VALUE;
103-
if (deadline != null) {
99+
String deadlineName;
100+
long remainingNanos;
101+
if (deadline != null && isAbeforeB(deadline, contextDeadline)) {
102+
deadlineName = "CallOptions";
104103
remainingNanos = deadline.timeRemaining(NANOSECONDS);
105-
}
106-
107-
if (contextDeadline != null && contextDeadline.timeRemaining(NANOSECONDS) < remainingNanos) {
104+
} else if (contextDeadline != null) {
105+
deadlineName = "Context";
108106
remainingNanos = contextDeadline.timeRemaining(NANOSECONDS);
109107
if (logger.isLoggable(Level.FINE)) {
110108
StringBuilder builder =
@@ -121,29 +119,29 @@ private ScheduledFuture<?> scheduleDeadlineIfNeeded(
121119
}
122120
logger.fine(builder.toString());
123121
}
124-
}
125-
126-
long seconds = Math.abs(remainingNanos) / TimeUnit.SECONDS.toNanos(1);
127-
long nanos = Math.abs(remainingNanos) % TimeUnit.SECONDS.toNanos(1);
128-
final StringBuilder buf = new StringBuilder();
129-
String deadlineName = isAbeforeB(contextDeadline, deadline) ? "Context" : "CallOptions";
130-
if (remainingNanos < 0) {
131-
buf.append("ClientCall started after ");
132-
buf.append(deadlineName);
133-
buf.append(" deadline was exceeded. Deadline has been exceeded for ");
134122
} else {
135-
buf.append("Deadline ");
136-
buf.append(deadlineName);
137-
buf.append(" will be exceeded in ");
123+
return null;
138124
}
139-
buf.append(seconds);
140-
buf.append(String.format(Locale.US, ".%09d", nanos));
141-
buf.append("s. ");
142125

143126
/* Cancels the call if deadline exceeded prior to the real call being set. */
144127
class DeadlineExceededRunnable implements Runnable {
145128
@Override
146129
public void run() {
130+
long seconds = Math.abs(remainingNanos) / TimeUnit.SECONDS.toNanos(1);
131+
long nanos = Math.abs(remainingNanos) % TimeUnit.SECONDS.toNanos(1);
132+
StringBuilder buf = new StringBuilder();
133+
if (remainingNanos < 0) {
134+
buf.append("ClientCall started after ");
135+
buf.append(deadlineName);
136+
buf.append(" deadline was exceeded. Deadline has been exceeded for ");
137+
} else {
138+
buf.append("Deadline ");
139+
buf.append(deadlineName);
140+
buf.append(" was exceeded after ");
141+
}
142+
buf.append(seconds);
143+
buf.append(String.format(Locale.US, ".%09d", nanos));
144+
buf.append("s");
147145
cancel(
148146
Status.DEADLINE_EXCEEDED.withDescription(buf.toString()),
149147
// We should not cancel the call if the realCall is set because there could be a

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2239,7 +2239,7 @@ public long nanoTime() {
22392239
assertThat(testCall).isNull();
22402240
verifyRpcDelayedThenAborted(observer, 4000L, Status.DEADLINE_EXCEEDED.withDescription(
22412241
"Deadline exceeded after up to 5000 ns of fault-injected delay:"
2242-
+ " Deadline CallOptions will be exceeded in 0.000004000s. "));
2242+
+ " Deadline CallOptions was exceeded after 0.000004000s"));
22432243
}
22442244

22452245
@Test

0 commit comments

Comments
 (0)