[GitHub] [pulsar] merlimat edited a comment on pull request #10480: [Broker] Fix race condition in invalidating ledger cache entries

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

merlimat edited a comment on pull request #10480:
URL: https://github.com/apache/pulsar/pull/10480#issuecomment-833757513


   > As mentioned earlier, that is prevented with AbstractCASReferenceCounted in Pulsar in most cases.
   
   At this point, we can actually get rid of `AbstractCASReferenceCounted`. It was added in #2995  as a temporary measure to work around a change in behavior in Netty. The Netty issue was then fixed in 4.1.32 and we don't need the special treatment anymore.
   
   The change https://github.com/apache/pulsar/blob/dcaa1d350fec920ef0ba32b4aeecd2e9604d4824/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/EntryCacheImpl.java#L187-L196 was added as a way to handle the same bug, but I agree that it's really dangerous in that we don't really what got wrong, as in where the corruption was...
   
   As for the 2nd part (the Cursor.readEntryFailed), that seems ok to me. The `entries` there are the partial entries that were already read before. They might be coming from the cache or directly from bookies. In any case, the ref-count was increased when they came out of the cache, so we're required to release (and it should be safe to do so). 
   
   


-- 
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.

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