fix: keep keys cached longer - #6881
Open
Flo4604 wants to merge 3 commits into
Open
Conversation
Amp-Thread-ID: https://ampcode.com/threads/T-019fa925-b448-749a-b474-fa870491726b Co-authored-by: Flo <flo@unkey.com>
Flo4604
marked this pull request as ready for review
July 28, 2026 19:49
Member
|
What about large customers who rely on us? This would reduce the 500x but also reduce the accuracy? Either letting loads of requests through or the opposite blocking a customer who may have been on the edge of a ratelimit window. Or am I too tired to use my brain? |
Member
Author
|
too tired :D this is just for saying is this key valid/yes no |
Member
Author
|
ratelimiting will still be done |
Member
|
Ah yes I am dumb enough now I'm re-reading |
Collaborator
|
I don't want to derail this too much, but wdyt about using pkg.go.dev/golang.org/x/sync/singleflight here? |
Member
Author
|
Yeah sure |
Amp-Thread-ID: https://ampcode.com/threads/T-019fa925-b448-749a-b474-fa870491726b Co-authored-by: Flo <flo@unkey.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019fa925-b448-749a-b474-fa870491726b Co-authored-by: Flo <flo@unkey.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem:
We sit directly in the hot path of our customers and when our cache+database cannot answer a verification lookup, a key that we've already seen and successfully verified can eventually start returning a 5xx errors because our cached copy was more than ten minutes old.
Solution:
Treat previously verified keys as last-known-good serving state during a db incidents instead of discarding them after ten minutes.
Known keys can continue to be verified for up to 24 hours while we fix the db and eventually it will refresh in the background.
When many requests arrive for the same key before it is cached, they now share one database lookup instead of multiplying identical work against an already degraded dependency.
Impact:
Normal behavior remains unchanged.
If a key is not fresh anymore <10s we will try to refresh it if that doesnt work we keep serving it, so it can continue to work trough smaller outtages/temp query failures(reparenting) instead of throwing a 5xx.
Request bursts for the same uncached key no longer amplify into an equivalent burst of database queries.
Testing:
Tests should pass.
Added regression coverage that sends 100 concurrent misses for the same key and verifies they produce one database load and all receive the shared result. Race tests, key-service tests, lint, and the build pass.