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.

Sign up for GitHub

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

Jump to bottom

feat: Add cacheTtlMs option #760

Merged
merged 14 commits into from
Jan 30, 2025
Merged

feat: Add cacheTtlMs option #760

merged 14 commits into from
Jan 30, 2025

Conversation

keelerm84
Copy link
Member

@keelerm84 keelerm84 commented Jan 24, 2025 •
edited
Loading

When the SDK retrieves data from the EdgeKV, it does so using a single
sub-request per call to variation, variationDetail, or
allFlagsState. The problem is that Akamai imposes a limit of 4
sub-requests per handler event.

If a customer evaluates more than 4 distinct flags during the handling
of a single event, subsequent flag lookups would fail. To combat this,
we now cache the flag values for a specified amount of time.

@keelerm84 keelerm84 requested a review from a team as a code owner January 24, 2025 20:52
@keelerm84
fix: Akamai data is fetched for the lifetime of the SDK connection
0a87198 
When the SDK retrieves data from the EdgeKV, it does so using a single
sub-request per call to `variation`, `variationDetail`, or
`allFlagsState`. The problem is that Akamai imposes a limit of 4
sub-requests per handler event.

If a customer evaluates more than 4 distinct flags during the handling
of a single event, subsequent flag lookups would fail. To combat this,
we now cache the flag values the first time an evaluation is requested,
and we keep that cache in memory for the full duration of the EdgeWorker
execution.
@keelerm84 keelerm84 force-pushed the mk/sdk-1049/akamai-cache branch from 544565d to 0a87198 Compare January 24, 2025 20:52
@github-actions GitHub Actions
Copy link
Contributor

@launchdarkly/js-sdk-common size report
This is the brotli compressed size of the ESM build.
Size: 19014 bytes
Size limit: 21000

@github-actions GitHub Actions
Copy link
Contributor

@launchdarkly/js-client-sdk-common size report
This is the brotli compressed size of the ESM build.
Size: 15354 bytes
Size limit: 20000

@github-actions GitHub Actions
Copy link
Contributor

@launchdarkly/js-client-sdk size report
This is the brotli compressed size of the ESM build.
Size: 19826 bytes
Size limit: 21000

kinyoklion
kinyoklion reviewed Jan 24, 2025
View reviewed changes
packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts Outdated
@@ -2,14 +2,13 @@ import { EdgeProvider } from '.';

/**
* Wraps around an edge provider to cache a copy of the sdk payload locally an explicit request is made to refetch data from the origin.
* The wrapper is neccessary to ensure that we dont make redundant sub-requests from Akamai to fetch an entire environment payload.
* The wrapper is necessary to ensure that we don't make redundant sub-requests from Akamai to fetch an entire environment payload.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should maybe note that, at the time of writing, there is a 4 request sub-limit.

I think we also want to update the docs to indicate using the SDK uses one of their sub-requests. In case they already need to make their own sub requests for other reasons.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think that is a fair point. I still have some loose ends I want to follow up with on this PR, so this isn't getting merged any time soon I don't think.

I'll make sure to get the docs updated and much more explicit about this one we have all this clarified.

@keelerm84 keelerm84 changed the title fix: Akamai data is fetched for the lifetime of the SDK connection feat: Add cacheTtlMs option Jan 27, 2025
@keelerm84 keelerm84 requested a review from kinyoklion January 28, 2025 00:59
kinyoklion
kinyoklion requested changes Jan 28, 2025
View reviewed changes
packages/sdk/svelte/svelte.config.js Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll want to revert this file.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

packages/shared/akamai-edgeworker-sdk/__tests__/featureStore/cacheableStore.test.ts Outdated Show resolved Hide resolved
packages/shared/akamai-edgeworker-sdk/__tests__/featureStore/cacheableStore.test.ts Outdated Show resolved Hide resolved
packages/shared/akamai-edgeworker-sdk/__tests__/featureStore/cacheableStore.test.ts Outdated
expect(mockGet).toHaveBeenCalledTimes(1);

// eslint-disable-next-line no-promise-executor-return
await new Promise((resolve) => setTimeout(resolve, 60));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of depending on time we instead should use fake timers.

Setup

jest.useFakeTimers();

If we want all pending timers to execute, then we use:

await jest.runAllTimersAsync()'

Alternatively you can advance them by a specific amount.

await jest.advanceTimersByTimeAsync(100); // advances by 100ms, things scheduled for less than that should execute.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts Outdated Show resolved Hide resolved
packages/shared/akamai-edgeworker-sdk/src/featureStore/cacheableStoreProvider.ts Outdated Show resolved Hide resolved
@keelerm84 keelerm84 requested a review from kinyoklion January 28, 2025 18:23
@kinyoklion kinyoklion self-requested a review January 29, 2025 23:13
@keelerm84 keelerm84 merged commit 4f961dd into main Jan 30, 2025
22 checks passed
@keelerm84 keelerm84 deleted the mk/sdk-1049/akamai-cache branch January 30, 2025 02:23
@github-actions github-actions bot mentioned this pull request Jan 30, 2025
Merged
keelerm84 pushed a commit that referenced this pull request Jan 30, 2025
@github-actions
chore: release main (#769)
24d3bba 
🤖 I have created a release *beep* *boop*
---


<details><summary>akamai-edgeworker-sdk-common: 1.4.0</summary>

##
[1.4.0](akamai-edgeworker-sdk-common-v1.3.3...akamai-edgeworker-sdk-common-v1.4.0)
(2025-01-30)


### Features

* Add cacheTtlMs option
([#760](#760))
([4f961dd](4f961dd))
</details>

<details><summary>akamai-server-base-sdk: 2.1.22</summary>

##
[2.1.22](akamai-server-base-sdk-v2.1.21...akamai-server-base-sdk-v2.1.22)
(2025-01-30)


### Dependencies

* The following workspace dependencies were updated
  * dependencies
* @launchdarkly/akamai-edgeworker-sdk-common bumped from ^1.3.3 to
^1.4.0
</details>

<details><summary>akamai-server-edgekv-sdk: 1.4.0</summary>

##
[1.4.0](akamai-server-edgekv-sdk-v1.3.1...akamai-server-edgekv-sdk-v1.4.0)
(2025-01-30)


### Features

* Add cacheTtlMs option
([#760](#760))
([4f961dd](4f961dd))


### Dependencies

* The following workspace dependencies were updated
  * dependencies
* @launchdarkly/akamai-edgeworker-sdk-common bumped from ^1.3.3 to
^1.4.0
</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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kinyoklion kinyoklion kinyoklion approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@keelerm84 @kinyoklion