fix(kms-keyring-node): coalesce concurrent branch key cache misses#1664
Open
yosefbs wants to merge 1 commit into
Open
fix(kms-keyring-node): coalesce concurrent branch key cache misses#1664yosefbs wants to merge 1 commit into
yosefbs wants to merge 1 commit into
Conversation
When many encrypt/decrypt operations run concurrently against a cold cache for the same branch key, each cache miss independently queried the keystore, firing N DynamoDB GetItem and N KMS Decrypt calls instead of one. getBranchKeyMaterials now shares a single in-flight request per cache entry id, evicting it on settle so the cryptographic materials cache keeps ownership of caching and TTL. A rejected request is evicted too, so the next call retries rather than sharing the failure. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.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
1 participant
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.
Issue #, if available: #1663
Description of changes:
If a lot of encrypt/decrypt operations run at once against a cold cache for the same branch key, they all miss the cache together and each one queries the keystore on its own — so I get N DynamoDB
GetItem+ N KMSDecryptcalls instead of one.getBranchKeyMaterialsnow shares a single in-flight request per cache entry id: the first caller on a miss starts the fetch and stores the promise, everyone else for the same key awaits it. The entry is dropped once it settles, so the materials cache still owns caching and TTL, and a failed request isn't shared — the next call just retries.This covers encrypt and decrypt, since both go through the same helper. Nothing changes about cache eviction, TTL, or the cache entry id.
I also added a test that fires 3000 concurrent lookups for the same key and checks the keystore is hit once — it fails without the fix (
expected 3000 to equal 1) and passes with it.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Check any applicable: