Dangling reference on cache_evict when using referenced keys

[ahovgaard]

I have observed an issue with referenced keys and dangling references on cache_evict. This is based on the UserAccounts example from the docs (https://hexdocs.pm/nebulex/Nebulex.Caching.Decorators.html#cacheable/3-referenced-keys).

I have a mock database implementation, like so:

defmodule MyApp.DB do
  use Agent

  def start_link(_) do
    Agent.start_link(fn -> %{} end, name: __MODULE__)
  end

  def upsert(record) do
    Agent.update(__MODULE__, &Map.put(&1, record.id, record))
  end

  def get(id) do
    Agent.get(__MODULE__, &Map.get(&1, id))
  end

  def delete(id) do
    Agent.update(__MODULE__, &Map.delete(&1, id))
  end

  def get_by_field(field, value) do
    Agent.get(__MODULE__, fn state ->
      state
      |> Map.values()
      |> Enum.find(&(Map.get(&1, field) == value))
    end)
  end
end

And the implementation using the caching decorators:

defmodule MyApp.UserAccounts do
  use Nebulex.Caching

  alias MyApp.{Cache, DB}

  @decorate cacheable(cache: Cache, key: id)
  def get_user_account(id) do
    DB.get(id)
  end

  @decorate cacheable(cache: Cache, key: email, references: & &1.id)
  def get_user_account_by_email(email) do
    DB.get_by_field(:email, email)
  end

  @decorate cache_evict(cache: Cache, key: rec.id)
  def delete_user_account(rec) do
    DB.delete(rec.id)
  end
end

Consider the following interaction:

iex(1)> MyApp.DB.upsert(%{id: 123, email: "123@example.com"})
:ok
iex(3)> MyApp.UserAccounts.get_user_account_by_email "123@example.com"
%{id: 123, email: "123@example.com"}
iex(4)> MyApp.Cache.all() |> Enum.map(&{&1, MyApp.Cache.get(&1)}) |> Enum.into(%{})
%{
  123 => %{id: 123, email: "123@example.com"},
  "123@example.com" => {:"$nbx_cache_keyref", nil, 123}
}
iex(5)> MyApp.UserAccounts.delete_user_account(%{id: 123, email: "123@example.com"})
:ok
iex(6)> MyApp.Cache.all() |> Enum.map(&{&1, MyApp.Cache.get(&1)}) |> Enum.into(%{})
%{"123@example.com" => {:"$nbx_cache_keyref", nil, 123}}                            # <-- dangling reference
iex(7)> MyApp.DB.upsert(%{id: 123, email: "456@example.com"})
:ok
iex(8)> MyApp.Cache.all() |> Enum.map(&{&1, MyApp.Cache.get(&1)}) |> Enum.into(%{})
%{"123@example.com" => {:"$nbx_cache_keyref", nil, 123}}
iex(9)> MyApp.UserAccounts.get_user_account(123)
%{id: 123, email: "456@example.com"}
iex(10)> MyApp.Cache.all() |> Enum.map(&{&1, MyApp.Cache.get(&1)}) |> Enum.into(%{})
%{
  123 => %{id: 123, email: "456@example.com"},
  "123@example.com" => {:"$nbx_cache_keyref", nil, 123}
}
iex(11)> MyApp.UserAccounts.get_user_account_by_email "123@example.com"
%{id: 123, email: "456@example.com"}
iex(12)>
nil
iex(13)> MyApp.UserAccounts.get_user_account_by_email "456@example.com"
%{id: 123, email: "456@example.com"}
iex(14)> MyApp.Cache.all() |> Enum.map(&{&1, MyApp.Cache.get(&1)}) |> Enum.into(%{})
%{
  123 => %{id: 123, email: "456@example.com"},
  "123@example.com" => {:"$nbx_cache_keyref", nil, 123},
  "456@example.com" => {:"$nbx_cache_keyref", nil, 123}
}

In step 5, the cache_evict is invoked, which only specifies the id key. From the documentation:

On the other hand, in the eviction function update_user_account/1, since the user record is stored only once under the user id, we just need to set the user id in the option :key; no need to specify multiple keys like in the previous case.

This now leaves a dangling reference from the email "123@example.com" to the nonexistent ID 123.

In step 7, a new record is written to the DB (say from another machine running the app) using the old ID 123. When we read the record using that ID in step 9, the previously dangling reference from "123@example.com" now points to the new record with the mismatched email.

My expectation would be that the cache_evict would remove references to the deleted key, so that no dangling references would be left.

In this particular scenario, I could specify

@decorate cache_evict(cache: Cache, keys: [rec.id, rec.email])

however, this is exactly what the documentation suggests shouldn't be necessary. Additionally, if the delete_user_account/1 function received just the ID as argument, I wouldn't be able to reference the email in :keys.

[cabol]

Hey @ahovgaard!!

Thanks for spotting this issue and for the very detailed information. I know the references are not being removed on the evict decorator (just the given key but not its references), but you're right, it is an issue when a referencing key is updated, like you said, we end up with dangling keys in the cache, which I agree is not desirable. Let me look into it, and will try to come back with a solution soon.

[hissssst]

Deleting references during GC or backtracking them can be a solution

[cabol]

I've been thinking about this and the best way to address it, but there is no quick fix, and any solution has pros and cons. So, let me break down the problem and address it incrementally.

• The dangling keys. IMHO, I wouldn't consider this critical because there are some ways to handle it (will explain later). I agree that is not ideal, but providing a solution like backtracking the key references may bring also downsides as well in terms of memory and performance. So, I'd like to explore other alternatives I have in mind, but that will be for Nebulex v3; which is the highest priority now. In the meantime, to address this, there are a few things we can do. The first one (and perhaps the simplest one), is using the :ttl option. Another alternative, and the one I recommend and use myself, is having a separate cache for keeping the references only, a local cache with the local adapter for instance. This way, one could provide a different eviction or GC configuration, to run the GC more often and keep the references cache clean.

• Inconsistencies due to a dangling key (I'm talking about this: In step 7, a new record is written to the DB (say from another machine running the app) using the old ID 123. When we read the record using that ID in step 9, the previously dangling reference from "123@example.com" now points to the new record with the mismatched email.). This issue, on the contrary, I think is critical to fix, because we don't want to retrieve an inconsistent value. I've been working on something to address it. The idea is to leverage the match function when retrieving a reference to ensure the value is what we're expecting, and also be able to set a TTL for a reference (plus other stuff). However, it will be available in Nebulex v3; like I mentioned before, Nebulex v3 is the top priority now. I will keep you posted.

• Lastly, and perhaps the main problem, is the inaccurate and misleading documentation. I will fix the documentation based on what we have discussed here, explaining the issue and the possible immediate alternatives to address it.

Thank you very much for all the comments, suggestions, and so on, they're very well appreciated. I'll keep you posted about the progress.

[cabol]

I've pushed a change fixing the docs about the cache_evict decorator.

Also, as mentioned before, the inconsistency issue will be addressed on Nebulex v3 (https://github.com/cabol/nebulex/blob/v3.0.0-dev/lib/nebulex/caching/decorators.ex#L742-L805).