Skip to content

Commit bfd48ce

Browse files
committed
Make backed off request after previous throttling always send cache miss as the route lookup reason.
1 parent 8ce971d commit bfd48ce

File tree

2 files changed

+15
-11
lines changed

2 files changed

+15
-11
lines changed

rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ private CachedRouteLookupResponse asyncRlsCall(
303303
// on this result
304304
return CachedRouteLookupResponse.backoffEntry(createBackOffEntry(
305305
routeLookupRequestKey, Status.RESOURCE_EXHAUSTED.withDescription("RLS throttled"),
306-
backoffPolicy, routeLookupReason));
306+
backoffPolicy));
307307
}
308308
final SettableFuture<RouteLookupResponse> response = SettableFuture.create();
309309
io.grpc.lookup.v1.RouteLookupRequest routeLookupRequest = REQUEST_CONVERTER.convert(
@@ -417,7 +417,7 @@ private void pendingRpcComplete(PendingCacheEntry entry) {
417417
// reattempt picks when the child LB is done connecting
418418
} catch (Exception e) {
419419
createBackOffEntry(entry.routeLookupRequestKey, Status.fromThrowable(e),
420-
entry.backoffPolicy, entry.routeLookupReason);
420+
entry.backoffPolicy);
421421
// Cache updated. updateBalancingState() to reattempt picks
422422
helper.triggerPendingRpcProcessing();
423423
}
@@ -439,9 +439,8 @@ private DataCacheEntry createDataEntry(
439439
}
440440

