search

ben-manes / caffeine

A high performance caching library for Java
Apache License 2.0
15.8k stars 1.59k forks source link

Support a `getAll(keys, func)` method in manual caches #310

Closed rtyley closed 5 years ago

rtyley commented 5 years ago

As I griped about on twitter, I'm looking for a cache with async bulk-loading of keys, while permitting a logging context (in the loading code) that comes from the requesting call (permitting zipkin-like tracing).

Ben suggested by DM that a getAll(keys, func) method on AsyncCache would fulfil that need, which would be great (although he also mentioned he hasn't needed it personally, as he uses request-scoped DI... does that mean in Ben's setup the loader-code has access to the request-scoped DI context...? If that's a thread-local thing I don't think that would work for my multi-threaded code...).

This might be asking too much but I think that 'ideal' behaviour would be for implementations of AsyncCache to help guard against cache stampedes by only passing keys to func for values that aren't already in-flight. This could mean that the signature of the getAll method would look something like this:

CompletableFuture<Map<K, V>> getAll(
  Iterable<? extends K> keys,
  Function<Iterable<? extends K>, Map<K,V>> mappingFunction func)

Incidentally I tried to sign the CLA for this repo, as specified in the contributing guidelines, but could only get this error on clicking 'I agree': image

ben-manes commented 5 years ago

The code for AsyncLoadingCache#getAll could easily be generalized to call a function instead of the supplied loader. So I think this is a pretty minor change, and copying the unit tests to over cover this variation. We should probably do the same for LoadingCache?

For the method signature, it would be that and a variant where the function also accepts an executor and returns the future. That way if your bulk call is asynchronous you can pass back the client's future.

I use Guice and most often my context would be request-scoped, e.g. the calling user's details. I would then be able to inject in a Provider<UserContext> and resolve to the scoped instance during the call. When setting up other threads, I establish the scope for that execution (e.g. via ServletScopes.scopeRequest). It is likely a thread-local thing, but using DI to propagate things instead of passing them through manually. For Java that's quite natural, but may feel awkward in Scala's idioms.

hmm, not sure about the CLA - will look. I'm lenient about it since it's not really required, but seemed like an easy thing to do to make companies feel more comfortable adopting or contributing.

rtyley commented 5 years ago

Ah thanks for that - I would have liked to have contributed a PR for this but to be honest I don't think I'd be confident doing that refactoring.

ben-manes commented 5 years ago

AsyncCache#getAll is now implemented. It should be done for Cache as well for consistency, before releasing. I'll close this when I get to that.

ben-manes commented 5 years ago

Released in v2.8