smengcl commented on code in PR #4567:
URL: https://github.com/apache/ozone/pull/4567#discussion_r1232844063
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java:
##########
@@ -246,15 +246,8 @@ public OmSnapshotManager(OzoneManager ozoneManager) {
}
};
- // init LRU cache
- this.snapshotCache = CacheBuilder.newBuilder()
- // Indicating OmSnapshot instances are softly referenced from the
cache.
- // If no thread is holding a strong reference to an OmSnapshot instance
- // (e.g. SnapDiff), the instance could be garbage collected by JVM at
- // its discretion.
- .softValues()
Review Comment:
Hi @GeorgeJahad , the plan was to do `softValues()` first (because that was
much easier and quicker to impl), then do custom cache impl to entirely replace
the `LoadingCache` (as the title suggests).
We recently are suspecting a small issue with `softValues()`, where
seemingly too many instances are open at a time and each are holding quite a
few files open. Thus we filed #4884 as a mitigation.
It is plausible to make the interface pluggable. But with the only advantage
of rolling back to `LoadingCache` impl when custom cache impl goes wrong. Would
you like to file a new jira for this? (requires adding a new OM config, adding
a new interface, tweaking `RC#close` / `decrementRefCount`, etc.)
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
