Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expiration triggered by cache.asMap iterators doesn't notify RemovalListener #487

Closed
avgustinmm opened this issue Jan 12, 2021 · 3 comments

Comments

@avgustinmm
Copy link

I have an 'expire after access' cache (Caffeine version: 2.8.8) with values that I need to close (grpc managed channels). I have two close points:

  • in removal listener - when value is not used for some time
  • when my service is stopping - iterating cache.asMap().values() in order to close all that left

however, asMap iteration doesn't return expired values (which, seeing some posted issues, seems expected) , but the problem is that RemovalListener doesn't get notifications for expired entries.

There is a simple code that demonstrates this:

final LoadingCache<String, String> cache = Caffeine.newBuilder().weakKeys().removalListener((key, value, reason) -> {
  System.out.println("[" + value + "] remove -(" + reason + ")");
}).expireAfterAccess(1, TimeUnit.SECONDS).build(key -> {
  System.out.println("[" + key + "] create");
  return key;
});

for (int i = 0; i < 10; i++) {
  cache.get(String.valueOf(i));
}

Thread.sleep(2_000);

final Collection<String> values = cache.asMap().values();
System.out.println("cache.asMap() size: " + values.size());
for (final String value : values) {
  System.out.println("->cache.asMap()  value: " + value);
}

I found out that cache.cleanUp() just before cache asMap() will trigger expiration that notifies RemovalListener, but this just could mitigate the issue (will notify for some expirations). However, if an entry expire between cleanUp and asMap - this entry will still be lost.

I believe that triggered by cache.asMap iteration expirations shall notify RemovalListener.
Is there a way to implement that value cleanup correctly with current Caffeine release?

@ben-manes
Copy link
Owner

Would calling cache.invalidateAll() to clear it entirely resolve your shut down problem? In that case you are no longer accepting new work, so the cache accumulates no new entries, and all cache entries should be closed by the removal listener.

Note that the listener invocation is delegated to the Caffeine.executor(executor), which by default is the ForkJoinPool.commonPool(). You might prefer having that work performed on the calling thread so that clearing the cache invokes the listener and you have ensured the resources are closed. That could be done using Caffeine.executor(Runnable::run).

If you want resources to be closed more promptly during normal operations, then consider using Caffeine.scheduler(scheduler). That will use the next expiration time to schedule a cleanUp on a provided thread. If you are on Java 9+ then Scheduler.systemScheduler() uses the JVM's thread, which is usually preferable.

Does that help?

@avgustinmm
Copy link
Author

Thanks for the VERY prompt response! Good hints! It helps a lot.

I'd just thought about replacing cleanUp with invalidataAll. And now I tested it. Seems that it will do the work - RemovalListener gets explicit and expired events on invalidateAll - so I hope such cleanup will cover all values.
So, my problem is solved. I also think I'd use scheduler - thanks for the hint.

Anyway, it seems as a work around - don't you feel it will be more consistent (and matching user expectations) if expiration triggered by asMap iterators are reported to RemovalListener?

@ben-manes
Copy link
Owner

I don't have a strong opinion either way and it seemed harmless to add, so done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants