divijvaidya commented on code in PR #14483:
URL: https://github.com/apache/kafka/pull/14483#discussion_r1366804550


##########
storage/src/main/java/org/apache/kafka/storage/internals/log/RemoteIndexCache.java:
##########
@@ -151,9 +151,8 @@ private Cache<Uuid, Entry> initEmptyCache(long maxSize) {
                 .weigher((Uuid key, Entry entry) -> {
                     return (int) entry.entrySizeBytes;
                 })
-                // removeListener is invoked when either the entry is 
invalidated (means manual removal by the caller) or
-                // evicted (means removal due to the policy)
-                .removalListener((Uuid key, Entry entry, RemovalCause cause) 
-> {
+                // evictionListener is invoked when RemovalCause.wasEvicted() 
is true

Review Comment:
   Please update the comment here to explicitly specify whether it would be 
executed sync or async with cache eviction. You can say:
   
   This listener is invoked each time an entry is being automatically removed 
due to eviction. The cache with invoke this listener during the atomic 
operation to remove the entry (refer: 
https://github.com/ben-manes/caffeine/wiki/Removal), hence, care must be taken 
to ensure that this operation is not expensive.
   
   Note that this listener is not invoked when RemovalCause from cache is 
EXPLICIT or REPLACED (e.g. on Cache.invalidate(), Cache.put() etc.) For a 
complete list see: 
https://github.com/ben-manes/caffeine/blob/e12bb5cf8a6d1b1318835ad1b63901cd338de3dc/caffeine/src/main/java/com/github/benmanes/caffeine/cache/RemovalCause.java#L23.
 Hence, any operation required after removal from cache must be performed 
manually for these scenarios.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to