Get returns nothing when TTL set

[PawelJ-PL]

Hi,

It looks, that it's impossible to get value with CaffeineCache, if it was inserted with TTL.

Sample code:

import cats.effect.{ExitCode, IO, IOApp}
import scalacache.caffeine.CaffeineCache
import scalacache.{Cache, get, put}

object Main extends IOApp {
  val cache: IO[CaffeineCache[IO, String]] = CaffeineCache[IO, String]

  private def testCache(implicit c: Cache[IO, String]): IO[Unit] = for {
  _ <- put("foo")("bar", Some(scala.concurrent.duration.FiniteDuration(10, "days")))
  result <- get("foo")
  _ <- IO(println(result))
  } yield ()

  override def run(args: List[String]): IO[ExitCode] = cache.flatMap(implicit c => testCache.map(_ => ExitCode.Success))
}

It prints None but it should be Some(bar). If I remove TTL, it works fine. My suspicion is that the following condition can be incorrect:

scalacache/modules/caffeine/src/main/scala/scalacache/caffeine/CaffeineCache.scala

Line 34 in 1fb5879

.flatMap(_.filterA(Entry.isExpired[F, V]))

[PawelJ-PL]

The updated code confirms my suspicions. The value is only available after it expires.

import cats.effect.{ExitCode, IO, IOApp}
import scalacache.caffeine.CaffeineCache
import scalacache.{Cache, get, put}

object Main extends IOApp {
  val cache: IO[CaffeineCache[IO, String]] = CaffeineCache[IO, String]

  private def testCache(implicit c: Cache[IO, String]): IO[Unit] = for {
  _ <- put("foo")("bar", Some(scala.concurrent.duration.FiniteDuration(7, "seconds")))
  result <- get("foo")
  _ <- IO(println(result))
  _ <- IO(Thread.sleep(10000))
  result2 <- get("foo")
  _ <- IO(println(result2))
  } yield ()

  override def run(args: List[String]): IO[ExitCode] = cache.flatMap(implicit c => testCache.map(_ => ExitCode.Success))
}

[Qi77Qi]

Hi, I think I'm running into similar issue.

Here's a snippet to reproduce

import com.github.benmanes.caffeine.cache.Caffeine
import scala.concurrent.duration._

val underlyingCache = Caffeine.newBuilder().build[String, scalacache.Entry[String]]()
val cache: CaffeineCache[IO, String] = CaffeineCache[IO, String](underlyingCache)

val cacheWithTtl = cache.cachingF("keyTtl")(Some(1 hour))(IO(println("fetching value")).as("v1"))

scala> cacheWithTtl.unsafeRunSync()
fetching value
val res9: String = v1

scala> cacheWithTtl.unsafeRunSync()
fetching value
val res10: String = v1

scala> underlyingCache.asMap()
val res11: java.util.concurrent.ConcurrentMap[String,scalacache.Entry[String]] = {k1=Entry(v1,None), keyTtl=Entry(v1,Some(1970-01-08T02:23:22.244Z))}

Note "fetching value" gets printed out twice, which means the function is run twice to fetch the value. I think it's because expiration isn't set properly, somehow expiresAt is 1970-01-08T02:23:22.244Z when ttl is set. If I don't set ttl, then expiresAt is set to None, and cache behaves correctly.

[kubukoz]

Might be related to the CE3 migration. I'll look at it when I have a sec

[PawelJ-PL]

I think my problem would be resolved by PR #562.

Regarding date 1970 it's probably because, that cache is using montonic time, but it should't cause any problems.

[kubukoz]

Merged #562, should be published soon in https://github.com/cb372/scalacache/actions/runs/1217511186

@PawelJ-PL if you can check that snapshot (might need to add a Sonatype snaphots resolver) and let me know it's working for you, I'll publish another milestone :)

[PawelJ-PL]

With version 1.0.0-M4+18-e1b730d9-SNAPSHOT it always returns None (after and before expirtation) :/

[kubukoz]

Gonna play around with it some more, then

[ben-manes]

Caffeine has native support for variable expiration and the put(key, value, duration) method. This is available in cache.policy().expiresVariably(). It requires configuring with expiresAfter(Expiry), which could default to an infinite duration. It would be nice if as much as possible is delegated to the cache provider since it can implement functionality in a more efficient and tested manner (same with memoization).

[PawelJ-PL]

I started using this approach when I came across this bug (and incidentally discovered that scalacache does not delete expired entries but only filters them out)

[kubukoz]

I'll take a PR that moves to the caffeine-native expiration methods, unless I remember to take care of it myself when I have a few moments 😅

[yurique]

https://github.com/cb372/scalacache/pull/562/files:

clock.realTimeInstant.map(_.isBefore(expiration))

expiration is in monotonic time, realTimeInstant will never be "isBefore"

[kubukoz]

Caught up on the caffeine API...

If the cache was not constructed with variable expiration or the implementation does not support these operations, an empty Optional is returned.

@ben-manes I'm not a Caffeine user, do you know how likely it is that a user-provided cache would fall into that case? (forcing us to fall back to our own expiration mechanism)

In the meantime, I'll just use realTimeInstant everywhere.

[yurique]

BTW the Entry class with the monotonic/realtime mix up is not caffeine-specific. It's in core.
I tried to fix it in the PR above.

[ben-manes]

It could be quite common as users decide what features to enable at construction. That dictates what data structures are allocated and the work to perform. There is no requirement for any policy (size, time, reference), so accepting a raw Cache leaves it unknown. It is rare for a local cache to ever use an explicit put rather than a memoizing loader, so most users never touch cache.policy() for feature-specific apis.

I suspect that ScalaCache tried to abstract away too many concepts (local, remote, memoization, expiration, vendor agnostic, etc). In this case you were coerced into reinventing the feature for your API, and at best redirect to an optimized wrapper if the expiration support was enabled on the cache. Of potential interest is the opposite approach of simply making Caffeine idiomatic in Scala as done in this library.