Caches not expiring

[tfwright]

I have a few relatively simple caches that look like this:

      Supervisor.child_spec(
        {ConCache, [name: :active_users_cache, ttl_check_interval: :timer.seconds(10), global_ttl: :timer.minutes(1)]},
        id: {ConCache, :active_users_cache}
      ),
     # .... more with exp
      Supervisor.child_spec(
        {ConCache, [name: :jobs_cache, ttl_check_interval: false]},
        id: {ConCache, :jobs_cache}
      )

The active_users_cache just contains a user id and a timestamp (their latest request), added like so:

      ConCache.put(:active_users_cache, [id], %ConCache.Item{value: DateTime.utc_now()})

But AFAICT, it's never expiring. When I inspect the ets table I see entries like this:

  {"250", ~U[2024-02-02 18:08:06.498382Z]},
  {"26", ~U[2024-02-02 18:26:50.815029Z]},
  {"236", ~U[2024-02-02 18:49:55.533782Z]},
  {"279", ~U[2024-02-02 18:49:55.812741Z]},
  {"276", ~U[2024-02-02 18:07:00.917742Z]},
  {"34", ~U[2024-02-02 18:49:01.212477Z]},
  {"275", ~U[2024-02-02 16:48:50.586609Z]},
  {"14", ~U[2024-02-02 16:55:30.335637Z]}

My understanding would be with my ttl settings I should never expect to see 2 entries more than 70 secs apart since the put should overwrite the last value, meaning the timestamp should reflect the last time the ttl was set.

Aside from exp the values all look correct and it works as expected.

Is there anything obvious I am doing wrong? What is the recommended way to go about troubleshooting this?

[sasa1977]

There are two types of data you can pass to put:

1. A plain value (e.g. ConCache.put(:active_users_cache, [id], DateTime.utc_now()))
2. A ConCache.Item instance (as in your example)

In the former case, the item ttl is the "global" ttl (provided to ConCache.start_link). In the latter case, the ttl is taken from the :ttl field of the item. If you don't provide that value explicitly when creating a struct, the default is :infinity.

So I think it would work if you used the 1st approach (provide just the raw value). The 2nd option is meant to be used if you want to override the default cache ttl. If you prefer passing the item, then I think setting :ttl to :renew should do the job.

[tfwright]

Thanks for the quick response and explanation. I adjusted the code to use a plain value and now everything is working as expected. A couple of suggestions:

1. I'm having a hard time seeing this info in the README. Maybe highlight the facts that a) either a plain value or Item struct can be passes as value and b) what happens if an Item is passed without a value
2. In terms of default ttl, either using the global ttl config, or requiring a ttl be explicitly set on Item would be a slighter less surprising API behavior

I realize 2. might involve a breaking change so may not be worth it currently, but I would be happy to open PRs for both of these on your advice.

Thanks again!

[sasa1977]

I'm having a hard time seeing this info in the README.

Yeah, it's kind of there, but not obvious. The first few examples use value (which is supposed to mean raw value). Then later, a few TTL examples use %ConCache.Item{} to demonstrate fine-grained row-level TTL control (which is the only reason why you'd pass the item).

It's also worth noting that this duality in the specs (https://hexdocs.pm/con_cache/ConCache.html#put/3), but the function docs are missing, so the difference is again not clear.

I agree that we should make this more obvious, at the very least in README. We should perhaps use some concrete value in the first set of examples. Then in TTL examples we could have an explicit sentence which explains that item struct can be passed for fine-grained TTL control.

2. In terms of default ttl, either using the global ttl config, or requiring a ttl be explicitly set on Item would be a slighter less surprising API behavior

Yeah, I think making this field required would be the best option. It is a breaking change, but at least it will break compilation, instead of causing non-obvious behaviour change.

TBH, looking back, I don't like the existence of ConCache.Item. It's added for pure convenience, nothing more. If I were doing this today, I'd just accept operation-specific parameters as options (e.g. ConCache.put(key, value, ttl: overide_ttl)). That's a bit more tedious in the lib implementation, but the interface would be clearer, which is IMO a good trade-off. This requires more work (and it's a breaking change), but it's the option I currently prefer.

But that said, I'm fine with a simpler PR which improves the readme and requires item ttl to be explicitly set.