Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Normalize courier calls to simplify :commit handling #295

Merged
merged 2 commits into from
Jul 11, 2022
Merged

Conversation

whitfin
Copy link
Owner

@whitfin whitfin commented Jul 11, 2022

This fixes #290.

I'm going to label this as a bug because I think it was unintended, and thus has now been fixed. Currently if you execute many fetch/4 calls at the same time, it becomes impossible to know which of them actually wrote a value to the cache:

iex(1)> Cachex.start(:test)
{:ok, #PID<0.528.0>}
iex(2)> my_func = fn _key -> :timer.sleep(2000) && "value" end
#Function<44.65746770/1 in :erl_eval.expr/5>
iex(3)> for _ <- 1..10 do
...(3)>   spawn(fn -> IO.inspect(Cachex.fetch(:test, "key", my_func)) end)
...(3)> end
[#PID<0.539.0>, #PID<0.540.0>, #PID<0.541.0>, #PID<0.542.0>, #PID<0.543.0>,
 #PID<0.544.0>, #PID<0.545.0>, #PID<0.547.0>, #PID<0.548.0>, #PID<0.549.0>]
{:commit, "value"}
{:commit, "value"}
{:commit, "value"}
{:commit, "value"}
{:commit, "value"}
{:commit, "value"}
{:commit, "value"}
{:commit, "value"}
{:commit, "value"}
{:commit, "value"}

This makes chaining calls on (such as then providing an expiration for your record) will run for every call instead of just the one which triggered the write. Ideally, this would run once in the process which was first to initiate the fetch. This PR fixes this to be the case, thus making the recommended expire pattern work correctly.

iex(1)> Cachex.start(:test)
{:ok, #PID<0.528.0>}
iex(2)> my_func = fn _key -> :timer.sleep(2000) && "value" end
#Function<44.65746770/1 in :erl_eval.expr/5>
iex(3)> for _ <- 1..10 do
...(3)>   spawn(fn -> IO.inspect(Cachex.fetch(:test, "key", my_func)) end)
...(3)> end
[#PID<0.539.0>, #PID<0.540.0>, #PID<0.542.0>, #PID<0.543.0>, #PID<0.544.0>,
 #PID<0.545.0>, #PID<0.546.0>, #PID<0.547.0>, #PID<0.548.0>, #PID<0.549.0>]
{:commit, "value"}
{:ok, "value"}
{:ok, "value"}
{:ok, "value"}
{:ok, "value"}
{:ok, "value"}
{:ok, "value"}
{:ok, "value"}
{:ok, "value"}
{:ok, "value"}

With this in place, chained calls based on :commit will only fire in a single process instead of all of them. This is far better ergonomically, and also in terms of performance as we're not wasting many cache calls unnecessarily.

@whitfin whitfin added this to the v3.5.0 milestone Jul 11, 2022
@whitfin whitfin self-assigned this Jul 11, 2022
@whitfin whitfin merged commit 1ed5bcf into master Jul 11, 2022
@whitfin whitfin deleted the issue-290 branch November 24, 2022 00:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Only one call to fetch should return :commit
1 participant