Skip to content

Commit

Permalink
rls: Make LinkedHashLruCache non-threadsafe
Browse files Browse the repository at this point in the history
CachingRlsLbClient already calls it with a lock held. The only reason
the cache needs to manage the lock itself is for the periodic cleanup.
Let the consumer of the cache handle the timer.
  • Loading branch information
ejona86 committed May 29, 2024
1 parent c31dbf4 commit 5c6b808
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 161 deletions.
23 changes: 12 additions & 11 deletions rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ final class CachingRlsLbClient {
// LRU cache based on access order (BACKOFF and actual data will be here)
@GuardedBy("lock")
private final RlsAsyncLruCache linkedHashLruCache;
private final Future<?> periodicCleaner;
// any RPC on the fly will cached in this map
@GuardedBy("lock")
private final Map<RouteLookupRequest, PendingCacheEntry> pendingCallCache = new HashMap<>();
Expand Down Expand Up @@ -177,10 +178,10 @@ private CachingRlsLbClient(Builder builder) {
new RlsAsyncLruCache(
rlsConfig.cacheSizeBytes(),
new AutoCleaningEvictionListener(builder.evictionListener),
scheduledExecutorService,
ticker,
lock,
helper);
periodicCleaner =
scheduledExecutorService.scheduleAtFixedRate(this::periodicClean, 1, 1, TimeUnit.MINUTES);
logger = helper.getChannelLogger();
String serverHost = null;
try {
Expand Down Expand Up @@ -267,6 +268,12 @@ static Status convertRlsServerStatus(Status status, String serverName) {
serverName, status.getCode(), status.getDescription()));
}

private void periodicClean() {
synchronized (lock) {
linkedHashLruCache.cleanupExpiredEntries();
}
}

/** Populates async cache entry for new request. */
@GuardedBy("lock")
private CachedRouteLookupResponse asyncRlsCall(
Expand Down Expand Up @@ -349,6 +356,7 @@ final CachedRouteLookupResponse get(final RouteLookupRequest request) {
void close() {
logger.log(ChannelLogLevel.DEBUG, "CachingRlsLbClient closed");
synchronized (lock) {
periodicCleaner.cancel(false);
// all childPolicyWrapper will be returned via AutoCleaningEvictionListener
linkedHashLruCache.close();
// TODO(creamsoup) maybe cancel all pending requests
Expand Down Expand Up @@ -887,15 +895,8 @@ private static final class RlsAsyncLruCache

RlsAsyncLruCache(long maxEstimatedSizeBytes,
@Nullable EvictionListener<RouteLookupRequest, CacheEntry> evictionListener,
ScheduledExecutorService ses, Ticker ticker, Object lock, RlsLbHelper helper) {
super(
maxEstimatedSizeBytes,
evictionListener,
1,
TimeUnit.MINUTES,
ses,
ticker,
lock);
Ticker ticker, RlsLbHelper helper) {
super(maxEstimatedSizeBytes, evictionListener, ticker);
this.helper = checkNotNull(helper, "helper");
}

Expand Down
Loading

0 comments on commit 5c6b808

Please sign in to comment.