ttl option of Cachex.fetch

[qhwa]

Hi there, I was using Cachex to make a simple caching, some codes like below:

{_, ret} =
  Cachex.fetch(
    :my_cache,
    cache_key,
    fn _ ->
      case do_some_stuff() do
        pages when is_list(pages) ->
          {:commit, pages}

        others ->
          {:ignore, []}
      end
    end,

    # here I am using `ttl` option
    ttl: :timer.seconds(1)
  )

ret

Unfortunately it doesn't work. After 1 second the cache is still available.

I went to the source code and figured out that the reason is that ttl option is not supported in fetch. I ended up with changing Cachex.fetch into Cachex.put .

Will it be better with ttl supported in fetch ?

thanks!

[whitfin]

This has been discussed before, but the decision was to avoid replicating options everywhere. In your case, you can do the following:

with { :commit, _val } <- Cachex.fetch(:my_cache, cache_key, &my_func/1) do
  Cachex.expire(:my_cache, cache_key, :timer.seconds(1)
  { :commit, val }
end

[qhwa]

Thanks for replying and sorry for the duplicated question :)

Your solution is much cleaner and the codes might be:

with { :commit, val } <- Cachex.fetch(:my_cache, cache_key, &my_func/1) do
  Cachex.expire(:my_cache, cache_key, :timer.seconds(1))
  val
else
  {:ok, val} ->
     val

  _ ->
    []
end

I might have to repeat this pattern several where and end up with a macro or something like that.
If I am wrong please correct me.

This is not a big problem for developers though.

[karlseguin]

I know this is a closed issue, and it wasn't the first of its kind, but I just wanted to add a +1 to the thought that I think this should be improved. As-is, it's both not-intuitive (and not really documented) and less than ideal.

[whitfin]

@karlseguin the issue is that there is a very obvious way of setting an expiration (it's right there in the public API). If I were to add a :ttl option to fetch, then I should also add a :ttl option to warmers, and anything else where this is also a perceived problem. Rather than then duplicate that option logic everywhere, I'd rather recommend everyone simply use the public API as shown above.

Even at this point, I'm unconvinced that put should have had a :ttl option in the first place - I probably should have not brought that option across and made expire explicit everywhere.

[mindok]

I'll add my voice - I'm really very happy with Cachex, but I just wasted an hour or two debugging why :ttl wasn't working with fetch after refactoring code with put. To me, get, fetch and put are the main, day to day interactions with the cache and the API should be orthogonal. If an option isn't available in fetch it shouldn't be in put and vice versa.

I appreciate the fact the API is well documented, but it isn't obvious as it stands. Maybe documenting the possibilities of options in Cachex.fetch would help, because currently, it is easy to infer that :ttl should do something in the absence of definitive information for that function.

FWIW - my preference would be for the :ttl option to work with fetch because there are a lot of places in the codebase where you are likely to use fetch but not many where you are using warmers - it helps keep the business logic code cleaner.

[whitfin]

@mindok while I totally appreciate the difference in the API, the correct way to go is to simply remove it from put. Clearly I can't do that until a major, so we're stuck as it is for now.

While it sounds pretty simple to "just add it to fetch", in reality it's way more than that because there are many ways to first initialize a key. An obvious example is incr and decr, where being able to assign an expiration on initialization isn't possible without (comparably) large overhead. It really does need to be everything, or nothing.

I think maybe the documentation could be improved specifically on put to make it clear that the ttl option is a shorthand specifically because that function is used so often - with a reference to expire so it's obvious how to do it yourself. I'm happy to accept a PR for this, or I can get to it (hopefully) this weekend!

[mindok]

Thanks @whitfin - makes sense now. I've also had a dig around the codebase - I can see there's a fair bit of nuance there to keep things performant.

I'll put together a PR with the following:

• Update put docs as per your suggestion
• Update fetch docs to add an options section which makes particular mention that :ttl will be ignored, as will any other options. Plus a link to expire
• I'm thinking of adding a paragraph or two to ttl-implementation.md. It talks about how expirations are honoured, but not how they are set. I'm only a day into using Cachex so I might scramble things a little, but hopefully it will be a useful starting point.

[thbar]

We got bitten by this as well. I wonder if raising an error when ttl: is passed to fetch wouldn't be a good idea 😄

Thanks for the clear proposal at #195 (comment), much appreciated!