StevenLuMT commented on PR #3522: URL: https://github.com/apache/bookkeeper/pull/3522#issuecomment-1281078578
> > My question is if would rather make more sense to optimize for the fact that the masterkey is empty in the overwhelmingly major part of the cases. > > +1 In Pulsar, each ledger's default password is `"".getBytes(StandardCharsets.UTF_8)` and most clusters are not set ledger passwords, and the `masterKey` is generated from ledger password, which means most of the ledgers' masterKey are default value. We can optimize this case. > > Changing the cache from `ConcurrentLongHashMap ` to Guava cache will introduce extra cost no matter in `putIfAbsent` or eviction. This cache get and put is a high-frequency operation in Bookie, and I'm not sure about the impact on the addEntry performance. > > Maybe we can try to optimize the default `masterKey` case first to see the achivments. @hangc0276 Do you understand the purpose of this PR change? The current replacement of ConcurrentLongHashMap is not because there is any problem with ConcurrentLongHashMap, but because the current masterKeyCache has not been cleaned up How to optimize? Please give some specific advice? All I can think of is to regularly clean up the masterKeyCache logic. Now that guava cache has done this, there is no need to do this implementation. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
