From 3e20e4e453872716b4b135fc788127e50a0f7eef Mon Sep 17 00:00:00 2001 From: Kannan J Date: Fri, 24 Oct 2025 06:18:57 +0000 Subject: [PATCH 1/9] Save changes. --- .../main/java/io/grpc/rls/RlsProtoConverters.java | 2 ++ rls/src/main/java/io/grpc/rls/RlsProtoData.java | 12 ++++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/rls/src/main/java/io/grpc/rls/RlsProtoConverters.java b/rls/src/main/java/io/grpc/rls/RlsProtoConverters.java index aa5147449c4..3e4bf89c660 100644 --- a/rls/src/main/java/io/grpc/rls/RlsProtoConverters.java +++ b/rls/src/main/java/io/grpc/rls/RlsProtoConverters.java @@ -64,6 +64,7 @@ static final class RouteLookupRequestConverter @Override protected RlsProtoData.RouteLookupRequest doForward(RouteLookupRequest routeLookupRequest) { return RlsProtoData.RouteLookupRequest.create( + RlsProtoData.RouteLookupRequest.Reason.valueOf(routeLookupRequest.getReason().name()), ImmutableMap.copyOf(routeLookupRequest.getKeyMapMap())); } @@ -72,6 +73,7 @@ protected RouteLookupRequest doBackward(RlsProtoData.RouteLookupRequest routeLoo return RouteLookupRequest.newBuilder() .setTargetType("grpc") + .setReason(RouteLookupRequest.Reason.valueOf(routeLookupRequest.reason().name())) .putAllKeyMap(routeLookupRequest.keyMap()) .build(); } diff --git a/rls/src/main/java/io/grpc/rls/RlsProtoData.java b/rls/src/main/java/io/grpc/rls/RlsProtoData.java index 49f32c6b6e3..1c58c4d3091 100644 --- a/rls/src/main/java/io/grpc/rls/RlsProtoData.java +++ b/rls/src/main/java/io/grpc/rls/RlsProtoData.java @@ -32,11 +32,19 @@ private RlsProtoData() {} @Immutable abstract static class RouteLookupRequest { + // Names should match those in grpc.lookup.v1.RouteLookupRequest.Reason + enum Reason { + REASON_UNKNOWN, // Unused + REASON_MISS, // No data available in local cache + REASON_STALE; // Data in local cache is stale + } + // Reason for making this request. + abstract Reason reason(); /** Returns a map of key values extracted via key builders for the gRPC or HTTP request. */ abstract ImmutableMap keyMap(); - static RouteLookupRequest create(ImmutableMap keyMap) { - return new AutoValue_RlsProtoData_RouteLookupRequest(keyMap); + static RouteLookupRequest create(Reason reason, ImmutableMap keyMap) { + return new AutoValue_RlsProtoData_RouteLookupRequest(reason, keyMap); } } From 3c0f873ca87c2e0757667348537e2a94f421071f Mon Sep 17 00:00:00 2001 From: Kannan J Date: Fri, 24 Oct 2025 14:08:14 +0000 Subject: [PATCH 2/9] Add RLS request lookup reason. --- .../java/io/grpc/rls/CachingRlsLbClient.java | 146 ++++++++------- .../java/io/grpc/rls/RlsProtoConverters.java | 5 +- .../main/java/io/grpc/rls/RlsProtoData.java | 17 +- .../java/io/grpc/rls/RlsRequestFactory.java | 18 +- .../io/grpc/rls/CachingRlsLbClientTest.java | 170 ++++++++++-------- .../java/io/grpc/rls/RlsLoadBalancerTest.java | 12 +- .../io/grpc/rls/RlsProtoConvertersTest.java | 4 +- .../io/grpc/rls/RlsRequestFactoryTest.java | 26 +-- 8 files changed, 239 insertions(+), 159 deletions(-) diff --git a/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java b/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java index cc3ac9f516e..c60a2304290 100644 --- a/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java +++ b/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java @@ -61,6 +61,7 @@ import io.grpc.rls.RlsProtoConverters.RouteLookupResponseConverter; import io.grpc.rls.RlsProtoData.RouteLookupConfig; import io.grpc.rls.RlsProtoData.RouteLookupRequest; +import io.grpc.rls.RlsProtoData.RouteLookupRequestKey; import io.grpc.rls.RlsProtoData.RouteLookupResponse; import io.grpc.stub.StreamObserver; import io.grpc.util.ForwardingLoadBalancerHelper; @@ -112,7 +113,7 @@ final class CachingRlsLbClient { private final Future periodicCleaner; // any RPC on the fly will cached in this map @GuardedBy("lock") - private final Map pendingCallCache = new HashMap<>(); + private final Map pendingCallCache = new HashMap<>(); private final ScheduledExecutorService scheduledExecutorService; private final Ticker ticker; @@ -127,6 +128,7 @@ final class CachingRlsLbClient { private final RlsLbHelper helper; private final ManagedChannel rlsChannel; private final RouteLookupServiceStub rlsStub; + private final RlsRequestFactory requestFactory; private final RlsPicker rlsPicker; private final ResolvedAddressFactory childLbResolvedAddressFactory; @GuardedBy("lock") @@ -195,7 +197,7 @@ private CachingRlsLbClient(Builder builder) { ChannelLogLevel.DEBUG, "Can not get hostname from authority: {0}", helper.getAuthority()); serverHost = helper.getAuthority(); } - RlsRequestFactory requestFactory = new RlsRequestFactory( + requestFactory = new RlsRequestFactory( lbPolicyConfig.getRouteLookupConfig(), serverHost); rlsPicker = new RlsPicker(requestFactory, rlsConfig.lookupService()); // It is safe to use helper.getUnsafeChannelCredentials() because the client authenticates the @@ -292,18 +294,22 @@ private void periodicClean() { /** Populates async cache entry for new request. */ @GuardedBy("lock") private CachedRouteLookupResponse asyncRlsCall( - RouteLookupRequest request, @Nullable BackoffPolicy backoffPolicy) { + RouteLookupRequestKey routeLookupRequestKey, @Nullable BackoffPolicy backoffPolicy, + RouteLookupRequest.Reason routeLookupReason) { if (throttler.shouldThrottle()) { - logger.log(ChannelLogLevel.DEBUG, "[RLS Entry {0}] Throttled RouteLookup", request); + logger.log(ChannelLogLevel.DEBUG, "[RLS Entry {0}] Throttled RouteLookup", + routeLookupRequestKey); // Cache updated, but no need to call updateBalancingState because no RPCs were queued waiting // on this result return CachedRouteLookupResponse.backoffEntry(createBackOffEntry( - request, Status.RESOURCE_EXHAUSTED.withDescription("RLS throttled"), backoffPolicy)); + routeLookupRequestKey, Status.RESOURCE_EXHAUSTED.withDescription("RLS throttled"), + backoffPolicy, routeLookupReason)); } final SettableFuture response = SettableFuture.create(); - io.grpc.lookup.v1.RouteLookupRequest routeLookupRequest = REQUEST_CONVERTER.convert(request); + io.grpc.lookup.v1.RouteLookupRequest routeLookupRequest = REQUEST_CONVERTER.convert( + requestFactory.create(routeLookupRequestKey, routeLookupReason)); logger.log(ChannelLogLevel.DEBUG, - "[RLS Entry {0}] Starting RouteLookup: {1}", request, routeLookupRequest); + "[RLS Entry {0}] Starting RouteLookup: {1}", routeLookupRequestKey, routeLookupRequest); rlsStub.withDeadlineAfter(callTimeoutNanos, TimeUnit.NANOSECONDS) .routeLookup( routeLookupRequest, @@ -311,14 +317,14 @@ private CachedRouteLookupResponse asyncRlsCall( @Override public void onNext(io.grpc.lookup.v1.RouteLookupResponse value) { logger.log(ChannelLogLevel.DEBUG, - "[RLS Entry {0}] RouteLookup succeeded: {1}", request, value); + "[RLS Entry {0}] RouteLookup succeeded: {1}", routeLookupRequestKey, value); response.set(RESPONSE_CONVERTER.reverse().convert(value)); } @Override public void onError(Throwable t) { logger.log(ChannelLogLevel.DEBUG, - "[RLS Entry {0}] RouteLookup failed: {1}", request, t); + "[RLS Entry {0}] RouteLookup failed: {1}", routeLookupRequestKey, t); response.setException(t); throttler.registerBackendResponse(true); } @@ -329,7 +335,7 @@ public void onCompleted() { } }); return CachedRouteLookupResponse.pendingResponse( - createPendingEntry(request, response, backoffPolicy)); + createPendingEntry(routeLookupRequestKey, response, backoffPolicy, routeLookupReason)); } /** @@ -338,16 +344,17 @@ public void onCompleted() { * changed after the return. */ @CheckReturnValue - final CachedRouteLookupResponse get(final RouteLookupRequest request) { + final CachedRouteLookupResponse get(final RouteLookupRequestKey routeLookupRequestKey) { synchronized (lock) { final CacheEntry cacheEntry; - cacheEntry = linkedHashLruCache.read(request); + cacheEntry = linkedHashLruCache.read(routeLookupRequestKey); if (cacheEntry == null) { - PendingCacheEntry pendingEntry = pendingCallCache.get(request); + PendingCacheEntry pendingEntry = pendingCallCache.get(routeLookupRequestKey); if (pendingEntry != null) { return CachedRouteLookupResponse.pendingResponse(pendingEntry); } - return asyncRlsCall(request, /* backoffPolicy= */ null); + return asyncRlsCall(routeLookupRequestKey, /* backoffPolicy= */ null, + RouteLookupRequest.Reason.REASON_MISS); } if (cacheEntry instanceof DataCacheEntry) { @@ -383,13 +390,15 @@ void requestConnection() { @GuardedBy("lock") private PendingCacheEntry createPendingEntry( - RouteLookupRequest request, + RouteLookupRequestKey routeLookupRequestKey, ListenableFuture pendingCall, - @Nullable BackoffPolicy backoffPolicy) { - PendingCacheEntry entry = new PendingCacheEntry(request, pendingCall, backoffPolicy); + @Nullable BackoffPolicy backoffPolicy, + RouteLookupRequest.Reason routeLookupReason) { + PendingCacheEntry entry = new PendingCacheEntry(routeLookupRequestKey, pendingCall, + backoffPolicy, routeLookupReason); // Add the entry to the map before adding the Listener, because the listener removes the // entry from the map - pendingCallCache.put(request, entry); + pendingCallCache.put(routeLookupRequestKey, entry); // Beware that the listener can run immediately on the current thread pendingCall.addListener(() -> pendingRpcComplete(entry), MoreExecutors.directExecutor()); return entry; @@ -397,17 +406,18 @@ private PendingCacheEntry createPendingEntry( private void pendingRpcComplete(PendingCacheEntry entry) { synchronized (lock) { - boolean clientClosed = pendingCallCache.remove(entry.request) == null; + boolean clientClosed = pendingCallCache.remove(entry.routeLookupRequestKey) == null; if (clientClosed) { return; } try { - createDataEntry(entry.request, Futures.getDone(entry.pendingCall)); + createDataEntry(entry.routeLookupRequestKey, Futures.getDone(entry.pendingCall)); // Cache updated. DataCacheEntry constructor indirectly calls updateBalancingState() to // reattempt picks when the child LB is done connecting } catch (Exception e) { - createBackOffEntry(entry.request, Status.fromThrowable(e), entry.backoffPolicy); + createBackOffEntry(entry.routeLookupRequestKey, Status.fromThrowable(e), + entry.backoffPolicy, entry.routeLookupReason); // Cache updated. updateBalancingState() to reattempt picks helper.triggerPendingRpcProcessing(); } @@ -416,21 +426,22 @@ private void pendingRpcComplete(PendingCacheEntry entry) { @GuardedBy("lock") private DataCacheEntry createDataEntry( - RouteLookupRequest request, RouteLookupResponse routeLookupResponse) { + RouteLookupRequestKey routeLookupRequestKey, RouteLookupResponse routeLookupResponse) { logger.log( ChannelLogLevel.DEBUG, "[RLS Entry {0}] Transition to data cache: routeLookupResponse={1}", - request, routeLookupResponse); - DataCacheEntry entry = new DataCacheEntry(request, routeLookupResponse); + routeLookupRequestKey, routeLookupResponse); + DataCacheEntry entry = new DataCacheEntry(routeLookupRequestKey, routeLookupResponse); // Constructor for DataCacheEntry causes updateBalancingState, but the picks can't happen until // this cache update because the lock is held - linkedHashLruCache.cacheAndClean(request, entry); + linkedHashLruCache.cacheAndClean(routeLookupRequestKey, entry); return entry; } @GuardedBy("lock") private BackoffCacheEntry createBackOffEntry( - RouteLookupRequest request, Status status, @Nullable BackoffPolicy backoffPolicy) { + RouteLookupRequestKey routeLookupRequestKey, Status status, + @Nullable BackoffPolicy backoffPolicy, RouteLookupRequest.Reason routeLookupReason) { if (backoffPolicy == null) { backoffPolicy = backoffProvider.get(); } @@ -438,12 +449,13 @@ private BackoffCacheEntry createBackOffEntry( logger.log( ChannelLogLevel.DEBUG, "[RLS Entry {0}] Transition to back off: status={1}, delayNanos={2}", - request, status, delayNanos); - BackoffCacheEntry entry = new BackoffCacheEntry(request, status, backoffPolicy); + routeLookupRequestKey, status, delayNanos); + BackoffCacheEntry entry = new BackoffCacheEntry(routeLookupRequestKey, status, backoffPolicy, + routeLookupReason); // Lock is held, so the task can't execute before the assignment entry.scheduledFuture = scheduledExecutorService.schedule( () -> refreshBackoffEntry(entry), delayNanos, TimeUnit.NANOSECONDS); - linkedHashLruCache.cacheAndClean(request, entry); + linkedHashLruCache.cacheAndClean(routeLookupRequestKey, entry); return entry; } @@ -455,9 +467,9 @@ private void refreshBackoffEntry(BackoffCacheEntry entry) { return; } logger.log(ChannelLogLevel.DEBUG, - "[RLS Entry {0}] Calling RLS for transition to pending", entry.request); - linkedHashLruCache.invalidate(entry.request); - asyncRlsCall(entry.request, entry.backoffPolicy); + "[RLS Entry {0}] Calling RLS for transition to pending", entry.routeLookupRequestKey); + linkedHashLruCache.invalidate(entry.routeLookupRequestKey); + asyncRlsCall(entry.routeLookupRequestKey, entry.backoffPolicy, entry.routeLookupReason); } } @@ -590,23 +602,26 @@ public String toString() { /** A pending cache entry when the async RouteLookup RPC is still on the fly. */ static final class PendingCacheEntry { private final ListenableFuture pendingCall; - private final RouteLookupRequest request; + private final RouteLookupRequestKey routeLookupRequestKey; @Nullable private final BackoffPolicy backoffPolicy; + private final RouteLookupRequest.Reason routeLookupReason; PendingCacheEntry( - RouteLookupRequest request, + RouteLookupRequestKey routeLookupRequestKey, ListenableFuture pendingCall, - @Nullable BackoffPolicy backoffPolicy) { - this.request = checkNotNull(request, "request"); + @Nullable BackoffPolicy backoffPolicy, RouteLookupRequest.Reason routeLookupReason) { + this.routeLookupRequestKey = checkNotNull(routeLookupRequestKey, "request"); this.pendingCall = checkNotNull(pendingCall, "pendingCall"); this.backoffPolicy = backoffPolicy; + this.routeLookupReason = routeLookupReason; } @Override public String toString() { return MoreObjects.toStringHelper(this) - .add("request", request) + .add("routeLookupRequestKey", routeLookupRequestKey) + .add("routeLookupReason", routeLookupReason) .toString(); } } @@ -614,10 +629,10 @@ public String toString() { /** Common cache entry data for {@link RlsAsyncLruCache}. */ abstract static class CacheEntry { - protected final RouteLookupRequest request; + protected final RouteLookupRequestKey routeLookupRequestKey; - CacheEntry(RouteLookupRequest request) { - this.request = checkNotNull(request, "request"); + CacheEntry(RouteLookupRequestKey routeLookupRequestKey) { + this.routeLookupRequestKey = checkNotNull(routeLookupRequestKey, "request"); } abstract int getSizeBytes(); @@ -640,8 +655,9 @@ final class DataCacheEntry extends CacheEntry { private final List childPolicyWrappers; // GuardedBy CachingRlsLbClient.lock - DataCacheEntry(RouteLookupRequest request, final RouteLookupResponse response) { - super(request); + DataCacheEntry(RouteLookupRequestKey routeLookupRequestKey, + final RouteLookupResponse response) { + super(routeLookupRequestKey); this.response = checkNotNull(response, "response"); checkState(!response.targets().isEmpty(), "No targets returned by RLS"); childPolicyWrappers = @@ -669,13 +685,14 @@ final class DataCacheEntry extends CacheEntry { */ void maybeRefresh() { synchronized (lock) { // Lock is already held, but ErrorProne can't tell - if (pendingCallCache.containsKey(request)) { + if (pendingCallCache.containsKey(routeLookupRequestKey)) { // pending already requested return; } logger.log(ChannelLogLevel.DEBUG, - "[RLS Entry {0}] Cache entry is stale, refreshing", request); - asyncRlsCall(request, /* backoffPolicy= */ null); + "[RLS Entry {0}] Cache entry is stale, refreshing", routeLookupRequestKey); + asyncRlsCall(routeLookupRequestKey, /* backoffPolicy= */ null, + RouteLookupRequest.Reason.REASON_STALE); } } @@ -745,7 +762,7 @@ void cleanup() { @Override public String toString() { return MoreObjects.toStringHelper(this) - .add("request", request) + .add("request", routeLookupRequestKey) .add("response", response) .add("expireTime", expireTime) .add("staleTime", staleTime) @@ -762,12 +779,15 @@ private static final class BackoffCacheEntry extends CacheEntry { private final Status status; private final BackoffPolicy backoffPolicy; + private final RouteLookupRequest.Reason routeLookupReason; private Future scheduledFuture; - BackoffCacheEntry(RouteLookupRequest request, Status status, BackoffPolicy backoffPolicy) { - super(request); + BackoffCacheEntry(RouteLookupRequestKey routeLookupRequestKey, Status status, + BackoffPolicy backoffPolicy, RouteLookupRequest.Reason routeLookupReason) { + super(routeLookupRequestKey); this.status = checkNotNull(status, "status"); this.backoffPolicy = checkNotNull(backoffPolicy, "backoffPolicy"); + this.routeLookupReason = checkNotNull(routeLookupReason, "routeLookupReason"); } Status getStatus() { @@ -792,7 +812,7 @@ void cleanup() { @Override public String toString() { return MoreObjects.toStringHelper(this) - .add("request", request) + .add("request", routeLookupRequestKey) .add("status", status) .toString(); } @@ -811,7 +831,7 @@ static final class Builder { private Throttler throttler = new HappyThrottler(); private ResolvedAddressFactory resolvedAddressFactory; private Ticker ticker = Ticker.systemTicker(); - private EvictionListener evictionListener; + private EvictionListener evictionListener; private BackoffPolicy.Provider backoffProvider = new ExponentialBackoffPolicy.Provider(); Builder setHelper(Helper helper) { @@ -845,7 +865,7 @@ Builder setTicker(Ticker ticker) { } Builder setEvictionListener( - @Nullable EvictionListener evictionListener) { + @Nullable EvictionListener evictionListener) { this.evictionListener = evictionListener; return this; } @@ -867,17 +887,17 @@ CachingRlsLbClient build() { * CacheEntry#cleanup()} after original {@link EvictionListener} is finished. */ private static final class AutoCleaningEvictionListener - implements EvictionListener { + implements EvictionListener { - private final EvictionListener delegate; + private final EvictionListener delegate; AutoCleaningEvictionListener( - @Nullable EvictionListener delegate) { + @Nullable EvictionListener delegate) { this.delegate = delegate; } @Override - public void onEviction(RouteLookupRequest key, CacheEntry value, EvictionType cause) { + public void onEviction(RouteLookupRequestKey key, CacheEntry value, EvictionType cause) { if (delegate != null) { delegate.onEviction(key, value, cause); } @@ -902,29 +922,29 @@ public void registerBackendResponse(boolean throttled) { /** Implementation of {@link LinkedHashLruCache} for RLS. */ private static final class RlsAsyncLruCache - extends LinkedHashLruCache { + extends LinkedHashLruCache { private final RlsLbHelper helper; RlsAsyncLruCache(long maxEstimatedSizeBytes, - @Nullable EvictionListener evictionListener, + @Nullable EvictionListener evictionListener, Ticker ticker, RlsLbHelper helper) { super(maxEstimatedSizeBytes, evictionListener, ticker); this.helper = checkNotNull(helper, "helper"); } @Override - protected boolean isExpired(RouteLookupRequest key, CacheEntry value, long nowNanos) { + protected boolean isExpired(RouteLookupRequestKey key, CacheEntry value, long nowNanos) { return value.isExpired(nowNanos); } @Override - protected int estimateSizeOf(RouteLookupRequest key, CacheEntry value) { + protected int estimateSizeOf(RouteLookupRequestKey key, CacheEntry value) { return value.getSizeBytes(); } @Override protected boolean shouldInvalidateEldestEntry( - RouteLookupRequest eldestKey, CacheEntry eldestValue, long now) { + RouteLookupRequestKey eldestKey, CacheEntry eldestValue, long now) { if (!eldestValue.isOldEnoughToBeEvicted(now)) { return false; } @@ -933,7 +953,7 @@ protected boolean shouldInvalidateEldestEntry( return this.estimatedSizeBytes() > this.estimatedMaxSizeBytes(); } - public CacheEntry cacheAndClean(RouteLookupRequest key, CacheEntry value) { + public CacheEntry cacheAndClean(RouteLookupRequestKey key, CacheEntry value) { CacheEntry newEntry = cache(key, value); // force cleanup if new entry pushed cache over max size (in bytes) @@ -989,9 +1009,9 @@ final class RlsPicker extends SubchannelPicker { public PickResult pickSubchannel(PickSubchannelArgs args) { String serviceName = args.getMethodDescriptor().getServiceName(); String methodName = args.getMethodDescriptor().getBareMethodName(); - RouteLookupRequest request = + RlsProtoData.RouteLookupRequestKey lookupRequestKey = requestFactory.create(serviceName, methodName, args.getHeaders()); - final CachedRouteLookupResponse response = CachingRlsLbClient.this.get(request); + final CachedRouteLookupResponse response = CachingRlsLbClient.this.get(lookupRequestKey); if (response.getHeaderData() != null && !response.getHeaderData().isEmpty()) { Metadata headers = args.getHeaders(); diff --git a/rls/src/main/java/io/grpc/rls/RlsProtoConverters.java b/rls/src/main/java/io/grpc/rls/RlsProtoConverters.java index 3e4bf89c660..70f9fb4d891 100644 --- a/rls/src/main/java/io/grpc/rls/RlsProtoConverters.java +++ b/rls/src/main/java/io/grpc/rls/RlsProtoConverters.java @@ -64,8 +64,9 @@ static final class RouteLookupRequestConverter @Override protected RlsProtoData.RouteLookupRequest doForward(RouteLookupRequest routeLookupRequest) { return RlsProtoData.RouteLookupRequest.create( - RlsProtoData.RouteLookupRequest.Reason.valueOf(routeLookupRequest.getReason().name()), - ImmutableMap.copyOf(routeLookupRequest.getKeyMapMap())); + ImmutableMap.copyOf(routeLookupRequest.getKeyMapMap()), + RlsProtoData.RouteLookupRequest.Reason.valueOf(routeLookupRequest.getReason().name()) + ); } @Override diff --git a/rls/src/main/java/io/grpc/rls/RlsProtoData.java b/rls/src/main/java/io/grpc/rls/RlsProtoData.java index 1c58c4d3091..1c06e7dd23e 100644 --- a/rls/src/main/java/io/grpc/rls/RlsProtoData.java +++ b/rls/src/main/java/io/grpc/rls/RlsProtoData.java @@ -27,6 +27,19 @@ final class RlsProtoData { private RlsProtoData() {} + /** A key object for the Rls route lookup data cache. */ + @AutoValue + @Immutable + abstract static class RouteLookupRequestKey { + + /** Returns a map of key values extracted via key builders for the gRPC or HTTP request. */ + abstract ImmutableMap keyMap(); + + static RouteLookupRequestKey create(ImmutableMap keyMap) { + return new AutoValue_RlsProtoData_RouteLookupRequestKey(keyMap); + } + } + /** A request object sent to route lookup service. */ @AutoValue @Immutable @@ -38,12 +51,14 @@ enum Reason { REASON_MISS, // No data available in local cache REASON_STALE; // Data in local cache is stale } + // Reason for making this request. abstract Reason reason(); + /** Returns a map of key values extracted via key builders for the gRPC or HTTP request. */ abstract ImmutableMap keyMap(); - static RouteLookupRequest create(Reason reason, ImmutableMap keyMap) { + static RouteLookupRequest create(ImmutableMap keyMap, Reason reason) { return new AutoValue_RlsProtoData_RouteLookupRequest(reason, keyMap); } } diff --git a/rls/src/main/java/io/grpc/rls/RlsRequestFactory.java b/rls/src/main/java/io/grpc/rls/RlsRequestFactory.java index e26e49979e1..82a1d98f5bf 100644 --- a/rls/src/main/java/io/grpc/rls/RlsRequestFactory.java +++ b/rls/src/main/java/io/grpc/rls/RlsRequestFactory.java @@ -28,12 +28,13 @@ import io.grpc.rls.RlsProtoData.NameMatcher; import io.grpc.rls.RlsProtoData.RouteLookupConfig; import io.grpc.rls.RlsProtoData.RouteLookupRequest; +import io.grpc.rls.RlsProtoData.RouteLookupRequestKey; import java.util.HashMap; import java.util.List; import java.util.Map; /** - * A RlsRequestFactory creates {@link RouteLookupRequest} using key builder map from {@link + * A RlsRequestFactory creates {@link RouteLookupRequestKey} using key builder map from {@link * RouteLookupConfig}. */ final class RlsRequestFactory { @@ -61,9 +62,9 @@ private static Map createKeyBuilderTable( return table; } - /** Creates a {@link RouteLookupRequest} for given request's metadata. */ + /** Creates a {@link RouteLookupRequestKey} for the given request lookup metadata. */ @CheckReturnValue - RouteLookupRequest create(String service, String method, Metadata metadata) { + RouteLookupRequestKey create(String service, String method, Metadata metadata) { checkNotNull(service, "service"); checkNotNull(method, "method"); String path = "/" + service + "/" + method; @@ -73,7 +74,7 @@ RouteLookupRequest create(String service, String method, Metadata metadata) { grpcKeyBuilder = keyBuilderTable.get("/" + service + "/*"); } if (grpcKeyBuilder == null) { - return RouteLookupRequest.create(ImmutableMap.of()); + return RouteLookupRequestKey.create(ImmutableMap.of()); } ImmutableMap.Builder rlsRequestHeaders = createRequestHeaders(metadata, grpcKeyBuilder.headers()); @@ -89,7 +90,14 @@ RouteLookupRequest create(String service, String method, Metadata metadata) { rlsRequestHeaders.put(extraKeys.method(), method); } rlsRequestHeaders.putAll(constantKeys); - return RouteLookupRequest.create(rlsRequestHeaders.buildOrThrow()); + return RouteLookupRequestKey.create(rlsRequestHeaders.buildOrThrow()); + } + + /** Creates a {@link RouteLookupRequest} using the given request lookup key and reason. */ + @CheckReturnValue + RouteLookupRequest create(RouteLookupRequestKey routeLookupRequestKey, + RouteLookupRequest.Reason reason) { + return RouteLookupRequest.create(routeLookupRequestKey.keyMap(), reason); } private ImmutableMap.Builder createRequestHeaders( diff --git a/rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java b/rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java index 4f086abc4a2..9b8d59be8d8 100644 --- a/rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java +++ b/rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java @@ -128,7 +128,7 @@ public class CachingRlsLbClientTest { public final GrpcCleanupRule grpcCleanupRule = new GrpcCleanupRule(); @Mock - private EvictionListener evictionListener; + private EvictionListener evictionListener; @Mock private SocketAddress socketAddress; @Mock @@ -200,14 +200,14 @@ public void tearDown() throws Exception { } private CachedRouteLookupResponse getInSyncContext( - final RouteLookupRequest request) + final RlsProtoData.RouteLookupRequestKey routeLookupRequestKey) throws ExecutionException, InterruptedException, TimeoutException { final SettableFuture responseSettableFuture = SettableFuture.create(); syncContext.execute(new Runnable() { @Override public void run() { - responseSettableFuture.set(rlsLbClient.get(request)); + responseSettableFuture.set(rlsLbClient.get(routeLookupRequestKey)); } }); return responseSettableFuture.get(5, TimeUnit.SECONDS); @@ -217,48 +217,50 @@ public void run() { public void get_noError_lifeCycle() throws Exception { setUpRlsLbClient(); InOrder inOrder = inOrder(evictionListener); - RouteLookupRequest routeLookupRequest = RouteLookupRequest.create(ImmutableMap.of( - "server", "bigtable.googleapis.com", "service-key", "foo", "method-key", "bar")); + RlsProtoData.RouteLookupRequestKey routeLookupRequestKey = + RlsProtoData.RouteLookupRequestKey.create( + ImmutableMap.of( + "server", "bigtable.googleapis.com", "service-key", "foo", "method-key", "bar")); rlsServerImpl.setLookupTable( ImmutableMap.of( - routeLookupRequest, + routeLookupRequestKey, RouteLookupResponse.create(ImmutableList.of("target"), "header"))); // initial request - CachedRouteLookupResponse resp = getInSyncContext(routeLookupRequest); + CachedRouteLookupResponse resp = getInSyncContext(routeLookupRequestKey); assertThat(resp.isPending()).isTrue(); // server response fakeClock.forwardTime(SERVER_LATENCY_MILLIS, TimeUnit.MILLISECONDS); - resp = getInSyncContext(routeLookupRequest); + resp = getInSyncContext(routeLookupRequestKey); assertThat(resp.hasData()).isTrue(); // cache hit for staled entry fakeClock.forwardTime(ROUTE_LOOKUP_CONFIG.staleAgeInNanos(), TimeUnit.NANOSECONDS); - resp = getInSyncContext(routeLookupRequest); + resp = getInSyncContext(routeLookupRequestKey); assertThat(resp.hasData()).isTrue(); // async refresh finishes fakeClock.forwardTime(SERVER_LATENCY_MILLIS, TimeUnit.MILLISECONDS); inOrder .verify(evictionListener) - .onEviction(eq(routeLookupRequest), any(CacheEntry.class), eq(EvictionType.REPLACED)); + .onEviction(eq(routeLookupRequestKey), any(CacheEntry.class), eq(EvictionType.REPLACED)); - resp = getInSyncContext(routeLookupRequest); + resp = getInSyncContext(routeLookupRequestKey); assertThat(resp.hasData()).isTrue(); // existing cache expired fakeClock.forwardTime(ROUTE_LOOKUP_CONFIG.maxAgeInNanos(), TimeUnit.NANOSECONDS); - resp = getInSyncContext(routeLookupRequest); + resp = getInSyncContext(routeLookupRequestKey); assertThat(resp.isPending()).isTrue(); inOrder .verify(evictionListener) - .onEviction(eq(routeLookupRequest), any(CacheEntry.class), eq(EvictionType.EXPIRED)); + .onEviction(eq(routeLookupRequestKey), any(CacheEntry.class), eq(EvictionType.EXPIRED)); inOrder.verifyNoMoreInteractions(); } @@ -287,21 +289,23 @@ public void rls_withCustomRlsChannelServiceConfig() throws Exception { .setThrottler(fakeThrottler) .setTicker(fakeClock.getTicker()) .build(); - RouteLookupRequest routeLookupRequest = RouteLookupRequest.create(ImmutableMap.of( - "server", "bigtable.googleapis.com", "service-key", "foo", "method-key", "bar")); + RlsProtoData.RouteLookupRequestKey routeLookupRequestKey = + RlsProtoData.RouteLookupRequestKey.create( + ImmutableMap.of( + "server", "bigtable.googleapis.com", "service-key", "foo", "method-key", "bar")); rlsServerImpl.setLookupTable( ImmutableMap.of( - routeLookupRequest, + routeLookupRequestKey, RouteLookupResponse.create(ImmutableList.of("target"), "header"))); // initial request - CachedRouteLookupResponse resp = getInSyncContext(routeLookupRequest); + CachedRouteLookupResponse resp = getInSyncContext(routeLookupRequestKey); assertThat(resp.isPending()).isTrue(); // server response fakeClock.forwardTime(SERVER_LATENCY_MILLIS, TimeUnit.MILLISECONDS); - resp = getInSyncContext(routeLookupRequest); + resp = getInSyncContext(routeLookupRequestKey); assertThat(resp.hasData()).isTrue(); assertThat(rlsChannelOverriddenAuthority).isEqualTo("bigtable.googleapis.com:443"); @@ -311,26 +315,28 @@ public void rls_withCustomRlsChannelServiceConfig() throws Exception { @Test public void get_throttledAndRecover() throws Exception { setUpRlsLbClient(); - RouteLookupRequest routeLookupRequest = RouteLookupRequest.create(ImmutableMap.of( - "server", "bigtable.googleapis.com", "service-key", "foo", "method-key", "bar")); + RlsProtoData.RouteLookupRequestKey routeLookupRequestKey = + RlsProtoData.RouteLookupRequestKey.create( + ImmutableMap.of( + "server", "bigtable.googleapis.com", "service-key", "foo", "method-key", "bar")); rlsServerImpl.setLookupTable( ImmutableMap.of( - routeLookupRequest, + routeLookupRequestKey, RouteLookupResponse.create(ImmutableList.of("target"), "header"))); fakeThrottler.nextResult = true; fakeBackoffProvider.nextPolicy = createBackoffPolicy(10, TimeUnit.MILLISECONDS); - CachedRouteLookupResponse resp = getInSyncContext(routeLookupRequest); + CachedRouteLookupResponse resp = getInSyncContext(routeLookupRequestKey); assertThat(resp.hasError()).isTrue(); fakeClock.forwardTime(10, TimeUnit.MILLISECONDS); // initially backed off entry is backed off again verify(evictionListener) - .onEviction(eq(routeLookupRequest), any(CacheEntry.class), eq(EvictionType.EXPLICIT)); + .onEviction(eq(routeLookupRequestKey), any(CacheEntry.class), eq(EvictionType.EXPLICIT)); - resp = getInSyncContext(routeLookupRequest); + resp = getInSyncContext(routeLookupRequestKey); assertThat(resp.hasError()).isTrue(); @@ -338,14 +344,14 @@ public void get_throttledAndRecover() throws Exception { fakeThrottler.nextResult = false; fakeClock.forwardTime(10, TimeUnit.MILLISECONDS); - resp = getInSyncContext(routeLookupRequest); + resp = getInSyncContext(routeLookupRequestKey); assertThat(resp.isPending()).isTrue(); // server responses fakeClock.forwardTime(SERVER_LATENCY_MILLIS, TimeUnit.MILLISECONDS); - resp = getInSyncContext(routeLookupRequest); + resp = getInSyncContext(routeLookupRequestKey); assertThat(resp.hasData()).isTrue(); } @@ -354,21 +360,24 @@ public void get_throttledAndRecover() throws Exception { public void get_updatesLbState() throws Exception { setUpRlsLbClient(); InOrder inOrder = inOrder(helper); - RouteLookupRequest routeLookupRequest = RouteLookupRequest.create(ImmutableMap.of( - "server", "bigtable.googleapis.com", "service-key", "service1", "method-key", "create")); + RlsProtoData.RouteLookupRequestKey routeLookupRequestKey = + RlsProtoData.RouteLookupRequestKey.create( + ImmutableMap.of( + "server", "bigtable.googleapis.com", "service-key", "service1", + "method-key", "create")); rlsServerImpl.setLookupTable( ImmutableMap.of( - routeLookupRequest, + routeLookupRequestKey, RouteLookupResponse.create( ImmutableList.of("primary.cloudbigtable.googleapis.com"), "header-rls-data-value"))); // valid channel - CachedRouteLookupResponse resp = getInSyncContext(routeLookupRequest); + CachedRouteLookupResponse resp = getInSyncContext(routeLookupRequestKey); assertThat(resp.isPending()).isTrue(); fakeClock.forwardTime(SERVER_LATENCY_MILLIS, TimeUnit.MILLISECONDS); - resp = getInSyncContext(routeLookupRequest); + resp = getInSyncContext(routeLookupRequestKey); assertThat(resp.hasData()).isTrue(); ArgumentCaptor pickerCaptor = ArgumentCaptor.forClass(SubchannelPicker.class); @@ -393,13 +402,13 @@ public void get_updatesLbState() throws Exception { // move backoff further back to only test error behavior fakeBackoffProvider.nextPolicy = createBackoffPolicy(100, TimeUnit.MILLISECONDS); // try to get invalid - RouteLookupRequest invalidRouteLookupRequest = - RouteLookupRequest.create(ImmutableMap.of()); - CachedRouteLookupResponse errorResp = getInSyncContext(invalidRouteLookupRequest); + RlsProtoData.RouteLookupRequestKey invalidRouteLookupRequestKey = + RlsProtoData.RouteLookupRequestKey.create(ImmutableMap.of()); + CachedRouteLookupResponse errorResp = getInSyncContext(invalidRouteLookupRequestKey); assertThat(errorResp.isPending()).isTrue(); fakeClock.forwardTime(SERVER_LATENCY_MILLIS, TimeUnit.MILLISECONDS); - errorResp = getInSyncContext(invalidRouteLookupRequest); + errorResp = getInSyncContext(invalidRouteLookupRequestKey); assertThat(errorResp.hasError()).isTrue(); // Channel is still READY because the subchannel for method /service1/create is still READY. @@ -423,21 +432,24 @@ public void get_updatesLbState() throws Exception { @Test public void timeout_not_changing_picked_subchannel() throws Exception { setUpRlsLbClient(); - RouteLookupRequest routeLookupRequest = RouteLookupRequest.create(ImmutableMap.of( - "server", "bigtable.googleapis.com", "service-key", "service1", "method-key", "create")); + RlsProtoData.RouteLookupRequestKey routeLookupRequestKey = + RlsProtoData.RouteLookupRequestKey.create( + ImmutableMap.of( + "server", "bigtable.googleapis.com", "service-key", "service1", + "method-key", "create")); rlsServerImpl.setLookupTable( ImmutableMap.of( - routeLookupRequest, + routeLookupRequestKey, RouteLookupResponse.create( ImmutableList.of("primary.cloudbigtable.googleapis.com", "target2", "target3"), "header-rls-data-value"))); // valid channel - CachedRouteLookupResponse resp = getInSyncContext(routeLookupRequest); + CachedRouteLookupResponse resp = getInSyncContext(routeLookupRequestKey); assertThat(resp.hasData()).isFalse(); fakeClock.forwardTime(SERVER_LATENCY_MILLIS, TimeUnit.MILLISECONDS); - resp = getInSyncContext(routeLookupRequest); + resp = getInSyncContext(routeLookupRequestKey); assertThat(resp.hasData()).isTrue(); ArgumentCaptor pickerCaptor = ArgumentCaptor.forClass(SubchannelPicker.class); @@ -493,21 +505,24 @@ public void get_withAdaptiveThrottler() throws Exception { .setTicker(fakeClock.getTicker()) .build(); InOrder inOrder = inOrder(helper); - RouteLookupRequest routeLookupRequest = RouteLookupRequest.create(ImmutableMap.of( - "server", "bigtable.googleapis.com", "service-key", "service1", "method-key", "create")); + RlsProtoData.RouteLookupRequestKey routeLookupRequestKey = + RlsProtoData.RouteLookupRequestKey.create( + ImmutableMap.of( + "server", "bigtable.googleapis.com", "service-key", "service1", + "method-key", "create")); rlsServerImpl.setLookupTable( ImmutableMap.of( - routeLookupRequest, + routeLookupRequestKey, RouteLookupResponse.create( ImmutableList.of("primary.cloudbigtable.googleapis.com"), "header-rls-data-value"))); // valid channel - CachedRouteLookupResponse resp = getInSyncContext(routeLookupRequest); + CachedRouteLookupResponse resp = getInSyncContext(routeLookupRequestKey); assertThat(resp.isPending()).isTrue(); fakeClock.forwardTime(SERVER_LATENCY_MILLIS, TimeUnit.MILLISECONDS); - resp = getInSyncContext(routeLookupRequest); + resp = getInSyncContext(routeLookupRequestKey); assertThat(resp.hasData()).isTrue(); ArgumentCaptor pickerCaptor = ArgumentCaptor.forClass(SubchannelPicker.class); @@ -524,13 +539,13 @@ public void get_withAdaptiveThrottler() throws Exception { // move backoff further back to only test error behavior fakeBackoffProvider.nextPolicy = createBackoffPolicy(100, TimeUnit.MILLISECONDS); // try to get invalid - RouteLookupRequest invalidRouteLookupRequest = - RouteLookupRequest.create(ImmutableMap.of()); - CachedRouteLookupResponse errorResp = getInSyncContext(invalidRouteLookupRequest); + RlsProtoData.RouteLookupRequestKey invalidRouteLookupRequestKey = + RlsProtoData.RouteLookupRequestKey.create(ImmutableMap.of()); + CachedRouteLookupResponse errorResp = getInSyncContext(invalidRouteLookupRequestKey); assertThat(errorResp.isPending()).isTrue(); fakeClock.forwardTime(SERVER_LATENCY_MILLIS, TimeUnit.MILLISECONDS); - errorResp = getInSyncContext(invalidRouteLookupRequest); + errorResp = getInSyncContext(invalidRouteLookupRequestKey); assertThat(errorResp.hasError()).isTrue(); // Channel is still READY because the subchannel for method /service1/create is still READY. @@ -560,22 +575,26 @@ private PickSubchannelArgsImpl getInvalidArgs(Metadata headers) { @Test public void get_childPolicyWrapper_reusedForSameTarget() throws Exception { setUpRlsLbClient(); - RouteLookupRequest routeLookupRequest = RouteLookupRequest.create(ImmutableMap.of( - "server", "bigtable.googleapis.com", "service-key", "foo", "method-key", "bar")); - RouteLookupRequest routeLookupRequest2 = RouteLookupRequest.create(ImmutableMap.of( - "server", "bigtable.googleapis.com", "service-key", "foo", "method-key", "baz")); + RlsProtoData.RouteLookupRequestKey routeLookupRequestKey = + RlsProtoData.RouteLookupRequestKey.create( + ImmutableMap.of( + "server", "bigtable.googleapis.com", "service-key", "foo", "method-key", "bar")); + RlsProtoData.RouteLookupRequestKey routeLookupRequestKey2 = + RlsProtoData.RouteLookupRequestKey.create( + ImmutableMap.of( + "server", "bigtable.googleapis.com", "service-key", "foo", "method-key", "baz")); rlsServerImpl.setLookupTable( ImmutableMap.of( - routeLookupRequest, + routeLookupRequestKey, RouteLookupResponse.create(ImmutableList.of("target"), "header"), - routeLookupRequest2, + routeLookupRequestKey2, RouteLookupResponse.create(ImmutableList.of("target"), "header2"))); - CachedRouteLookupResponse resp = getInSyncContext(routeLookupRequest); + CachedRouteLookupResponse resp = getInSyncContext(routeLookupRequestKey); assertThat(resp.isPending()).isTrue(); fakeClock.forwardTime(SERVER_LATENCY_MILLIS, TimeUnit.MILLISECONDS); - resp = getInSyncContext(routeLookupRequest); + resp = getInSyncContext(routeLookupRequestKey); assertThat(resp.hasData()).isTrue(); assertThat(resp.getHeaderData()).isEqualTo("header"); @@ -585,11 +604,11 @@ public void get_childPolicyWrapper_reusedForSameTarget() throws Exception { assertThat(childPolicyWrapper.getPicker()).isNotInstanceOf(RlsPicker.class); // request2 has same target, it should reuse childPolicyWrapper - CachedRouteLookupResponse resp2 = getInSyncContext(routeLookupRequest2); + CachedRouteLookupResponse resp2 = getInSyncContext(routeLookupRequestKey2); assertThat(resp2.isPending()).isTrue(); fakeClock.forwardTime(SERVER_LATENCY_MILLIS, TimeUnit.MILLISECONDS); - resp2 = getInSyncContext(routeLookupRequest2); + resp2 = getInSyncContext(routeLookupRequestKey2); assertThat(resp2.hasData()).isTrue(); assertThat(resp2.getHeaderData()).isEqualTo("header2"); assertThat(resp2.getChildPolicyWrapper()).isEqualTo(resp.getChildPolicyWrapper()); @@ -598,20 +617,22 @@ public void get_childPolicyWrapper_reusedForSameTarget() throws Exception { @Test public void get_childPolicyWrapper_multiTarget() throws Exception { setUpRlsLbClient(); - RouteLookupRequest routeLookupRequest = RouteLookupRequest.create(ImmutableMap.of( - "server", "bigtable.googleapis.com", "service-key", "foo", "method-key", "bar")); + RlsProtoData.RouteLookupRequestKey routeLookupRequestKey = + RlsProtoData.RouteLookupRequestKey.create( + ImmutableMap.of( + "server", "bigtable.googleapis.com", "service-key", "foo", "method-key", "bar")); rlsServerImpl.setLookupTable( ImmutableMap.of( - routeLookupRequest, + routeLookupRequestKey, RouteLookupResponse.create( ImmutableList.of("target1", "target2", "target3"), "header"))); - CachedRouteLookupResponse resp = getInSyncContext(routeLookupRequest); + CachedRouteLookupResponse resp = getInSyncContext(routeLookupRequestKey); assertThat(resp.isPending()).isTrue(); fakeClock.forwardTime(SERVER_LATENCY_MILLIS, TimeUnit.MILLISECONDS); - resp = getInSyncContext(routeLookupRequest); + resp = getInSyncContext(routeLookupRequestKey); assertThat(resp.hasData()).isTrue(); List policyWrappers = new ArrayList<>(); @@ -680,14 +701,15 @@ public void metricGauges() throws ExecutionException, InterruptedException, Time .recordLongGauge(argThat(new LongGaugeInstrumentArgumentMatcher("grpc.lb.rls.cache_size")), eq(0L), any(), any()); - RouteLookupRequest routeLookupRequest = RouteLookupRequest.create( - ImmutableMap.of("server", "bigtable.googleapis.com", "service-key", "foo", "method-key", - "bar")); - rlsServerImpl.setLookupTable(ImmutableMap.of(routeLookupRequest, + RlsProtoData.RouteLookupRequestKey routeLookupRequestKey = + RlsProtoData.RouteLookupRequestKey.create( + ImmutableMap.of("server", "bigtable.googleapis.com", "service-key", "foo", "method-key", + "bar")); + rlsServerImpl.setLookupTable(ImmutableMap.of(routeLookupRequestKey, RouteLookupResponse.create(ImmutableList.of("target"), "header"))); // Make a request that will populate the cache with an entry - getInSyncContext(routeLookupRequest); + getInSyncContext(routeLookupRequestKey); fakeClock.forwardTime(SERVER_LATENCY_MILLIS, TimeUnit.MILLISECONDS); // Gauge values should reflect the new cache entry. @@ -857,7 +879,8 @@ private static final class StaticFixedDelayRlsServerImpl private final long responseDelayNano; private final ScheduledExecutorService scheduledExecutorService; - private Map lookupTable = ImmutableMap.of(); + private Map lookupTable = + ImmutableMap.of(); public StaticFixedDelayRlsServerImpl( long responseDelayNano, ScheduledExecutorService scheduledExecutorService) { @@ -867,7 +890,8 @@ public StaticFixedDelayRlsServerImpl( checkNotNull(scheduledExecutorService, "scheduledExecutorService"); } - private void setLookupTable(Map lookupTable) { + private void setLookupTable(Map lookupTable) { this.lookupTable = checkNotNull(lookupTable, "lookupTable"); } @@ -880,7 +904,9 @@ public void routeLookup(final io.grpc.lookup.v1.RouteLookupRequest request, @Override public void run() { RouteLookupResponse response = - lookupTable.get(REQUEST_CONVERTER.convert(request)); + lookupTable.get( + RlsProtoData.RouteLookupRequestKey.create( + REQUEST_CONVERTER.convert(request).keyMap())); if (response == null) { responseObserver.onError(new RuntimeException("not found")); } else { diff --git a/rls/src/test/java/io/grpc/rls/RlsLoadBalancerTest.java b/rls/src/test/java/io/grpc/rls/RlsLoadBalancerTest.java index 354466f3caf..c180935b153 100644 --- a/rls/src/test/java/io/grpc/rls/RlsLoadBalancerTest.java +++ b/rls/src/test/java/io/grpc/rls/RlsLoadBalancerTest.java @@ -166,15 +166,19 @@ public void setUp() { .build(); fakeRlsServerImpl.setLookupTable( ImmutableMap.of( - RouteLookupRequest.create(ImmutableMap.of( + RouteLookupRequest.create( + ImmutableMap.of( "server", "fake-bigtable.googleapis.com", "service-key", "com.google", - "method-key", "Search")), + "method-key", "Search"), + RouteLookupRequest.Reason.REASON_MISS), RouteLookupResponse.create(ImmutableList.of("wilderness"), "where are you?"), - RouteLookupRequest.create(ImmutableMap.of( + RouteLookupRequest.create( + ImmutableMap.of( "server", "fake-bigtable.googleapis.com", "service-key", "com.google", - "method-key", "Rescue")), + "method-key", "Rescue"), + RouteLookupRequest.Reason.REASON_MISS), RouteLookupResponse.create(ImmutableList.of("civilization"), "you are safe"))); rlsLb = (RlsLoadBalancer) provider.newLoadBalancer(helper); diff --git a/rls/src/test/java/io/grpc/rls/RlsProtoConvertersTest.java b/rls/src/test/java/io/grpc/rls/RlsProtoConvertersTest.java index fc5fdb59f21..ad1ce8c363e 100644 --- a/rls/src/test/java/io/grpc/rls/RlsProtoConvertersTest.java +++ b/rls/src/test/java/io/grpc/rls/RlsProtoConvertersTest.java @@ -61,12 +61,14 @@ public void convert_toRequestObject() { Converter converter = new RouteLookupRequestConverter().reverse(); RlsProtoData.RouteLookupRequest requestObject = - RlsProtoData.RouteLookupRequest.create(ImmutableMap.of("key1", "val1")); + RlsProtoData.RouteLookupRequest.create(ImmutableMap.of("key1", "val1"), + RlsProtoData.RouteLookupRequest.Reason.REASON_MISS); RouteLookupRequest proto = converter.convert(requestObject); assertThat(proto.getTargetType()).isEqualTo("grpc"); assertThat(proto.getKeyMapMap()).containsExactly("key1", "val1"); + assertThat(proto.getReason()).isEqualTo(RouteLookupRequest.Reason.REASON_MISS); } @Test diff --git a/rls/src/test/java/io/grpc/rls/RlsRequestFactoryTest.java b/rls/src/test/java/io/grpc/rls/RlsRequestFactoryTest.java index 6ee2c01af8a..2b900994ed9 100644 --- a/rls/src/test/java/io/grpc/rls/RlsRequestFactoryTest.java +++ b/rls/src/test/java/io/grpc/rls/RlsRequestFactoryTest.java @@ -26,7 +26,6 @@ import io.grpc.rls.RlsProtoData.GrpcKeyBuilder.Name; import io.grpc.rls.RlsProtoData.NameMatcher; import io.grpc.rls.RlsProtoData.RouteLookupConfig; -import io.grpc.rls.RlsProtoData.RouteLookupRequest; import java.util.concurrent.TimeUnit; import org.junit.Test; import org.junit.runner.RunWith; @@ -82,8 +81,9 @@ public void create_pathMatches() { metadata.put(Metadata.Key.of("X-Google-Id", Metadata.ASCII_STRING_MARSHALLER), "123"); metadata.put(Metadata.Key.of("foo", Metadata.ASCII_STRING_MARSHALLER), "bar"); - RouteLookupRequest request = factory.create("com.google.service1", "Create", metadata); - assertThat(request.keyMap()).containsExactly( + RlsProtoData.RouteLookupRequestKey routeLookupRequestKey = + factory.create("com.google.service1", "Create", metadata); + assertThat(routeLookupRequestKey.keyMap()).containsExactly( "user", "test", "id", "123", "server-1", "bigtable.googleapis.com", @@ -97,9 +97,10 @@ public void create_pathFallbackMatches() { metadata.put(Metadata.Key.of("Password", Metadata.ASCII_STRING_MARSHALLER), "hunter2"); metadata.put(Metadata.Key.of("foo", Metadata.ASCII_STRING_MARSHALLER), "bar"); - RouteLookupRequest request = factory.create("com.google.service1" , "Update", metadata); + RlsProtoData.RouteLookupRequestKey routeLookupRequestKey = + factory.create("com.google.service1" , "Update", metadata); - assertThat(request.keyMap()).containsExactly( + assertThat(routeLookupRequestKey.keyMap()).containsExactly( "user", "test", "password", "hunter2", "service-2", "com.google.service1", @@ -113,9 +114,10 @@ public void create_pathFallbackMatches_optionalHeaderMissing() { metadata.put(Metadata.Key.of("X-Google-Id", Metadata.ASCII_STRING_MARSHALLER), "123"); metadata.put(Metadata.Key.of("foo", Metadata.ASCII_STRING_MARSHALLER), "bar"); - RouteLookupRequest request = factory.create("com.google.service1", "Update", metadata); + RlsProtoData.RouteLookupRequestKey routeLookupRequestKey = + factory.create("com.google.service1", "Update", metadata); - assertThat(request.keyMap()).containsExactly( + assertThat(routeLookupRequestKey.keyMap()).containsExactly( "user", "test", "service-2", "com.google.service1", "const-key-2", "const-value-2"); @@ -128,8 +130,9 @@ public void create_unknownPath() { metadata.put(Metadata.Key.of("X-Google-Id", Metadata.ASCII_STRING_MARSHALLER), "123"); metadata.put(Metadata.Key.of("foo", Metadata.ASCII_STRING_MARSHALLER), "bar"); - RouteLookupRequest request = factory.create("abc.def.service999", "Update", metadata); - assertThat(request.keyMap()).isEmpty(); + RlsProtoData.RouteLookupRequestKey routeLookupRequestKey = + factory.create("abc.def.service999", "Update", metadata); + assertThat(routeLookupRequestKey.keyMap()).isEmpty(); } @Test @@ -139,9 +142,10 @@ public void create_noMethodInRlsConfig() { metadata.put(Metadata.Key.of("X-Google-Id", Metadata.ASCII_STRING_MARSHALLER), "123"); metadata.put(Metadata.Key.of("foo", Metadata.ASCII_STRING_MARSHALLER), "bar"); - RouteLookupRequest request = factory.create("com.google.service3", "Update", metadata); + RlsProtoData.RouteLookupRequestKey routeLookupRequestKey = + factory.create("com.google.service3", "Update", metadata); - assertThat(request.keyMap()).containsExactly( + assertThat(routeLookupRequestKey.keyMap()).containsExactly( "user", "test", "const-key-4", "const-value-4"); } } From 8ce971d04c5125ce91b0862021d2e0f64dfd3541 Mon Sep 17 00:00:00 2001 From: Kannan J Date: Mon, 27 Oct 2025 06:48:40 +0000 Subject: [PATCH 3/9] Review comments. --- rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java | 2 +- rls/src/main/java/io/grpc/rls/RlsProtoData.java | 4 ++-- rls/src/main/java/io/grpc/rls/RlsRequestFactory.java | 7 ------- 3 files changed, 3 insertions(+), 10 deletions(-) diff --git a/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java b/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java index c60a2304290..eaddafd8ca7 100644 --- a/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java +++ b/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java @@ -307,7 +307,7 @@ private CachedRouteLookupResponse asyncRlsCall( } final SettableFuture response = SettableFuture.create(); io.grpc.lookup.v1.RouteLookupRequest routeLookupRequest = REQUEST_CONVERTER.convert( - requestFactory.create(routeLookupRequestKey, routeLookupReason)); + RouteLookupRequest.create(routeLookupRequestKey.keyMap(), routeLookupReason)); logger.log(ChannelLogLevel.DEBUG, "[RLS Entry {0}] Starting RouteLookup: {1}", routeLookupRequestKey, routeLookupRequest); rlsStub.withDeadlineAfter(callTimeoutNanos, TimeUnit.NANOSECONDS) diff --git a/rls/src/main/java/io/grpc/rls/RlsProtoData.java b/rls/src/main/java/io/grpc/rls/RlsProtoData.java index 1c06e7dd23e..350accce96c 100644 --- a/rls/src/main/java/io/grpc/rls/RlsProtoData.java +++ b/rls/src/main/java/io/grpc/rls/RlsProtoData.java @@ -45,14 +45,14 @@ static RouteLookupRequestKey create(ImmutableMap keyMap) { @Immutable abstract static class RouteLookupRequest { - // Names should match those in grpc.lookup.v1.RouteLookupRequest.Reason + /** Names should match those in {@link io.grpc.lookup.v1.RouteLookupRequest.Reason} */ enum Reason { REASON_UNKNOWN, // Unused REASON_MISS, // No data available in local cache REASON_STALE; // Data in local cache is stale } - // Reason for making this request. + /** Reason for making this request. */ abstract Reason reason(); /** Returns a map of key values extracted via key builders for the gRPC or HTTP request. */ diff --git a/rls/src/main/java/io/grpc/rls/RlsRequestFactory.java b/rls/src/main/java/io/grpc/rls/RlsRequestFactory.java index 82a1d98f5bf..8bb8b8ddfcc 100644 --- a/rls/src/main/java/io/grpc/rls/RlsRequestFactory.java +++ b/rls/src/main/java/io/grpc/rls/RlsRequestFactory.java @@ -93,13 +93,6 @@ RouteLookupRequestKey create(String service, String method, Metadata metadata) { return RouteLookupRequestKey.create(rlsRequestHeaders.buildOrThrow()); } - /** Creates a {@link RouteLookupRequest} using the given request lookup key and reason. */ - @CheckReturnValue - RouteLookupRequest create(RouteLookupRequestKey routeLookupRequestKey, - RouteLookupRequest.Reason reason) { - return RouteLookupRequest.create(routeLookupRequestKey.keyMap(), reason); - } - private ImmutableMap.Builder createRequestHeaders( Metadata metadata, List keyBuilder) { ImmutableMap.Builder rlsRequestHeaders = ImmutableMap.builder(); From bfd48ce315f95faeeb47333a096753054606ce2d Mon Sep 17 00:00:00 2001 From: Kannan J Date: Wed, 29 Oct 2025 06:38:43 +0000 Subject: [PATCH 4/9] Make backed off request after previous throttling always send cache miss as the route lookup reason. --- .../java/io/grpc/rls/CachingRlsLbClient.java | 18 +++++++----------- .../io/grpc/rls/CachingRlsLbClientTest.java | 8 ++++++++ 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java b/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java index eaddafd8ca7..d7b83c4a963 100644 --- a/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java +++ b/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java @@ -303,7 +303,7 @@ private CachedRouteLookupResponse asyncRlsCall( // on this result return CachedRouteLookupResponse.backoffEntry(createBackOffEntry( routeLookupRequestKey, Status.RESOURCE_EXHAUSTED.withDescription("RLS throttled"), - backoffPolicy, routeLookupReason)); + backoffPolicy)); } final SettableFuture response = SettableFuture.create(); io.grpc.lookup.v1.RouteLookupRequest routeLookupRequest = REQUEST_CONVERTER.convert( @@ -417,7 +417,7 @@ private void pendingRpcComplete(PendingCacheEntry entry) { // reattempt picks when the child LB is done connecting } catch (Exception e) { createBackOffEntry(entry.routeLookupRequestKey, Status.fromThrowable(e), - entry.backoffPolicy, entry.routeLookupReason); + entry.backoffPolicy); // Cache updated. updateBalancingState() to reattempt picks helper.triggerPendingRpcProcessing(); } @@ -439,9 +439,8 @@ private DataCacheEntry createDataEntry( } @GuardedBy("lock") - private BackoffCacheEntry createBackOffEntry( - RouteLookupRequestKey routeLookupRequestKey, Status status, - @Nullable BackoffPolicy backoffPolicy, RouteLookupRequest.Reason routeLookupReason) { + private BackoffCacheEntry createBackOffEntry(RouteLookupRequestKey routeLookupRequestKey, + Status status, @Nullable BackoffPolicy backoffPolicy) { if (backoffPolicy == null) { backoffPolicy = backoffProvider.get(); } @@ -450,8 +449,7 @@ private BackoffCacheEntry createBackOffEntry( ChannelLogLevel.DEBUG, "[RLS Entry {0}] Transition to back off: status={1}, delayNanos={2}", routeLookupRequestKey, status, delayNanos); - BackoffCacheEntry entry = new BackoffCacheEntry(routeLookupRequestKey, status, backoffPolicy, - routeLookupReason); + BackoffCacheEntry entry = new BackoffCacheEntry(routeLookupRequestKey, status, backoffPolicy); // Lock is held, so the task can't execute before the assignment entry.scheduledFuture = scheduledExecutorService.schedule( () -> refreshBackoffEntry(entry), delayNanos, TimeUnit.NANOSECONDS); @@ -469,7 +467,7 @@ private void refreshBackoffEntry(BackoffCacheEntry entry) { logger.log(ChannelLogLevel.DEBUG, "[RLS Entry {0}] Calling RLS for transition to pending", entry.routeLookupRequestKey); linkedHashLruCache.invalidate(entry.routeLookupRequestKey); - asyncRlsCall(entry.routeLookupRequestKey, entry.backoffPolicy, entry.routeLookupReason); + asyncRlsCall(entry.routeLookupRequestKey, entry.backoffPolicy, RouteLookupRequest.Reason.REASON_MISS); } } @@ -779,15 +777,13 @@ private static final class BackoffCacheEntry extends CacheEntry { private final Status status; private final BackoffPolicy backoffPolicy; - private final RouteLookupRequest.Reason routeLookupReason; private Future scheduledFuture; BackoffCacheEntry(RouteLookupRequestKey routeLookupRequestKey, Status status, - BackoffPolicy backoffPolicy, RouteLookupRequest.Reason routeLookupReason) { + BackoffPolicy backoffPolicy) { super(routeLookupRequestKey); this.status = checkNotNull(status, "status"); this.backoffPolicy = checkNotNull(backoffPolicy, "backoffPolicy"); - this.routeLookupReason = checkNotNull(routeLookupReason, "routeLookupReason"); } Status getStatus() { diff --git a/rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java b/rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java index 9b8d59be8d8..95979d546c5 100644 --- a/rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java +++ b/rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java @@ -239,6 +239,7 @@ public void get_noError_lifeCycle() throws Exception { // cache hit for staled entry fakeClock.forwardTime(ROUTE_LOOKUP_CONFIG.staleAgeInNanos(), TimeUnit.NANOSECONDS); + rlsServerImpl.routeLookupReason = null; resp = getInSyncContext(routeLookupRequestKey); assertThat(resp.hasData()).isTrue(); @@ -250,6 +251,7 @@ public void get_noError_lifeCycle() throws Exception { resp = getInSyncContext(routeLookupRequestKey); + assertThat(rlsServerImpl.routeLookupReason).isEqualTo(io.grpc.lookup.v1.RouteLookupRequest.Reason.REASON_STALE); assertThat(resp.hasData()).isTrue(); // existing cache expired @@ -298,6 +300,7 @@ public void rls_withCustomRlsChannelServiceConfig() throws Exception { routeLookupRequestKey, RouteLookupResponse.create(ImmutableList.of("target"), "header"))); + rlsServerImpl.routeLookupReason = null; // initial request CachedRouteLookupResponse resp = getInSyncContext(routeLookupRequestKey); assertThat(resp.isPending()).isTrue(); @@ -307,6 +310,7 @@ public void rls_withCustomRlsChannelServiceConfig() throws Exception { resp = getInSyncContext(routeLookupRequestKey); assertThat(resp.hasData()).isTrue(); + assertThat(rlsServerImpl.routeLookupReason).isEqualTo(io.grpc.lookup.v1.RouteLookupRequest.Reason.REASON_MISS); assertThat(rlsChannelOverriddenAuthority).isEqualTo("bigtable.googleapis.com:443"); assertThat(rlsChannelServiceConfig).isEqualTo(routeLookupChannelServiceConfig); @@ -348,8 +352,10 @@ public void get_throttledAndRecover() throws Exception { assertThat(resp.isPending()).isTrue(); + rlsServerImpl.routeLookupReason = null; // server responses fakeClock.forwardTime(SERVER_LATENCY_MILLIS, TimeUnit.MILLISECONDS); + assertThat(rlsServerImpl.routeLookupReason).isEqualTo(io.grpc.lookup.v1.RouteLookupRequest.Reason.REASON_MISS); resp = getInSyncContext(routeLookupRequestKey); @@ -881,6 +887,7 @@ private static final class StaticFixedDelayRlsServerImpl private Map lookupTable = ImmutableMap.of(); + io.grpc.lookup.v1.RouteLookupRequest.Reason routeLookupReason; public StaticFixedDelayRlsServerImpl( long responseDelayNano, ScheduledExecutorService scheduledExecutorService) { @@ -903,6 +910,7 @@ public void routeLookup(final io.grpc.lookup.v1.RouteLookupRequest request, new Runnable() { @Override public void run() { + routeLookupReason = request.getReason(); RouteLookupResponse response = lookupTable.get( RlsProtoData.RouteLookupRequestKey.create( From 97308408f4e49c201d3815dbf214e96bcfc2a3a9 Mon Sep 17 00:00:00 2001 From: Kannan J Date: Wed, 29 Oct 2025 06:42:12 +0000 Subject: [PATCH 5/9] Undo converting RlsRequestFactory from local variable to field. --- rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java b/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java index d7b83c4a963..0b1efe8a580 100644 --- a/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java +++ b/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java @@ -128,7 +128,6 @@ final class CachingRlsLbClient { private final RlsLbHelper helper; private final ManagedChannel rlsChannel; private final RouteLookupServiceStub rlsStub; - private final RlsRequestFactory requestFactory; private final RlsPicker rlsPicker; private final ResolvedAddressFactory childLbResolvedAddressFactory; @GuardedBy("lock") @@ -197,7 +196,7 @@ private CachingRlsLbClient(Builder builder) { ChannelLogLevel.DEBUG, "Can not get hostname from authority: {0}", helper.getAuthority()); serverHost = helper.getAuthority(); } - requestFactory = new RlsRequestFactory( + RlsRequestFactory requestFactory = new RlsRequestFactory( lbPolicyConfig.getRouteLookupConfig(), serverHost); rlsPicker = new RlsPicker(requestFactory, rlsConfig.lookupService()); // It is safe to use helper.getUnsafeChannelCredentials() because the client authenticates the From ae61750d8ba8bd4f47e9692d51c858cd1bb3afb2 Mon Sep 17 00:00:00 2001 From: Kannan J Date: Wed, 29 Oct 2025 06:38:43 +0000 Subject: [PATCH 6/9] Addressing review comments: Made lookup reason always as cache miss after a backed off entry is reprocessed, and the style related comments. Added assertions for lookup reason in unit tests. --- rls/src/main/java/io/grpc/rls/RlsProtoData.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/rls/src/main/java/io/grpc/rls/RlsProtoData.java b/rls/src/main/java/io/grpc/rls/RlsProtoData.java index 350accce96c..9e61d62a383 100644 --- a/rls/src/main/java/io/grpc/rls/RlsProtoData.java +++ b/rls/src/main/java/io/grpc/rls/RlsProtoData.java @@ -47,9 +47,12 @@ abstract static class RouteLookupRequest { /** Names should match those in {@link io.grpc.lookup.v1.RouteLookupRequest.Reason} */ enum Reason { - REASON_UNKNOWN, // Unused - REASON_MISS, // No data available in local cache - REASON_STALE; // Data in local cache is stale + /** Unused */ + REASON_UNKNOWN, + /** No data available in local cache */ + REASON_MISS, + /** Data in local cache is stale */ + REASON_STALE; } /** Reason for making this request. */ From 6a30fac657cca1745aa2911ce38fead92b500eee Mon Sep 17 00:00:00 2001 From: Kannan J Date: Wed, 29 Oct 2025 06:38:43 +0000 Subject: [PATCH 7/9] Addressing review comments: Made lookup reason always as cache miss after a backed off entry is reprocessed, and the style related comments. Added assertions for lookup reason in unit tests. --- .../java/io/grpc/rls/CachingRlsLbClient.java | 21 +++++++------------ .../main/java/io/grpc/rls/RlsProtoData.java | 9 +++++--- .../io/grpc/rls/CachingRlsLbClientTest.java | 8 +++++++ 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java b/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java index eaddafd8ca7..0b1efe8a580 100644 --- a/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java +++ b/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java @@ -128,7 +128,6 @@ final class CachingRlsLbClient { private final RlsLbHelper helper; private final ManagedChannel rlsChannel; private final RouteLookupServiceStub rlsStub; - private final RlsRequestFactory requestFactory; private final RlsPicker rlsPicker; private final ResolvedAddressFactory childLbResolvedAddressFactory; @GuardedBy("lock") @@ -197,7 +196,7 @@ private CachingRlsLbClient(Builder builder) { ChannelLogLevel.DEBUG, "Can not get hostname from authority: {0}", helper.getAuthority()); serverHost = helper.getAuthority(); } - requestFactory = new RlsRequestFactory( + RlsRequestFactory requestFactory = new RlsRequestFactory( lbPolicyConfig.getRouteLookupConfig(), serverHost); rlsPicker = new RlsPicker(requestFactory, rlsConfig.lookupService()); // It is safe to use helper.getUnsafeChannelCredentials() because the client authenticates the @@ -303,7 +302,7 @@ private CachedRouteLookupResponse asyncRlsCall( // on this result return CachedRouteLookupResponse.backoffEntry(createBackOffEntry( routeLookupRequestKey, Status.RESOURCE_EXHAUSTED.withDescription("RLS throttled"), - backoffPolicy, routeLookupReason)); + backoffPolicy)); } final SettableFuture response = SettableFuture.create(); io.grpc.lookup.v1.RouteLookupRequest routeLookupRequest = REQUEST_CONVERTER.convert( @@ -417,7 +416,7 @@ private void pendingRpcComplete(PendingCacheEntry entry) { // reattempt picks when the child LB is done connecting } catch (Exception e) { createBackOffEntry(entry.routeLookupRequestKey, Status.fromThrowable(e), - entry.backoffPolicy, entry.routeLookupReason); + entry.backoffPolicy); // Cache updated. updateBalancingState() to reattempt picks helper.triggerPendingRpcProcessing(); } @@ -439,9 +438,8 @@ private DataCacheEntry createDataEntry( } @GuardedBy("lock") - private BackoffCacheEntry createBackOffEntry( - RouteLookupRequestKey routeLookupRequestKey, Status status, - @Nullable BackoffPolicy backoffPolicy, RouteLookupRequest.Reason routeLookupReason) { + private BackoffCacheEntry createBackOffEntry(RouteLookupRequestKey routeLookupRequestKey, + Status status, @Nullable BackoffPolicy backoffPolicy) { if (backoffPolicy == null) { backoffPolicy = backoffProvider.get(); } @@ -450,8 +448,7 @@ private BackoffCacheEntry createBackOffEntry( ChannelLogLevel.DEBUG, "[RLS Entry {0}] Transition to back off: status={1}, delayNanos={2}", routeLookupRequestKey, status, delayNanos); - BackoffCacheEntry entry = new BackoffCacheEntry(routeLookupRequestKey, status, backoffPolicy, - routeLookupReason); + BackoffCacheEntry entry = new BackoffCacheEntry(routeLookupRequestKey, status, backoffPolicy); // Lock is held, so the task can't execute before the assignment entry.scheduledFuture = scheduledExecutorService.schedule( () -> refreshBackoffEntry(entry), delayNanos, TimeUnit.NANOSECONDS); @@ -469,7 +466,7 @@ private void refreshBackoffEntry(BackoffCacheEntry entry) { logger.log(ChannelLogLevel.DEBUG, "[RLS Entry {0}] Calling RLS for transition to pending", entry.routeLookupRequestKey); linkedHashLruCache.invalidate(entry.routeLookupRequestKey); - asyncRlsCall(entry.routeLookupRequestKey, entry.backoffPolicy, entry.routeLookupReason); + asyncRlsCall(entry.routeLookupRequestKey, entry.backoffPolicy, RouteLookupRequest.Reason.REASON_MISS); } } @@ -779,15 +776,13 @@ private static final class BackoffCacheEntry extends CacheEntry { private final Status status; private final BackoffPolicy backoffPolicy; - private final RouteLookupRequest.Reason routeLookupReason; private Future scheduledFuture; BackoffCacheEntry(RouteLookupRequestKey routeLookupRequestKey, Status status, - BackoffPolicy backoffPolicy, RouteLookupRequest.Reason routeLookupReason) { + BackoffPolicy backoffPolicy) { super(routeLookupRequestKey); this.status = checkNotNull(status, "status"); this.backoffPolicy = checkNotNull(backoffPolicy, "backoffPolicy"); - this.routeLookupReason = checkNotNull(routeLookupReason, "routeLookupReason"); } Status getStatus() { diff --git a/rls/src/main/java/io/grpc/rls/RlsProtoData.java b/rls/src/main/java/io/grpc/rls/RlsProtoData.java index 350accce96c..9e61d62a383 100644 --- a/rls/src/main/java/io/grpc/rls/RlsProtoData.java +++ b/rls/src/main/java/io/grpc/rls/RlsProtoData.java @@ -47,9 +47,12 @@ abstract static class RouteLookupRequest { /** Names should match those in {@link io.grpc.lookup.v1.RouteLookupRequest.Reason} */ enum Reason { - REASON_UNKNOWN, // Unused - REASON_MISS, // No data available in local cache - REASON_STALE; // Data in local cache is stale + /** Unused */ + REASON_UNKNOWN, + /** No data available in local cache */ + REASON_MISS, + /** Data in local cache is stale */ + REASON_STALE; } /** Reason for making this request. */ diff --git a/rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java b/rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java index 9b8d59be8d8..95979d546c5 100644 --- a/rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java +++ b/rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java @@ -239,6 +239,7 @@ public void get_noError_lifeCycle() throws Exception { // cache hit for staled entry fakeClock.forwardTime(ROUTE_LOOKUP_CONFIG.staleAgeInNanos(), TimeUnit.NANOSECONDS); + rlsServerImpl.routeLookupReason = null; resp = getInSyncContext(routeLookupRequestKey); assertThat(resp.hasData()).isTrue(); @@ -250,6 +251,7 @@ public void get_noError_lifeCycle() throws Exception { resp = getInSyncContext(routeLookupRequestKey); + assertThat(rlsServerImpl.routeLookupReason).isEqualTo(io.grpc.lookup.v1.RouteLookupRequest.Reason.REASON_STALE); assertThat(resp.hasData()).isTrue(); // existing cache expired @@ -298,6 +300,7 @@ public void rls_withCustomRlsChannelServiceConfig() throws Exception { routeLookupRequestKey, RouteLookupResponse.create(ImmutableList.of("target"), "header"))); + rlsServerImpl.routeLookupReason = null; // initial request CachedRouteLookupResponse resp = getInSyncContext(routeLookupRequestKey); assertThat(resp.isPending()).isTrue(); @@ -307,6 +310,7 @@ public void rls_withCustomRlsChannelServiceConfig() throws Exception { resp = getInSyncContext(routeLookupRequestKey); assertThat(resp.hasData()).isTrue(); + assertThat(rlsServerImpl.routeLookupReason).isEqualTo(io.grpc.lookup.v1.RouteLookupRequest.Reason.REASON_MISS); assertThat(rlsChannelOverriddenAuthority).isEqualTo("bigtable.googleapis.com:443"); assertThat(rlsChannelServiceConfig).isEqualTo(routeLookupChannelServiceConfig); @@ -348,8 +352,10 @@ public void get_throttledAndRecover() throws Exception { assertThat(resp.isPending()).isTrue(); + rlsServerImpl.routeLookupReason = null; // server responses fakeClock.forwardTime(SERVER_LATENCY_MILLIS, TimeUnit.MILLISECONDS); + assertThat(rlsServerImpl.routeLookupReason).isEqualTo(io.grpc.lookup.v1.RouteLookupRequest.Reason.REASON_MISS); resp = getInSyncContext(routeLookupRequestKey); @@ -881,6 +887,7 @@ private static final class StaticFixedDelayRlsServerImpl private Map lookupTable = ImmutableMap.of(); + io.grpc.lookup.v1.RouteLookupRequest.Reason routeLookupReason; public StaticFixedDelayRlsServerImpl( long responseDelayNano, ScheduledExecutorService scheduledExecutorService) { @@ -903,6 +910,7 @@ public void routeLookup(final io.grpc.lookup.v1.RouteLookupRequest request, new Runnable() { @Override public void run() { + routeLookupReason = request.getReason(); RouteLookupResponse response = lookupTable.get( RlsProtoData.RouteLookupRequestKey.create( From 67711d30df20e2e3edd8b828b2d822b5aa2370de Mon Sep 17 00:00:00 2001 From: Kannan J Date: Thu, 30 Oct 2025 12:16:58 +0000 Subject: [PATCH 8/9] Remove routeLookupReason from PendingCacheEntry. --- .../main/java/io/grpc/rls/CachingRlsLbClient.java | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java b/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java index 0b1efe8a580..af77d88c025 100644 --- a/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java +++ b/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java @@ -334,7 +334,7 @@ public void onCompleted() { } }); return CachedRouteLookupResponse.pendingResponse( - createPendingEntry(routeLookupRequestKey, response, backoffPolicy, routeLookupReason)); + createPendingEntry(routeLookupRequestKey, response, backoffPolicy)); } /** @@ -391,10 +391,9 @@ void requestConnection() { private PendingCacheEntry createPendingEntry( RouteLookupRequestKey routeLookupRequestKey, ListenableFuture pendingCall, - @Nullable BackoffPolicy backoffPolicy, - RouteLookupRequest.Reason routeLookupReason) { + @Nullable BackoffPolicy backoffPolicy) { PendingCacheEntry entry = new PendingCacheEntry(routeLookupRequestKey, pendingCall, - backoffPolicy, routeLookupReason); + backoffPolicy); // Add the entry to the map before adding the Listener, because the listener removes the // entry from the map pendingCallCache.put(routeLookupRequestKey, entry); @@ -602,23 +601,20 @@ static final class PendingCacheEntry { private final RouteLookupRequestKey routeLookupRequestKey; @Nullable private final BackoffPolicy backoffPolicy; - private final RouteLookupRequest.Reason routeLookupReason; PendingCacheEntry( RouteLookupRequestKey routeLookupRequestKey, ListenableFuture pendingCall, - @Nullable BackoffPolicy backoffPolicy, RouteLookupRequest.Reason routeLookupReason) { + @Nullable BackoffPolicy backoffPolicy) { this.routeLookupRequestKey = checkNotNull(routeLookupRequestKey, "request"); this.pendingCall = checkNotNull(pendingCall, "pendingCall"); this.backoffPolicy = backoffPolicy; - this.routeLookupReason = routeLookupReason; } @Override public String toString() { return MoreObjects.toStringHelper(this) .add("routeLookupRequestKey", routeLookupRequestKey) - .add("routeLookupReason", routeLookupReason) .toString(); } } From 892eb9103a5334031189c8ebfee81a8f7ac3d947 Mon Sep 17 00:00:00 2001 From: Kannan J Date: Fri, 31 Oct 2025 11:28:26 +0000 Subject: [PATCH 9/9] Style fixes. --- rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java | 3 ++- rls/src/main/java/io/grpc/rls/RlsProtoData.java | 8 ++++---- rls/src/main/java/io/grpc/rls/RlsRequestFactory.java | 1 - .../test/java/io/grpc/rls/CachingRlsLbClientTest.java | 9 ++++++--- 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java b/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java index af77d88c025..2c26d29f14d 100644 --- a/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java +++ b/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java @@ -465,7 +465,8 @@ private void refreshBackoffEntry(BackoffCacheEntry entry) { logger.log(ChannelLogLevel.DEBUG, "[RLS Entry {0}] Calling RLS for transition to pending", entry.routeLookupRequestKey); linkedHashLruCache.invalidate(entry.routeLookupRequestKey); - asyncRlsCall(entry.routeLookupRequestKey, entry.backoffPolicy, RouteLookupRequest.Reason.REASON_MISS); + asyncRlsCall(entry.routeLookupRequestKey, entry.backoffPolicy, + RouteLookupRequest.Reason.REASON_MISS); } } diff --git a/rls/src/main/java/io/grpc/rls/RlsProtoData.java b/rls/src/main/java/io/grpc/rls/RlsProtoData.java index 9e61d62a383..39c404870f9 100644 --- a/rls/src/main/java/io/grpc/rls/RlsProtoData.java +++ b/rls/src/main/java/io/grpc/rls/RlsProtoData.java @@ -45,13 +45,13 @@ static RouteLookupRequestKey create(ImmutableMap keyMap) { @Immutable abstract static class RouteLookupRequest { - /** Names should match those in {@link io.grpc.lookup.v1.RouteLookupRequest.Reason} */ + /** Names should match those in {@link io.grpc.lookup.v1.RouteLookupRequest.Reason}. */ enum Reason { - /** Unused */ + /** Unused. */ REASON_UNKNOWN, - /** No data available in local cache */ + /** No data available in local cache. */ REASON_MISS, - /** Data in local cache is stale */ + /** Data in local cache is stale. */ REASON_STALE; } diff --git a/rls/src/main/java/io/grpc/rls/RlsRequestFactory.java b/rls/src/main/java/io/grpc/rls/RlsRequestFactory.java index 8bb8b8ddfcc..1fed78f4df3 100644 --- a/rls/src/main/java/io/grpc/rls/RlsRequestFactory.java +++ b/rls/src/main/java/io/grpc/rls/RlsRequestFactory.java @@ -27,7 +27,6 @@ import io.grpc.rls.RlsProtoData.GrpcKeyBuilder.Name; import io.grpc.rls.RlsProtoData.NameMatcher; import io.grpc.rls.RlsProtoData.RouteLookupConfig; -import io.grpc.rls.RlsProtoData.RouteLookupRequest; import io.grpc.rls.RlsProtoData.RouteLookupRequestKey; import java.util.HashMap; import java.util.List; diff --git a/rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java b/rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java index 95979d546c5..93c7d0f00ff 100644 --- a/rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java +++ b/rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java @@ -251,7 +251,8 @@ public void get_noError_lifeCycle() throws Exception { resp = getInSyncContext(routeLookupRequestKey); - assertThat(rlsServerImpl.routeLookupReason).isEqualTo(io.grpc.lookup.v1.RouteLookupRequest.Reason.REASON_STALE); + assertThat(rlsServerImpl.routeLookupReason).isEqualTo( + io.grpc.lookup.v1.RouteLookupRequest.Reason.REASON_STALE); assertThat(resp.hasData()).isTrue(); // existing cache expired @@ -310,7 +311,8 @@ public void rls_withCustomRlsChannelServiceConfig() throws Exception { resp = getInSyncContext(routeLookupRequestKey); assertThat(resp.hasData()).isTrue(); - assertThat(rlsServerImpl.routeLookupReason).isEqualTo(io.grpc.lookup.v1.RouteLookupRequest.Reason.REASON_MISS); + assertThat(rlsServerImpl.routeLookupReason).isEqualTo( + io.grpc.lookup.v1.RouteLookupRequest.Reason.REASON_MISS); assertThat(rlsChannelOverriddenAuthority).isEqualTo("bigtable.googleapis.com:443"); assertThat(rlsChannelServiceConfig).isEqualTo(routeLookupChannelServiceConfig); @@ -355,7 +357,8 @@ public void get_throttledAndRecover() throws Exception { rlsServerImpl.routeLookupReason = null; // server responses fakeClock.forwardTime(SERVER_LATENCY_MILLIS, TimeUnit.MILLISECONDS); - assertThat(rlsServerImpl.routeLookupReason).isEqualTo(io.grpc.lookup.v1.RouteLookupRequest.Reason.REASON_MISS); + assertThat(rlsServerImpl.routeLookupReason).isEqualTo( + io.grpc.lookup.v1.RouteLookupRequest.Reason.REASON_MISS); resp = getInSyncContext(routeLookupRequestKey);