-
Notifications
You must be signed in to change notification settings - Fork 12
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
[draft] True Cache API #14
base: main
Are you sure you want to change the base?
Conversation
* @param fetcher called if not value is associated with the key in the cache | ||
* @return a future containing the cached entry if present, otherwise the value returned by the supplier | ||
*/ | ||
CompletionStage<V> get(K key, Function<K, V> fetcher); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From an asynchronous perspective, should Function<K, V>
be Function<K, CompletionStage<V>>
instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could provide even a blocking and non blocking variant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to redeem null
or use Optional
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could provide even a blocking and non blocking variant.
Not sure about providing a blocking variant since it can be derived from the non blocking one? A blocking one can be created out of taking V
and converting it to CompletionStage<V>
, but I see that more like a static helper method or something. I want to avoid overloading.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to redeem null or use Optional?
Eventually should be Optional. So this would eventually look like this?
CompletionStage<Optional<V>> get(K key, Function<K, CompletionStage<Optional<V>> fetcher);
* @param comparator called if a cached value exists when the fetcher returns | ||
* @return a future containing the cached value or null if not values exist | ||
*/ | ||
CompletionStage<V> get(K key, Function<K, V> fetcher, Comparator<V> comparator); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From an asynchronous perspective, should Function<K, V>
be Function<K, CompletionStage<V>
instead?
The problem then is how the comparator, you'd probably need a custom comparator that has something like this?
CompletionStage<Integer> compare(CompletionStage<T> o1, CompletionStage<T> o2)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comparator should still work as is, we would just have to do use CompletionStage#thenCombine
internally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wburns Yes.
* @param remover function that deletes data from source | ||
* @return a future that is completed when key has been removed from both cache and source | ||
*/ | ||
CompletionStage<Void> remove(K key, Consumer<K> remover); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder, what are the semantics around the Consumer
here? Shouldn't it be okay if the returned CompletionStage
just has the K
value type? Then the user can chain off of the completion there in a non blocking way if needed. If we invoke this Consumer
we lose ability to be non-blocking from our perspective.
Something like
remove(key).thenAccept(..)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The point of the Consumer<K>
is that it defines how the key is removed in the source of truth (e.g. a database). Just like the get()
, we want the cache to have some control over this so that it can easily know when the source of truth has completed.
The problem with your suggestion is that the cache cannot know when the source of truth remove has completed.
The code above is blocking, so to be fully non-blocking it should look more like this:
CompletionStage<Void> remove(K key, Function<K, CompletionStage<Void>> remover);
* @param comparator called if a cached value exists when the fetcher returns | ||
* @return a future containing the cached value or null if not values exist | ||
*/ | ||
CompletionStage<V> get(K key, Function<K, V> fetcher, Comparator<V> comparator); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comparator should still work as is, we would just have to do use CompletionStage#thenCombine
internally.
* @param fetcher called if not value is associated with the key in the cache | ||
* @return a future containing the cached entry if present, otherwise the value returned by the supplier | ||
*/ | ||
CompletionStage<V> get(K key, Function<K, V> fetcher); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
* @param fetcher called if not value is associated with the key in the cache | ||
* @return a future containing the cached entry if present, otherwise the value returned by the supplier | ||
*/ | ||
CompletionStage<V> get(K key, Function<K, V> fetcher); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could provide even a blocking and non blocking variant.
Design draft for a true Cache API.
It's centered around our Hibernate experience, which is the most popular cache user we have for Infinispan. The suggested APIs would not be compatible with current Hibernate cache region APIs but that's fine.
Methods in which values are fetched/updated/removed in the source of truth data source are crucial to how the cache behaves externally, that's why they're integral to the cache API, e.g.
fetcher
function.I'm not 100% sure about the exact asynchronous aspects. I'll be making some comments in the PR with doubts I currently have.