-
Notifications
You must be signed in to change notification settings - Fork 275
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
Add cache of recently seen blobs in CloudBlobStore #1225
Conversation
…ary Azure requests.
Codecov Report
@@ Coverage Diff @@
## master #1225 +/- ##
============================================
+ Coverage 69.51% 69.53% +0.02%
- Complexity 5552 5560 +8
============================================
Files 432 432
Lines 33943 33978 +35
Branches 4342 4349 +7
============================================
+ Hits 23596 23628 +32
- Misses 9146 9149 +3
Partials 1201 1201
Continue to review full report at Codecov.
|
ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java
Outdated
Show resolved
Hide resolved
ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java
Outdated
Show resolved
Hide resolved
missingKeys = store.findMissingKeys(keys); | ||
assertTrue("Expected no missing keys", missingKeys.isEmpty()); | ||
// getBlobMetadata should not have been called a second time. | ||
verify(dest).getBlobMetadata(anyList()); |
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.
Perhaps a unit test to confirm that the LRU nature of the cache works as expected (set the cache limit artificially low, add PUTs over that number, confirm that redoing the PUTs yields (number of PUTs - limit) unskipped cloud PUTs)?
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.
Done (method a bit different).
ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java
Outdated
Show resolved
Hide resolved
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.
LGTM.
To reduce unnecessary Azure round trips when multiple replicas send same data.