get_or_store with a fallback feature to check for value elsewhere and load into cache

[churcho]

I would like to perform a get_or_store, check if the value exists in the cache, if not, I would like to call db store lookup function, reload the result of that into the cache and if that return is {:ok, nil} I would like to inform the cache that the value doesn't exist.

My use case is as follows: I want to save a copy of my cache entry to the database so that I can persist that value over my cluster (Riak). If a user checks for a value that is missing in the cache, the system would check if that value exists in the db store, if it does then it loads it into the cache.

On cache delete, I would also like to delete the instance of that key from the data store.

[sasa1977]

I would like to perform a get_or_store, check if the value exists in the cache, if not, I would like to call db store lookup function, reload the result of that into the cache and if that return is {:ok, nil} I would like to inform the cache that the value doesn't exist.

You can normally do this with get_or_store:

ConCache.get_or_store(cache_id, key, fn ->
  {:ok, value} = read_from_db(key)
  value
end)

The problem here is that if the value is nil (which, judging from your description, means that there's nothing in the database), we'll still cache it. Maybe that's what you want, and in that case, the solution above is the way to go.

Otherwise, conditional storing is currently not supported for get_or_store. It might be interesting to add this feature, but until that happens, there are a couple of workarounds.

Probably the simplest one is to set a very low TTL for nil values:

ConCache.get_or_store(cache_id, key, fn ->
  case read_from_db(key) do
    {:ok, nil} -> %ConCache.item(ttl: 1, value: nil)
    {:ok, existing_value} -> existing_value
  end
end)

This is a bit hacky, since nil is still going to be cached, but not for long (it depends on TTL settings of your cache).

A better solution is to use ConCache.isolated to conditionally store the item. The code is however more elaborate:

# First try a non-isolated read from the cache.
if (value = ConCache.get(cache_id, key)) != nil do
  value
else
  # Perform isolated operation on this key. No one else can modify the value for 
  # this key until we finish.
  ConCache.isolated(cache_id, key, fn ->
    # We need to try another read, because someone could have stored the value 
    # before we got here.
    if (value = ConCache.get(cache_id, key)) != nil do
      # The value is in the cache after all -> just return it.
      value
    else
      # There's no value in the cache, and we're the only ones who can modify 
      # the cache.
      case read_from_db(key) do
        {:ok, nil} -> 
          # Nothing in db -> just return `nil` without storing into the cache
          nil
        {:ok, existing_value} ->
          # Something in db -> store the value into the cache, and return it
          ConCache.put(cache_id, key, value)
          value
      end
    end
  )
end

This would be simpler if get_or_store supported conditional storing. The implementation should be simple (mostly similar to the snippet above). However, changing the existing function would lead to a breaking change. So maybe a better approach would be to introduce another function (e.g. get_or_prime) and deprecate get_or_store.

I need to give this some more thought. In the meantime, let me know if the proposals above work for you.

[sasa1977]

My use case is as follows: I want to save a copy of my cache entry to the database so that I can persist that value over my cluster (Riak). If a user checks for a value that is missing in the cache, the system would check if that value exists in the db store, if it does then it loads it into the cache.

On cache delete, I would also like to delete the instance of that key from the data store.

You may want to be careful here, because there are some race conditions with this approach. For example, if you delete an entry from the cache, but then the system stops before you persist the change, the entry is not deleted from the database.

Usually, a better way is to modify the database first, and then update the cache later. If the system crashes, the cache will reprime itself from the database, and everything will be in proper sync.

[churcho]

You are absolutely right, database insert first and based on return, cache save follows thereafter. I like the ConCache.isolated approach more so I will try it out and revert. Thanks

[churcho]

I need to give this some more thought. In the meantime, let me know if the proposals above work for you.

Which params are passed as part of the callback? Can it be used to check the ttl param so that it doesn't run a db_save operation on update if the ttl is say 1 or less than n?

[sasa1977]

Which params are passed as part of the callback? Can it be used to check the ttl param so that it doesn't run a db_save operation on update if the ttl is say 1 or less than n?

Something like that could be done, but I think it's a bit hacky. So instead I'd like to consider a cleaner approach. Perhaps fetch_or_store which returns {:ok, value} or :error would be the cleanest approach here.

[Secretmapper]

+1 Conditional storing would be great! My use case is basically just keeping a stale value in my cache if my function fails (better old value than nothing) because my resolution function relies on a very unreliable external service

The isolated approach works great until this becomes first class though!

[sasa1977]

@Secretmapper @churcho I think it would be nice to introduce the following functions: fetch, fetch_or_store, dirty_fetch_or_store. Pull requests are welcome :-)

[stefanchrobot]

@sasa1977 How about an extra argument for get_or_store? Something like: get_or_store(cache_id, key, store_fun, cond_fun \\ fn _ -> true end) That's backwards-compatible and looks fairly easy to implement. I'll gladly contribute a PR.

My use case is that I call external services and those calls return an ok/error tuple. Errors are most likely transient, so I don't want to cache them. I'm using get+put, but that doesn't lock so I might end up doing multiple HTTP calls in the end.

[sasa1977]

@stefanchrobot I'm wondering if the proposed fetch_or_store solves the same problem in a more idiomatic way? This would allow you to do something like:

fetch_or_store(cache_id, key, &invoke_external_service/0)

Where invoke_external_service/0 returns {:ok, result} | :error?

So basically, fetch_or_store will cache the result if lambda returns {:ok, result}. Otherwise if it returns :error, nothing is cached. The result of fetch_or_store is also in the shape of {:ok, result} | :error. We could also add fetch_or_store! which crashes the client on error.

What do you think?

[stefanchrobot]

Oh, now I get it. You mean more idiomatic as in Access.fetch vs Access.get?

In my case I'd like to return some reason with the error: {:error, reason}, but we can still make this work by saying:

• fetch_or_store returns whatever invoke_external_service returns,
• if invoke_external_service returns a tuple {:ok, result}, fetch_or_store will cache the result

The only issue seems to be that fetch_or_store will have a different result than Access.fetch. What do you think?

[sasa1977]

We don't need to mirror the access interface, since con_cache is not a data structure, and so it doesn't make sense to implement the access protocol. We just use the convention to distinguish between fetch and get.

Returning an error reason makes sense. So then the contract would be something like this:

@type fetch_or_store_result :: {:ok, store_value} | {:error, any)
@spec fetch_or_store(t, key, (() -> result)) :: result when result: fetch_or_store_result

Does that sound ok?

[sasa1977]

Actually, that signature isn't right, because store_value is not necessarily the same as returned value (it can also be ConCache.Item.t). So I guess we'd have to settle with something like this:

@type fetch_or_store_fun() :: (() -> {:ok, store_value} | {:error, any})
@spec fetch_or_store(t, key, fetch_or_store_fun) :: {:ok, value) | {:error, any}

[stefanchrobot]

Looks good, I'll start the work on the PR.