open-feature / flagd

A feature flag daemon with a Unix philosophy
https://openfeature.dev
Apache License 2.0
520 stars 63 forks source link

[FEATURE] Caching #160

Closed skyerus closed 1 year ago

skyerus commented 2 years ago

Requirements

Research a solution for client-side caching of flagd.

The caching solution should be implemented in flagd providers. Avoiding creating any issues in the provider repositories until a solution is found.

AlexsJones commented 2 years ago

Can you elaborate a little more? I thought that react+redux would be adequate for caching client side requests?

skyerus commented 2 years ago

Can you elaborate a little more? I thought that react+redux would be adequate for caching client side requests?

This is following on from https://github.com/open-feature/flagd/issues/159. By client I don't necessarily mean a browser client, this would be useful at the provider level to avoid making superfluous calls to flagd.

e.g. flagd-cache

AlexsJones commented 2 years ago

Wouldn't the caching be in the SDK then rather than flagD?

For example, in the SDK you could do something like...

    // Create a cache with a default expiration time of 5 minutes, and which
    // purges expired items every 10 minutes
    c := cache.New(5*time.Minute, 10*time.Minute)

    // Set the value of the key "foo" to "bar", with the default expiration time
    c.Set("foo", "bar", cache.DefaultExpiration)

    // Set the value of the key "baz" to 42, with no expiration time
    // (the item won't be removed until it is re-set, or removed using
    // c.Delete("baz")
    c.Set("baz", 42, cache.NoExpiration)

    // Get the string associated with the key "foo" from the cache
    foo, found := c.Get("foo")
    if found {
        fmt.Println(foo)
    }

An example from go-cache.

I won't remark much more until it's clear whether this is an SDK proposal change or flagD. If it's in flagD there would be a number of issues around caching lifecycles across sync_providers as well as data going stale on updates ( especially with the informer based update system within k8s )

skyerus commented 2 years ago

As you remarked, a caching system using expiration would result in stale data on configuration changes. I created an issue for the proposal change here rather than the provider/sdk to research whether there's a more robust solution that can bust the cache on configuration changes.

AlexsJones commented 2 years ago

On the other hand it might be trying to solve a problem that doesn't exist.

The latency to flagD is going to be 1-2ms, and the CPU overhead can be throttled with container resource limits.

If the host process is calling flagD often you could argue that those calls might incur some small penalty, but none that would significantly impact performance.

Is there a particular use case you're thinking of here?

james-milligan commented 2 years ago

As todd mentioned previously in this issue 30% of vendors flag evaluations come directly from the browser, these evaluations will commonly be involved in UX / layout configurations that will likely be reevaluated on every page load. I believe its in this usecase that caching will be a powerful tool, especially considereing that we will go from 1 to n number of open connections at a time.

As for cache busting im currently looking into 2 different implementations of flag update subscriptions, one will reevaluate the given flag and return the new value, however the other will be an non specific signal to indicate that there has been a configuration change, this could be used as the indicator to bust the cache as we can be confident that until we see this signal that the evaluation result will always be the same (provided the context being passed matches). At least it should be a little cleaner than a time based implementation.

beeme1mr commented 2 years ago

I would expect caching to be handled within the SDKs. I don't see any reason to add caching to flagD at this time.

skyerus commented 2 years ago

After seeing @james-milligan's work on event subscription I think a more thorough caching mechanism should be revisited once that work is complete as it could solve the cache busting problem. In the meantime it could be worth implementing a naive expiration based cache mechanism in the provider. I'm leaving the results of my research below anyhow for reference.


This redis doc describes the problem eloquently.

flagd flag evaluation operations are pure functions. Any evaluation always produces the same result, at least until the flag configurations are changed. Therefore we can store the result of the flag evaluation in the client and keep using it until the flag configuration is changed. On mutation of the flag configuration flagd can emit an event to notify subscribers to invalidate their cache store (important to avoid stale data).

This would require changes in flagd to notify subscribers on configuration change.

james-milligan commented 1 year ago

Closing this as complete, notifications are now exported to the flagd provider via the event stream which will allow for cache busting in a client side caching implementation https://github.com/open-feature/flagd/pull/187/files