Feature request: support putting a mapping with variable expiration

[ahadadi]

The existing way to specify expiration on a mapping requires adding the needed context to calculate the expiration to the key or value.
It would be useful to have put & putIfAbsent methods with duration parameter added to VarExpiration, so there would be no need to add the context to the key or value.

[ben-manes]

I plan on bundling this with #156, which asks for an async asMap() view. I may take the plunge and make an AsyncCache (non-loading)`, though still not sure that would be a good idea.

In yours, I need to remember to handle async caches since the puts go directly into the cache (rather than adapted through LocalAsyncLoadingCache). Just a note to myself as if I forget it would put V into the map instead of CompletableFuture<V>.

[davidhoyt]

@ben-manes I didn't understand your response (and perhaps the request neither). I was going to submit a feature request myself and wasn't sure, based on your response, if my request was the same.

What I was seeking to do was to specify an expiration per entry. So entry "foo" expires in 1 hour but entry "bar" expires in 2 hours. Is that already possible and does this issue capture that same request?

[ben-manes]

That is possible using an Expiry, but only by evaluating the entry as a callback. This is similar to Weigher where the weight is determined separate from the caller's put or load.

This request is to provide methods so that the caller can pass the duration explicitly. That would override the provided Expiry which would still be required for compatibility. This is possible and just requires me to find the time.

At the technical level this is done using a timer wheel so that it is O(1). That was the hard blocker that kept me from doing it in Guava as doing this right was not trivial. This further improvement is low cost.

[ben-manes]

I have this prototyped locally. I only need guidance on the API methods. I think the request is,

V put(K key, V value, long duration, TimeUnit unit);
V putIfAbsent(K key, V value, long duration, TimeUnit unit);

Behind the scenes, the put adapter method will wrap the calls in custom Expiry and delegate it to the cache's internal put(key, value, expiry, onlyIfAbsent). The other public put methods all delegate to that internal one, but resolve to the configured expiry. I presume that you want the value returned per Map?

[ben-manes]

Also, I presume that the duration is only set if the write was successful? As in, putIfAbsent will not modify the time if failing to a read.

[ben-manes]

and in that case, would a read delegate to the configured expiry or have no effect?

[ben-manes]

and if the cache is asynchronous, I presume the internal future is joined on to make the call appear synchronous? That would be required if you expect to have a return value. That leads me to prefer not returning the value unless there is a strong argument for it (as one generally does not manage the state of the cache explicitly).

[ben-manes]

That leaves me a bit stuck. If the return value, then the call is synchronous which is undesirable. If no return value then the previous state is unknown, especially on a putIfAbsent which is conditional. Does put return nothing and putIfAbsent return a boolean?

The code and tests are mostly done, but I don't know what the proper API is yet. Please advise!

[ben-manes]

/**
 * Associates the {@code value} with the {@code key} in this cache if the specified key is not
 * already associated with a value and returns null, else returns the current value. This method
 * differs from {@link Map#putIfAbsent} by substituting the configured {@link Expiry} with the
 * specified duration and by not returning the existing value.
 *
 * @param key the key with which the specified value is to be associated
 * @param value value to be associated with the specified key
 * @param duration the length of time from now when the entry should be automatically removed
 * @param unit the unit that {@code duration} is expressed in
 * @return <tt>true</tt> if this cache did not already contain the specified entry
 * @throws IllegalArgumentException if {@code duration} is negative
 * @throws NullPointerException if the specified key or value is null
 */
default boolean putIfAbsent(@Nonnull K key, @Nonnull V value,
    @Nonnegative long duration, @Nonnull TimeUnit unit) {
  // This method will be abstract in version 3.0.0; added & implemented as of version 2.6.0
  throw new UnsupportedOperationException();
}

/**
 * Associates the {@code value} with the {@code key} in this cache. If the cache previously
 * contained a value associated with the {@code key}, the old value is replaced by the new
 * {@code value}. This method differs from {@link Cache#put} by substituting the
 * configured {@link Expiry} with the specified duration.
 *
 * @param key the key with which the specified value is to be associated
 * @param value value to be associated with the specified key
 * @param duration the length of time from now when the entry should be automatically removed
 * @param unit the unit that {@code duration} is expressed in
 * @throws IllegalArgumentException if {@code duration} is negative
 * @throws NullPointerException if the specified key or value is null
 */
default void put(@Nonnull K key, @Nonnull V value,
    @Nonnegative long duration, @Nonnull TimeUnit unit) {
  // This method will be abstract in version 3.0.0; added & implemented as of version 2.6.0
  throw new UnsupportedOperationException();
}

[ben-manes]

Merged into master what I think is the appropriate fix, but please reply tomorrow before I release it.

Also please let me know if any other methods are missing. I see Play's API has a getOrElseUpdate(k, func, seconds). However it is backed by APIs that are not atomic, e.g. Ehcache 2, so operations are racy get-compute-put. This makes sense since many users would swap the cache out for memcached or redis, which cannot be computed through. So I don't think having special cases for all the various methods makes sense, and the requested two cover the common usages.

[ahadadi]

Thanks for implementing that!

I didn't get that bit:

"Also, I presume that the duration is only set if the write was successful? As in, putIfAbsent will not modify the time if failing to a read. And in that case, would a read delegate to the configured expiry or have no effect?"

Are you referring to a race between a read and putIfAbsent that both try to (only) update the expiration? Or to a race where the read is trigerring value loading? or something else?

Note that the javadoc for putIfAbsent return value is inconsistent with what you ended up implementing.

I guesss that the root cause of your API doubts is that LoadingCache & AsyncLoadingCache have different semantics for put / putIfAbsent. Also AsyncLoadingCache has a void put method with CompletableFuture, while its sync counterpart does not.
The most consistent implementation is to split VarExpiration to a sync and async version, and keep the same semantics as in their non expiration setting counterparts.
Do you think it's worth the extra API surface area?

[ben-manes]

Are you referring to a race between a read and putIfAbsent that both try to (only) update the expiration? Or to a race where the read is trigerring value loading? or something else?

I meant that the expiration can be set on a read (time to idle) or a write (time to live). If the putIfAbsent fails then it usually returns the present value, which means it becomes a read. The duration could be set regardless or only if on a successful write. I did the latter, leaving the entry unchanged, as I think that is the desired intent.

Note that the javadoc for putIfAbsent return value is inconsistent with what you ended up implementing.

Thanks, I’ll give it another pass. I think I see what you mean.

Do you think it's worth the extra API surface area?

I think it’s not worth the surface area. Note that the policy is obtained through the sync view, so it should be consistent with that API. At first I imagined that a lower level API would expect more Map-like put methods, and we do offer asMap. So the return value confusion was trying to be consistent with Map, but the void method is consistent with Cache. For your case of memcached, it’s set does not return the value so I guess the extra information isn’t necessary for most usages.

[ben-manes]

I've looked over a few different libraries to see usage patterns.

• Play's cache: void return type for set(k, v, seconds)
• dropwizard-caching-bundle: void return type on put(k, response)
• Netflix's EVCache: a memcached client, so boolean return type
• ExpiringMap: the old value for put(k, v, dur, unit) and no time setting on putIfAbsent. I couldn't find usage in Github inspecting the return value.

I couldn't find other well known frameworks that use a cache adapter api with an expiration option. Might be due to lack of imagination of what to look for.

I think the APIs with the revised JavaDoc are okay. I'll release this evening if there's no other feedback. Thanks!