feat: Improve KV data caching and deserialization #903
Conversation
abarker-launchdarkly
requested review from
kinyoklion and
ldhenry
and removed request for
a team
|
@launchdarkly/browser size report |
|
@launchdarkly/js-sdk-common size report |
|
@launchdarkly/js-client-sdk-common size report |
|
@launchdarkly/js-client-sdk size report |
| } | ||
|
|
||
| init(allData: LDFeatureStoreDataStorage, callback: () => void): void { | ||
| this._getKVData(); |
There was a problem hiding this comment.
For reviewers: I'm not entirely sure if this is necessary but wanted to play it safe.
There was a problem hiding this comment.
I think this is probably good.
There was a problem hiding this comment.
@abarker-launchdarkly, I ran this branch locally and I saw these logs when evaluating two flags:
Started EventProcessor.
LD Client initialized
Requesting example-flag from LD-Env-local.flags
No cached data found, loading from KV store
Deserializing KV store data
Successfully cached deserialized data
example-flag evaluation: 1.000250ms
Requesting example-flag from LD-Env-local.flags
Using cached deserialized data
example-flag evaluation 2: 0.212166ms
I'm not sure how much it matters, but I was expecting the first flag eval to use cached date since you are calling this._getKVData() in init. FWIW I am calling await ldClient.waitForInitialization(); before evaluating the flag. Is this expected behavior?
Great to see the caching is working for the second flag eval though!
There was a problem hiding this comment.
@ldhenry From what I can tell, nothing actually is calling either EdgeFeatureStore.init() or EdgeFeatureStore.initialized() during client initialization so caching only happens on first flag eval.
In your branch, you were calling EdgeFeatureStore.initialized() in your overridden LDClient.waitForInitialization() which had the side effect of pre-caching the data. @kinyoklion is this reason enough to attempt to override LDClient.waitForInitialization()? If so, we'll need to discuss some of those issues I brought up in the Slack thread.
There was a problem hiding this comment.
Oh, that makes sense. Thanks for the clarification.
There was a problem hiding this comment.
I would follow what we are doing with daemon mode, which I think it immediately initialized. As waitForInitialization would generally apply to cases where the evaluation result would be different if you waited versus not waiting. We are initialized from that perspective, but we haven't cached, which is the same as daemon mode.
Though it may be easier to explain to customers.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
This looks good to me but I'll defer to @kinyoklion for final approval.
abarker-launchdarkly
deleted the
abarker/sdk-1404/improve-kv-caching-deserialization
branch
🤖 I have created a release *beep* *boop* --- <details><summary>fastly-server-sdk: 0.2.0</summary> ## [0.2.0](fastly-server-sdk-v0.1.9...fastly-server-sdk-v0.2.0) (2025-08-01) ### Features * Improve KV data caching and deserialization ([#903](#903)) ([ae47860](ae47860)) </details> <details><summary>server-sdk-ai: 0.11.0</summary> ## [0.11.0](server-sdk-ai-v0.10.1...server-sdk-ai-v0.11.0) (2025-08-01) ### Features * Adding agent support for AI Configs ([#893](#893)) ([bf95b92](bf95b92)) * Update AI tracker to include model & provider name for metrics generation ([#901](#901)) ([9474862](9474862)) ### Bug Fixes * Remove deprecated track generation event ([#902](#902)) ([40f8593](40f8593)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
3 participants
Requirements
Describe the solution you've provided
Improve KV data caching and deserialization