[GitHub] [pulsar] lhotari commented on pull request #16996: feat:return true if entry exists in cache

[GitBox <gi...@apache.org>]

lhotari commented on PR #16996:
URL: https://github.com/apache/pulsar/pull/16996#issuecomment-1208978792

   > @leizhiyuan I'm thinking why we need to the check here.
   > 
   > ```
   >             // EntryCache.insert: duplicates entry by allocating new entry and data. so, recycle entry after calling
   >             // insert
   >             // Entry cache doesn't copy the data if entry already exist into the cache.
   >             // Backlog read tries to add entry into cache which can try to add duplicate entry into cache.
   >             if (ml.entryCache.insert(entry)) {
   >                 entry.release();
   >             }
   > ```
   > 
   > any cases that we should not release the entry if can't insert to the entry cache?
   > 
   > IMO, we should remove the check here, the entry owner should not decide whether to release based on whether it is added to the cache or not. The cache implementation should add the ref count if the entry added to the cache.
   
   good catch @codelipenghui . I reverted this change in #17007 


-- 
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: commits-unsubscribe@pulsar.apache.org

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