Lock-per-key functionality when using custom loader functions

[lukasbindreiter]

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.

[swithek]

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]

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 😄 )

[thespags]

Any updates on this issue?

[lukasbindreiter]

I implemented this internally for us back then, as a ConcurrentTTLCache utility we are using quite heavily in our code-base.
It's a quite short, 70-line implementation that uses ttlcache under the hood and doesn't support all of the configuration options that ttlcache does, since I only included those that actually matter for us.

Here is how we use it:

fetchURL := func(ctx context.Context, url string) ([]byte, error) {
    // use http client to fetch the url, and return the response body as byte slice
    // ...
    return response, nil 
}

cacheDuration := 10 * time.Minute
urlCache := NewConcurrentTTLCache(fetchURL, cacheDuration)


content, err := urlCache.Get("https://example.com/blablabla")

but it also supports exactly this concurrency issue outlined here:

func processURL(urlCache *ConcurrentTTLCache, url string) {
    content, err := urlCache.Get(url)
    if err != nil {
        return
    }
    fmt.Println("Processing content:")
    fmt.Println(len(content))
}


go processURL(urlCache, "https://example.com/page1")
go processURL(urlCache, "https://example.com/page2")
go processURL(urlCache, "https://example.com/page3")
go processURL(urlCache, "https://example.com/page1")

would fetch page 1-3 in parallel, while the last goroutine would block until the first one is done and then return the cached result.

Errors are not cached, so if the first goroutine fails, goroutine 4 would re-run the request instead after goroutine 1 is done.

It's probably not mature/ready enough to directly end up as part of the library but if you want @thespags I can share our implementation 👍

Or alternatively could also submit it as a PR as a starting point for that as a feature, not sure if you would be interested in that as well @swithek?

[thespags]

I ended up writing a key-based lock in front, and I don't know if I'll keep using the library. I'm not sure what the use cases are for not wanting a key-based lock.

[swithek]

Have you tried using the SuppressedLoader wrapper? It was specifically created to suppress multiple concurrent calls to the same key retrieval function.

[thespags]

@swithek, I'm not sure how that helps the problem of locking the entire cache when performing long-running set functions. (Looking more closely at the code, I assume it's to allow the cache to use an internal map and not sync the map.)

Why not have GetOrSetFunc with something like the following to not lock during the call to fn()?

        // Lock while we try to get the value.
        c.items.mu.Lock()
	elem := c.getWithOpts(key, false, opts...)
        c.items.mu.Unlock()

	if elem != nil {
		return elem, true
	}

	setOpts := options[K, V]{
		ttl: c.options.ttl,
	}
	setOpts = applyOptions(setOpts, opts...) // used only to update the TTL

        value :=  fn()

        // Lock again now that we are setting the value.
        c.items.mu.Lock()
	item := c.set(key,, setOpts.ttl)
        c.items.mu.Unlock()

	return item, false

[lukasbindreiter]

@swithek Ah SuppressedLoader looks interesting, it's apparently usingsingleflight.do under the hood. (Rather than keeping it's own map[key, sync.Mutex] instead)

What makes it unsuitable for our use-case though still is the way it just suppresses errors and doesn't forward them up until to the caller.
https://github.com/jellydator/ttlcache/blob/v3/cache.go#L889

I think there would be a valid usecase for a cache and a loader that has an error as part of its return value, especially here since the need to lock per key often times indicates an IO error which you may want to handle at the caller site.

[swithek]

@lukasbindreiter the SuppressedLoader cannot suppress/handle errors by itself, the underlying Loader interface simply doesn't return any errors, so there's nothing to propagate. Error handling is a separate feature (which can be further discussed here: #120) and has nothing to do with duplicate request prevention.

@thespags If I understand your use case correctly, what you're describing isn't the same as the feature/issue mentioned in the original post, which focuses on repeated Gets/fetches, rather than GetOrSets. I think GetOrSetFunc is the only place where a long running fn() is wrapped by a mutex; other places, like Gets that use Loaders, are already non-blocking. So this specific method (i.e., GetOrSetFunc) should probably be updated.

[thespags]

If I'm reading @lukasbindreiter's comments correctly, I believe we want the same thing, but we're going about it differently: I was using GetOrSet and @lukasbindreiter is using get with a loader. I want the ability to make long-running loads not block the rest of the cache, but still block the same keys and avoid making the same load call.

Now with the ttlcache and mutex ttlcache for each key, I'm doing the following ( I see my early suggestion wouldn't lock on the call to fn() for per key):

  item := cache.Get(key)
  if item != nil {
       return item.Value()
  }
  mutexes, _ := mutexes.GetOrSet(key, ...)
  
  mutex.Value().Lock()
  defer mutex.Value().Unlock()
  
  // check if we lost the race
  item := cache.Get(key)
  if item != nil {
       return item.Value()
  }
  // we won the race, do the long running request
  value := doSomeLongHttpCall()
  
  cache.Set(key, value)
  return item