Conversation
There was a problem hiding this comment.
We should add it to javaDoc of getBulk that its expected to return a map with no null values.
There was a problem hiding this comment.
I did, on Cache interface docs
|
LGTM |
|
Needs docs |
There was a problem hiding this comment.
wondering if we should also warm up level1 cache when there is a miss in l1 cache and hit in l2 cache ?
There was a problem hiding this comment.
|
@drcrallen will add docs once the code has been ok'ed |
There was a problem hiding this comment.
was it required to change semantics of this method or this is done to be "cleaner" ?
There was a problem hiding this comment.
@himanshug well, it's not technically required, but would make the code both more complex and slower if we have to check the value of each key in the map in order to filter them out in order to do a bulk L2 request.
There was a problem hiding this comment.
Since the upstream code doesn't require the null entries for cache misses, I figured it was easier to enforce it.
|
LGTM |
There was a problem hiding this comment.
does getBulk guarantee a mutable map is returned?
There was a problem hiding this comment.
it does not, the question is do we want to add the overhead of copying
There was a problem hiding this comment.
both memcached and mapcache use hashmaps. Can we just change the contract for getBulk to return mutable maps?
There was a problem hiding this comment.
I changed it to make a copy if necessary, if it becomes a performance drag we can revisit
4 participants
Can be used to combine a local (L1) cache with a remote (L2) cache to reduce cache latency.