dlmarion commented on code in PR #40:
URL:
https://github.com/apache/accumulo-classloaders/pull/40#discussion_r2691024035
##########
modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/util/DeduplicationCache.java:
##########
@@ -51,6 +69,7 @@ public VALUE computeIfAbsent(final KEY key, final
Supplier<PARAMS> params) {
}
public boolean anyMatch(final Predicate<KEY> keyPredicate) {
+ canonicalWeakValuesCache.cleanUp();
Review Comment:
Without the `cleanup` call here, the line below returns `true` even if the
value has been nulled out due to `weakValues`. I don't think the `.asMap()`
call alone is not enough to force a cleanup of the entry.
##########
modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/util/DeduplicationCache.java:
##########
@@ -33,15 +39,27 @@
*/
public class DeduplicationCache<KEY,PARAMS,VALUE> {
+ private static final Logger LOG =
LoggerFactory.getLogger(DeduplicationCache.class);
+
private final Cache<KEY,VALUE> canonicalWeakValuesCache;
private final Cache<KEY,VALUE> expireAfterAccessStrongRefs;
private final BiFunction<KEY,PARAMS,VALUE> loaderFunction;
+ private final RemovalListener<KEY,VALUE> defaultListener = new
RemovalListener<>() {
+ @Override
+ public void onRemoval(KEY key, VALUE value, RemovalCause cause) {
+ LOG.info("Entry removed due to {}. K = {}, V = {}", cause, key, value);
+ }
+ };
+
public DeduplicationCache(final BiFunction<KEY,PARAMS,VALUE> loaderFunction,
- final Duration minLifetime) {
+ final Duration minLifetime, RemovalListener<KEY,VALUE> listener) {
this.loaderFunction = loaderFunction;
- this.canonicalWeakValuesCache = Caffeine.newBuilder().weakValues().build();
- this.expireAfterAccessStrongRefs =
Caffeine.newBuilder().expireAfterAccess(minLifetime).build();
+ this.canonicalWeakValuesCache = Caffeine.newBuilder().weakValues()
+ .evictionListener(listener == null ? defaultListener :
listener).build();
+ this.expireAfterAccessStrongRefs =
Caffeine.newBuilder().expireAfterAccess(minLifetime)
+ .evictionListener(listener == null ? defaultListener : listener)
+ .scheduler(Scheduler.systemScheduler()).build();
Review Comment:
Without the `.scheduler` added, only a subsequent call to
`DeduplicationCache.computeIfAbsent` would remove the expired entry. If that
call is never made, or not made for a long time, then the entry remains. There
is no other code that manipulates the `expireAfterAccessStrongRefs` Cache where
the maintenance would be performed.
--
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]
