-
Notifications
You must be signed in to change notification settings - Fork 4k
11243: RLS cleanups #12085
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
11243: RLS cleanups #12085
Conversation
44a715f to
8974a30
Compare
…manikag-FIX_11243_RLS # Conflicts: # rls/src/main/java/io/grpc/rls/LinkedHashLruCache.java
ejona86
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In CachingRlsLbClient, shouldInvalidateEldestEntry() is managed based on time, as we make sure an entry is not too new. But this is valid, even if it is misleading on how the class is intended to be used.
| @Test | ||
| public void testFitToLimitWithReSize() { | ||
|
|
||
| Entry entry1 = new Entry("Entry1", ticker.read() + 10,4); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add space after comma (ditto in next test)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed the review comments
| assertThat(localCache.fitToLimit()).isEqualTo(true); | ||
|
|
||
| assertThat(localCache.values().contains(entry1)).isFalse(); | ||
| assertThat(localCache.values().containsAll(Arrays.asList(entry2,entry3))).isTrue(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add space after comma
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed the review comments
shivaspeaks
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merge branch master into your branch to include #12169 so that your Linux artifacts Kokoro build passes.
addressed the review comments provided on #11243
Fixes : #11243