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

Lock-per-key functionality when using custom loader functions #122

Open
lukasbindreiter opened this issue Feb 13, 2024 · 2 comments
Open

Comments

@lukasbindreiter
Copy link

Hi,

I was skimming through the code a little bit, trying to figure out if there is support for a use case for locking just a specific key in the cache while a custom loader (thats slow, like e.g. making an HTTP request) is running.

For example, imaging something like this:

websiteLoader := ttlcache.LoaderFunc[string, string](
	func(c *ttlcache.Cache[string, string], key string) *ttlcache.Item[string, string] {
		// load website content
                content := ...  // make http request to fetch url=key
		item := c.Set(key, content)
		return item
	},
)
websiteCache := ttlcache.New[string, string](
	ttlcache.WithLoader[string, string](loader),
)

Now imagine I have three goroutines called at the exact same time:

// goroutine1
content := websiteCache.Get("https://www.some.site")
// goroutine2
content := websiteCache.Get("https://www.other.site")
// goroutine3
content := websiteCache.Get("https://www.some.site")

Is there a way to add a lock-per-cache-key, e.g. to make goroutine3 block while goroutine1 is still running, but let goroutine2 continue as normal? From what I saw in the code, I think not, right?

There is a lockAndLoad parameter here, but if I understood this correctly it locks the whole cache, so in the above example goroutine2 would block while goroutine1 is running, right?

Do you think this would be a feature worth adding?
I was thinking about implementing a wrapper around ttlcache that supports something like that, but then I thought this could be a potential useful feature in general?

Of course one thing about this is that error handling is maybe a bit unclear then, so I also see an argument that this is already too much magic happening by the cache, instead of letting the user implement it, but I would be interested in opinions in any case.

@lukasbindreiter lukasbindreiter changed the title Item key lock for custom loader functions Lock-per-key functionality when using custom loader functions Feb 13, 2024
@swithek
Copy link
Contributor

swithek commented Feb 19, 2024

That's an interesting idea, but I'm not sure how big the scope of this issue would be, because:

  • If method calls can block each other for an unspecified amount of time (e.g., the loader might be loading something from a db), each of these calls should be able to be cancelled (a context would have to be used). There's an issue for context-related functionality already here: Add GetContext() method and LoaderContext interface #120
  • Should this be implemented at the cache level or perhaps there could be a loader wrapper, like SuppressedLoader, that would handle per-key locking? At the moment, the latter would make more sense.

@lukasbindreiter
Copy link
Author

I like the suggestion of the GetContext api in the issue you linked, I think it could make a lot of sense here as well.

GetContext(context.Context, key K, opts... Options) (Item, error)

This way we can pass in a context (for cancellation) and we can also get back potential errors by the loader.

If I adapt my loader a little bit to something like the LoaderContext proposed in #120 it could look like this, right?

websiteLoader := ttlcache.LoaderContext[string, string](
	func(c *ttlcache.Cache[string, string], ctx context.Context, key string) (*ttlcache.Item[string, string], error) {
		// load website content
                content, err := ...  // make http request to fetch url=key
                if err != nil {
                    return nil, err
                }
		item := c.Set(key, content)
		return item
	},
)

But this is now actually a separate from this issue (lock-per-key) but that could become a separate option then, e.g.

websiteCache := ttlcache.New[string, string](
	ttlcache.WithLoaderContext[string, string](websiteLoader),
        ttlcache.WithLockAndLoad(),  // a new option
)

Then the usage could look like this:

ctx := context.Background()

// goroutine1
content, err := websiteCache.GetContext(ctx, "https://www.some.site")
// goroutine2
content, err := websiteCache.GetContext(ctx, "https://www.other.site")
// goroutine3
content, err := websiteCache.GetContext(ctx, "https://www.some.site")

Alternatively, instead of a new option such as ttlcache.WithLockAndLoad a wrapped loader could also be used of course.
What do you think, would an API like this make sense? (It would at the very least solve our exact use-case at hand 😄 )

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

No branches or pull requests

2 participants