Skip to content
This repository has been archived by the owner on May 30, 2024. It is now read-only.

Fail to read from feature store cache prior to initialization #107

Closed
mmadson opened this issue Nov 29, 2017 · 2 comments
Closed

Fail to read from feature store cache prior to initialization #107

mmadson opened this issue Nov 29, 2017 · 2 comments

Comments

@mmadson
Copy link

mmadson commented Nov 29, 2017

When a connection issue to launch darkly's servers is simulated for a new uninitialized LDClient and a persistent feature store (redis) is supplied, the expectation is that the feature store cache will be used to evaluate the flags, however in my tests it appears that prior to initialization the feature store cache is not consulted and the hardcoded default value is returned instead. This greatly reduces the effectiveness of a persistent feature store in the event that launch darkly's servers are down for an extended period of time. In such a scenario we would not be able to restart our servers because doing so would put our LDClients in an uninitialized state and our persistent cache would be useless. This also seems to contradict the documentation:

https://support.launchdarkly.com/hc/en-us/articles/115002374068-Do-you-support-Redis-caching-

which states:

Our SDKs can be configured to use Redis as a persistent store, so even if an initial connection to LaunchDarkly fails, the last flag configurations will be used instead of the fallbacks. 

I believe the source of the problem is in this bit of code from LDClient.class:236:

            if (!this.initialized()) {
                logger.warn("Evaluation called before Client has been initialized for feature flag " + featureKey + "; returning default value");
                this.sendFlagRequestEvent(featureKey, user, defaultValue, defaultValue, (Integer)null);
                return defaultValue;
            } else {

As you can see, if an initial connection is not established; the featureStore is never consulted.

@brooswit
Copy link

Hi, thanks for pointing this out. We discussed it and we agree with you. We have added this to our backlog and we will let you know when we have a solution.

@pkaeding
Copy link
Contributor

pkaeding commented Jan 9, 2018

Hi @mmadson! I apologize for not updating this issue sooner. This should be fixed as of the 2.4.0 release.

Please let me know if you run into any other issues.

@pkaeding pkaeding closed this as completed Jan 9, 2018
eli-darkly added a commit that referenced this issue Nov 22, 2018
fix build script to ensure all of our packages are correctly included
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants