cache sigv4 compute#1652
Conversation
ashishdhingra
left a comment
There was a problem hiding this comment.
Looks good comparing it with existing SigV4aSignatureCalculator.
| override fun signingKey(config: AwsSigningConfig): ByteArray = runBlocking { | ||
| val date = config.signingDate.format(TimestampFormat.ISO_8601_CONDENSED_DATE) | ||
| val cacheKey = SigningKeyCacheKey(config.credentials.secretAccessKey, config.region, config.service, date) | ||
|
|
||
| signingKeyCache.get(cacheKey) { | ||
| ExpiringValue(deriveKey(it.secretKey, it.date, it.region, it.service), Clock.System.now() + 24.hours) | ||
| } | ||
| } |
There was a problem hiding this comment.
Correctness: We should not block on a coroutine in a non-suspend method. How we address this depends on how critical the cache is to the final result.
Can we withstand cache misses? If so, Mutex.tryLock is a non-suspend method that will acquire the lock if it's not already locked. The slight downside is that if it is already locked, it returns false immediately. That might mean sometimes we bypass the cache if it's busy and recalculate a signature. If we decide to go this route, I'd suggest adding a non-suspend tryGet method to ExpiringKeyedCache/PeriodicSweepCache.
If we cannot withstand cache misses then I think we need a different lock mechanism. kotlinx.atomicfu provides some lock primitives that spin rather than suspend. If we decide to go this route we should create a new interface/implementation of the cache.
There was a problem hiding this comment.
Cache misses is fine so I think Mutex.tryLock is the way to go.
How SigV4a is different in behavior to SigV4? What is the reason of having the block in SigV4a?
| private data class SigningKeyCacheKey( | ||
| val secretKey: String, | ||
| val region: String, | ||
| val service: String, | ||
| val date: String, | ||
| ) |
There was a problem hiding this comment.
Suggestion: This combination of data don't just represent a cache key, they represent the signing key regardless of caching. I suggest renaming it to SigningKey and using it as the argument to deriveKey instead of the four separate arguments. I also think you could add a secondary constructor that takes AwsSigningConfig and extracts the necessary fields (or alternately, an AwsSigningConfig.toSigningKey() extension method.
| assertNotSame(key1, key2) | ||
| assertEquals(false, key1.contentEquals(key2)) |
There was a problem hiding this comment.
Nit: This assertNotSame is a subset of the assertEquals and is thus redundant.
| assertNotSame(key1, key2) | ||
| assertEquals(false, key1.contentEquals(key2)) |
There was a problem hiding this comment.
Nit: Don't use assertEquals with boolean literals. Favor assertTrue and assertFalse.
| private val signingKeyCache = PeriodicSweepCache<SigningKeyCacheKey, ByteArray>( | ||
| minimumSweepPeriod = 24.hours, | ||
| ) |
There was a problem hiding this comment.
Question: A 24 hour sweep period combined with short-term credentials, multiservice usage pattern, and multiple accounts could stack up to quite a lot of entries. Should we be concerned about memory usage?
There was a problem hiding this comment.
You're right, I forgot about short-term credentials. Java SDK is using a FIFO cache with 300 entries. Do you have any suggestion on this?
The short-term credentials can last from few minutes to several hours. Computing SigV4 is cheap, the only problem is when it is in a high-concurrency environment then it becomes expensive, so probably using 1 hour like SigV4a?
…thy-lang/smithy-kotlin into optimization-http-engine-cache-sigv4
Summary
kDate → kRegion → kService → kSigning) usingPeriodicSweepCacheso that requests with the same credentials, region, service, and date skip the 4-HMAC key derivation chain entirelyMotivation
SigV4 signing key derivation performs 4 sequential HMAC-SHA256 operations. For high-concurrency workloads (hundreds of in-flight requests to the same service), these redundant derivations become a measurable CPU cost. Caching the result eliminates this overhead for the common case where the signing parameters are stable across requests.
What changed
SigV4SignatureCalculator.signingKey()now uses aPeriodicSweepCache<SigningKeyCacheKey, ByteArray>to cache derived signing keys. The cache key is a data class of(secretAccessKey, region, service, date). On a cache hit, the previously computed key is returned immediately. Entries expire after 24 hours and are cleaned by periodic sweep.Design alternatives considered
ConcurrentHashMapPeriodicSweepCache(chosen)runBlockingoverheadWe chose
PeriodicSweepCachebecause:The Mutex overhead is acceptable because the critical section is tiny (a HashMap lookup + clock check) — contention is negligible compared to the 4-HMAC chain it replaces.
Benchmark Results
All benchmarks run on m7i.2xlarge instances with the following configuration:
E2E Throughput (S3)
7 base instances, 4 target instances (2 outlier instances excluded from target due to host-level network throttling).
S3 Upload
S3 Download
Throughput observations:
E2E Latency (DynamoDB)
21 instances per variant for statistical significance.
DynamoDB PutItem
DynamoDB GetItem
Latency observations:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.