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

   > The tail latency improvement (**1.75×** at p99) is especially significant: 
`computeIfAbsent` causes severe jitter under lock contention (316–644 ms 
range), while `get+putIfAbsent` stays stable (236–368 ms).
   
   The benchmark seems to measure the miss of 1M cache entries. That's not a 
very realistic scenario that there would be such amount of misses at once.
   
   I think it would be more useful to have a solution that ensures that the 
memory use of the cache is bounded.
   That is something that I have added quite recently to AbstractMetadataStore 
children cache:
   
https://github.com/apache/pulsar/blob/6e577f0bc5a7c3184fdb75d6afdd54a56578aa26/pulsar-metadata/src/main/java/org/apache/pulsar/metadata/impl/AbstractMetadataStore.java#L123-L141
   in https://github.com/apache/pulsar/pull/24868
   The max size is limited to 20% of max heapsize.
   
   For TopicName cache there could be a bytesize limit too. The StringInterner 
solution would be useful at least for namespace and tenant Strings to ensure 
that those don't cause heap duplication if the namespace or tenant caches 
expire or overflow.


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

Reply via email to