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.

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

perf: add lru cache #84

Merged
merged 13 commits into from
Jan 26, 2024
Merged

perf: add lru cache #84

merged 13 commits into from
Jan 26, 2024

Conversation

matthewelwell
Copy link
Contributor

This PR adds LRU caching to the edge proxy which allows users to configure caching on the flags and identities endpoints.

When the edge proxy receives an updated environment document it clears the cache to ensure that updated values are received.

New settings:

{
  ...
  "endpoint_caches": {
    "identities": {
      "use_cache": true | false,
      "cache_max_size": 128
    },
    "flags": {
      "use_cache": true | false,
      "cache_max_size": 128
    }
  }
}

Current concerns: by clearing the cache when we receive an updated environment, we are risking a thundering herd issue in a platform with sustained high load.

An alternative approach (one which can be seen in the first commit of this PR) is to use a ttl_cache instead of one which is completely evicted when an environment update occurs.

I tend to think that the latest approach is better and there are unlikely to be scenarios where the platform is under such significant load that there will be a thundering herd issue, but it's definitely something that should be considered.

In terms of the implementation, it definitely needs some tweaks and some test coverage. I am most concerned about the tight coupling, but I wasn't able to find an easy solution. I will come back to it in the morning...

Finally, I have tested this manually and, for a project that has 300 features, I ran a load test (albeit one that requests the flags for the same identity over and over again) and got an improvement from 137rps to 2600rps for a single edge proxy docker container.

@matthewelwell matthewelwell marked this pull request as ready for review January 25, 2024 17:46
@matthewelwell matthewelwell requested review from a team and gagantrivedi and removed request for a team January 25, 2024 17:46
Copy link
Member

@gagantrivedi gagantrivedi left a comment

Choose a reason for hiding this comment

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

Approve with one minor comment (and lint fixes)

@matthewelwell matthewelwell merged commit 990b6f4 into main Jan 26, 2024
1 check passed
@matthewelwell matthewelwell deleted the perf/add-lru-cache branch January 26, 2024 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants