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

feat(get): add option replace to cache:get options #121

Closed
wants to merge 1 commit into from

Conversation

bungle
Copy link
Contributor

@bungle bungle commented May 26, 2023

Summary

There is a common need to update a cached value forcibly, before its expiry. The library currently has functions like cache:delete and cache:set that can either delete cached value or set the value and notify other workers about it. But there is one thing that cache:get has that those don't, and it is usage of resty.lock to prevent multiple light-threads (or workers) from trying to call the callback and retrieve the value at the same time.

So while in theory you could do something like this:

cache:delete("key")
local value, err = cache:get("key", nil, function()
  return "value"
end)

This is not nice when dealing with multiple light threads, and leads you to implement extra locking there to make it atomic, something like this:

local resty_lock = require "resty.lock"
local lock = require "resty.lock":new(...)
lock:lock(...)
cache:delete("key")
local value, err = cache:get("key", nil, function()
  return "value"
end)
lock:unlock()

The get already has the locking built-in, so it would be much nicer if we could just utilize that, so the above code could just be written as:

local value, err = cache:get("key", { replace = true }, function()
  return "value"
end)

Use cases:

  1. System relies on external keys and detects missing keys on failure. In this case we may need to fetch new keys to check if they were rotated in external source. So while the keys are currently in cache e.g. in L1 and L2, and not expired, they may not be current, and we need to forcibly check if there are newer ones, and only way to check it is to call the callback.
  2. System relies on external secrets which have a TTL and are rotated based on that. In this case we want to rotate secrets before they expire on caches by using a background timer.

In both cases it would be nice if this could be abstracted with the cache:get as it already implements the needed locking, and we can put the code in right locations. It also simplifies the usage of this library in such cases as the locks don't need to be implemented outside, and there is potential delay between delete and the callback so it is hard to even put things in exactly right places. cache:delete causes async events happening, before the callback is even called, while it would be more optimal to broadcast invalidation in these cases after the callback is called (similar to cache:set).

Alternatively, it would be possible to change cache:set to accept callbacks, but it feels like cache:get is a better interface as it is kinda THE interface that is used most of the time.

This commit adds a boolean option replace to opts table of cache:get(key, opts ...) to allow skipping L1 and early return of non-expired L2, and make it call the callback. And it also sends invalidations after replacing the value in L2.

### Summary

There is a common need to update a cached value forcibly, before its expiry.
The library currently has functions like `cache:delete` and `cache:set` that
can either delete cached value or set the value and notify other workers
about it. But there is one thing that `cache:get` has that those don't, and
it is usage of `resty.lock` to prevent multiple light-threads (or workers)
from trying to call the callback and retrieve the value at the same time.

So while in theory you could do something like this:
```lua
cache:delete("key")
local value, err = cache:get("key", nil, function()
  return "value"
end)
```

This is not nice when dealing with multiple light threads, and leads you
to implement extra locking there to make it atomic, something like this:

```
local resty_lock = require "resty.lock"
local lock = require "resty.lock":new(...)
lock:lock(...)
cache:delete("key")
local value, err = cache:get("key", nil, function()
  return "value"
end)
lock:unlock()
```

The `get` already has the locking built-in, so it would be much nicer if
we could just utilize that, so the above code could just be written as:

```lua
local value, err = cache:get("key", { replace = true }, function()
  return "value"
end)
```

Use cases:
1. System relies on external keys and detects missing keys on failure.
   In this case we may need to fetch new keys to check if they were
   rotated in external source. So while the keys are currently in cache
   e.g. in L1 and L2, and not expired, they may not be current, and we
   need to forcibly check if there are newer ones, and only way to check
   it is to call the callback.
2. System relies on external secrets which have a TTL and are rotated
   based on that.
   In this case we want to rotate secrets before they expire on caches
   by using a background timer.

In both cases it would be nice if this could be abstracted with the
`cache:get` as it already implements the needed locking, and we can
put the code in right locations. It also simplifies the usage of this
library in such cases as the locks don't need to be implemented
outside, and there is potential delay between `delete` and the
`callback` so it is hard to even put things in exactly right places.
`cache:delete` causes async events happening, before the callback is
even called, while it would be more optimal to broadcast invalidation
in these cases after the callback is called (similar to `cache:set`).

Alternatively, it would be possible to change `cache:set` to accept
callbacks, but it feels like `cache:get` is a better interface as it
is kinda _THE_ interface that is used most of the time.

This commit adds a `boolean` option `replace` to `opts` table of
 `cache:get(key, opts ...)` to allow skipping L1 and earlt return
of non-expired L2, and make it call the callback. And it also
sends invalidations after replacing the value in L2.

Signed-off-by: Aapo Talvensaari <aapo.talvensaari@gmail.com>
@bungle bungle closed this Jun 9, 2023
@bungle bungle deleted the feat/get-replace-option branch June 9, 2023 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant