Skip to content

rls: LruCache interface and implementation - #6799

Merged
creamsoup merged 9 commits into
grpc:masterfrom
creamsoup:rls_cache
Apr 10, 2020
Merged

rls: LruCache interface and implementation#6799
creamsoup merged 9 commits into
grpc:masterfrom
creamsoup:rls_cache

Conversation

@creamsoup

Copy link
Copy Markdown
Contributor

depends on Adaptive throttler (#6749)

@creamsoup
creamsoup requested a review from dapengzhang0 March 13, 2020 03:28
@creamsoup creamsoup closed this Mar 13, 2020
@creamsoup creamsoup reopened this Mar 13, 2020
@creamsoup
creamsoup marked this pull request as ready for review March 13, 2020 03:28
@dapengzhang0 dapengzhang0 self-assigned this Mar 31, 2020
Comment thread rls/src/main/java/io/grpc/rls/internal/LruCache.java
Comment thread rls/src/main/java/io/grpc/rls/internal/LinkedHashLruCache.java Outdated
Comment thread rls/src/main/java/io/grpc/rls/internal/LruCache.java
Comment thread rls/src/main/java/io/grpc/rls/internal/LruCache.java
Comment thread rls/src/main/java/io/grpc/rls/internal/LinkedHashLruCache.java Outdated
Comment thread rls/src/main/java/io/grpc/rls/internal/LinkedHashLruCache.java Outdated
Comment thread rls/src/test/java/io/grpc/rls/internal/LinkedHashLruCacheTest.java Outdated
Comment thread rls/src/main/java/io/grpc/rls/internal/LinkedHashLruCache.java
Comment thread rls/src/main/java/io/grpc/rls/internal/LinkedHashLruCache.java
Comment thread rls/src/main/java/io/grpc/rls/internal/LinkedHashLruCache.java
Comment thread rls/src/main/java/io/grpc/rls/internal/LinkedHashLruCache.java Outdated
Comment thread rls/src/main/java/io/grpc/rls/internal/LinkedHashLruCache.java
Comment thread rls/src/main/java/io/grpc/rls/internal/LinkedHashLruCache.java Outdated
@Override
@CheckReturnValue
public final boolean hasCacheEntry(K key) {
// call get to handle expired

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not clear what the comment means. Is it necessary?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i can improve the comment
if you get why it call readInternal instead of LinkedHashMap#contains without much thinking i am more than happy to remove it.

@dapengzhang0 dapengzhang0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly LGTM, some small nit.

Comment thread rls/src/main/java/io/grpc/rls/internal/LinkedHashLruCache.java Outdated
Comment thread rls/src/main/java/io/grpc/rls/internal/LinkedHashLruCache.java Outdated
Comment thread rls/src/main/java/io/grpc/rls/internal/LinkedHashLruCache.java
Comment thread rls/src/test/java/io/grpc/rls/internal/LinkedHashLruCacheTest.java Outdated
@creamsoup
creamsoup merged commit 4974b51 into grpc:master Apr 10, 2020
@creamsoup
creamsoup deleted the rls_cache branch April 10, 2020 17:52
dfawley pushed a commit to dfawley/grpc-java that referenced this pull request Jan 15, 2021
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jun 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants