Skip to content

rls: Make LinkedHashLruCache non-threadsafe #11203

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -261,6 +262,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 @@ -343,6 +350,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 @@ -892,15 +900,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
Loading