441441
@GuardedBy("lock")
442-
private BackoffCacheEntry createBackOffEntry(
443-
RouteLookupRequestKey routeLookupRequestKey, Status status,
444-
@Nullable BackoffPolicy backoffPolicy, RouteLookupRequest.Reason routeLookupReason) {
442+
private BackoffCacheEntry createBackOffEntry(RouteLookupRequestKey routeLookupRequestKey,
443+
Status status, @Nullable BackoffPolicy backoffPolicy) {
445444
if (backoffPolicy == null) {
446445
backoffPolicy = backoffProvider.get();
447446
}
@@ -450,8 +449,7 @@ private BackoffCacheEntry createBackOffEntry(
450449
ChannelLogLevel.DEBUG,
451450
"[RLS Entry {0}] Transition to back off: status={1}, delayNanos={2}",
452451
routeLookupRequestKey, status, delayNanos);
453-
BackoffCacheEntry entry = new BackoffCacheEntry(routeLookupRequestKey, status, backoffPolicy,
454-
routeLookupReason);
452+
BackoffCacheEntry entry = new BackoffCacheEntry(routeLookupRequestKey, status, backoffPolicy);
455453
// Lock is held, so the task can't execute before the assignment
456454
entry.scheduledFuture = scheduledExecutorService.schedule(
457455
() -> refreshBackoffEntry(entry), delayNanos, TimeUnit.NANOSECONDS);
@@ -469,7 +467,7 @@ private void refreshBackoffEntry(BackoffCacheEntry entry) {
469467
logger.log(ChannelLogLevel.DEBUG,
470468
"[RLS Entry {0}] Calling RLS for transition to pending", entry.routeLookupRequestKey);
471469
linkedHashLruCache.invalidate(entry.routeLookupRequestKey);
472-
asyncRlsCall(entry.routeLookupRequestKey, entry.backoffPolicy, entry.routeLookupReason);
470+
asyncRlsCall(entry.routeLookupRequestKey, entry.backoffPolicy, RouteLookupRequest.Reason.REASON_MISS);
473471
}
474472
}
475473

@@ -779,15 +777,13 @@ private static final class BackoffCacheEntry extends CacheEntry {
779777

780778
private final Status status;
781779
private final BackoffPolicy backoffPolicy;
782-
private final RouteLookupRequest.Reason routeLookupReason;
783780
private Future<?> scheduledFuture;
784781

785782
BackoffCacheEntry(RouteLookupRequestKey routeLookupRequestKey, Status status,
786-
BackoffPolicy backoffPolicy, RouteLookupRequest.Reason routeLookupReason) {
783+
BackoffPolicy backoffPolicy) {
787784
super(routeLookupRequestKey);
788785
this.status = checkNotNull(status, "status");
789786
this.backoffPolicy = checkNotNull(backoffPolicy, "backoffPolicy");
790-
this.routeLookupReason = checkNotNull(routeLookupReason, "routeLookupReason");
791787
}
792788

793789
Status getStatus() {

rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,7 @@ public void get_noError_lifeCycle() throws Exception {
239239
// cache hit for staled entry
240240
fakeClock.forwardTime(ROUTE_LOOKUP_CONFIG.staleAgeInNanos(), TimeUnit.NANOSECONDS);
241241

242+
rlsServerImpl.routeLookupReason = null;
242243
resp = getInSyncContext(routeLookupRequestKey);
243244
assertThat(resp.hasData()).isTrue();
244245

@@ -250,6 +251,7 @@ public void get_noError_lifeCycle() throws Exception {
250251

251252
resp = getInSyncContext(routeLookupRequestKey);
252253

254+
assertThat(rlsServerImpl.routeLookupReason).isEqualTo(io.grpc.lookup.v1.RouteLookupRequest.Reason.REASON_STALE);
253255
assertThat(resp.hasData()).isTrue();
254256

255257
// existing cache expired
@@ -298,6 +300,7 @@ public void rls_withCustomRlsChannelServiceConfig() throws Exception {
298300
routeLookupRequestKey,
299301
RouteLookupResponse.create(ImmutableList.of("target"), "header")));
300302

303+
rlsServerImpl.routeLookupReason = null;
301304
// initial request
302305
CachedRouteLookupResponse resp = getInSyncContext(routeLookupRequestKey);
303306
assertThat(resp.isPending()).isTrue();
@@ -307,6 +310,7 @@ public void rls_withCustomRlsChannelServiceConfig() throws Exception {
307310

308311
resp = getInSyncContext(routeLookupRequestKey);
309312
assertThat(resp.hasData()).isTrue();
313+
assertThat(rlsServerImpl.routeLookupReason).isEqualTo(io.grpc.lookup.v1.RouteLookupRequest.Reason.REASON_MISS);
310314

311315
assertThat(rlsChannelOverriddenAuthority).isEqualTo("bigtable.googleapis.com:443");
312316
assertThat(rlsChannelServiceConfig).isEqualTo(routeLookupChannelServiceConfig);
@@ -348,8 +352,10 @@ public void get_throttledAndRecover() throws Exception {
348352

349353
assertThat(resp.isPending()).isTrue();
350354

355+
rlsServerImpl.routeLookupReason = null;
351356
// server responses
352357
fakeClock.forwardTime(SERVER_LATENCY_MILLIS, TimeUnit.MILLISECONDS);
358+
assertThat(rlsServerImpl.routeLookupReason).isEqualTo(io.grpc.lookup.v1.RouteLookupRequest.Reason.REASON_MISS);
353359

354360
resp = getInSyncContext(routeLookupRequestKey);
355361

@@ -881,6 +887,7 @@ private static final class StaticFixedDelayRlsServerImpl
881887

882888
private Map<RlsProtoData.RouteLookupRequestKey, RouteLookupResponse> lookupTable =
883889
ImmutableMap.of();
890+
io.grpc.lookup.v1.RouteLookupRequest.Reason routeLookupReason;
884891

885892
public StaticFixedDelayRlsServerImpl(
886893
long responseDelayNano, ScheduledExecutorService scheduledExecutorService) {
@@ -903,6 +910,7 @@ public void routeLookup(final io.grpc.lookup.v1.RouteLookupRequest request,
903910
new Runnable() {
904911
@Override
905912
public void run() {
913+
routeLookupReason = request.getReason();
906914
RouteLookupResponse response =
907915
lookupTable.get(
908916
RlsProtoData.RouteLookupRequestKey.create(

0 commit comments

Comments
 (0)