From 166f0e29dd58398d403e145ba3bd8c7e1dbba7f1 Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Fri, 9 Feb 2024 13:57:33 -0600 Subject: [PATCH] Cap the max jitter fraction for cache refresh buffer time to 0.5 (#3402) ## Description Via code inspection, we have identified that there is a potential bug in `DEFAULT_BUFFER_TIME_JITTER_FRACTION`. Specifically, if the fraction happens to be set to 1.0, we end up not respecting the buffer time for cache refresh (see diagrams in #2335). This PR will cap the max fraction value to 0.5 to avoid the problem. ## Checklist - [x] I have updated `CHANGELOG.next.toml` if I made changes to the smithy-rs codegen or runtime crates - [x] I have updated `CHANGELOG.next.toml` if I made changes to the AWS SDK, generated SDK code, or SDK runtime crates ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --------- Co-authored-by: John DiSanti --- CHANGELOG.next.toml | 12 ++++++++++++ .../src/client/identity/cache/lazy.rs | 10 +++++----- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index fea7c5821f..86f909f4d2 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -86,3 +86,15 @@ was added. references = ["smithy-rs#3322"] meta = { "breaking" = false, "bug" = true, "tada" = false } author = "Velfi" + +[[smithy-rs]] +message = "Cap the maximum jitter fraction for identity cache refresh buffer time to 0.5. It was previously 1.0, and if the fraction was randomly set to 1.0, it was equivalent to disregarding the buffer time for cache refresh." +references = ["smithy-rs#3402"] +meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "client" } +author = "ysaito1001" + +[[aws-sdk-rust]] +message = "Cap the maximum jitter fraction for credentials cache refresh buffer time to 0.5. It was previously 1.0, and if the fraction was randomly set to 1.0, it was equivalent to disregarding the buffer time for cache refresh." +references = ["smithy-rs#3402"] +meta = { "breaking" = false, "tada" = false, "bug" = true } +author = "ysaito1001" diff --git a/rust-runtime/aws-smithy-runtime/src/client/identity/cache/lazy.rs b/rust-runtime/aws-smithy-runtime/src/client/identity/cache/lazy.rs index 8581d7c12c..2461754cff 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/identity/cache/lazy.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/identity/cache/lazy.rs @@ -24,7 +24,7 @@ use tracing::Instrument; const DEFAULT_LOAD_TIMEOUT: Duration = Duration::from_secs(5); const DEFAULT_EXPIRATION: Duration = Duration::from_secs(15 * 60); const DEFAULT_BUFFER_TIME: Duration = Duration::from_secs(10); -const DEFAULT_BUFFER_TIME_JITTER_FRACTION: fn() -> f64 = fastrand::f64; +const DEFAULT_BUFFER_TIME_JITTER_FRACTION: fn() -> f64 = || fastrand::f64() * 0.5; /// Builder for lazy identity caching. #[derive(Default, Debug)] @@ -86,7 +86,7 @@ impl LazyCacheBuilder { /// For example, if the identity are expiring in 15 minutes, and the buffer time is 10 seconds, /// then any requests made after 14 minutes and 50 seconds will load a new identity. /// - /// Note: random jitter value between [0.0, 1.0] is multiplied to this buffer time. + /// Note: random jitter value between [0.0, 0.5] is multiplied to this buffer time. /// /// Defaults to 10 seconds. pub fn buffer_time(mut self, buffer_time: Duration) -> Self { @@ -99,7 +99,7 @@ impl LazyCacheBuilder { /// For example, if the identity are expiring in 15 minutes, and the buffer time is 10 seconds, /// then any requests made after 14 minutes and 50 seconds will load a new identity. /// - /// Note: random jitter value between [0.0, 1.0] is multiplied to this buffer time. + /// Note: random jitter value between [0.0, 0.5] is multiplied to this buffer time. /// /// Defaults to 10 seconds. pub fn set_buffer_time(&mut self, buffer_time: Option) -> &mut Self { @@ -113,7 +113,7 @@ impl LazyCacheBuilder { /// and buffer time jitter fraction is 0.2, then buffer time is adjusted to 8 seconds. /// Therefore, any requests made after 14 minutes and 52 seconds will load a new identity. /// - /// Defaults to a randomly generated value between 0.0 and 1.0. This setter is for testing only. + /// Defaults to a randomly generated value between 0.0 and 0.5. This setter is for testing only. #[allow(unused)] #[cfg(test)] fn buffer_time_jitter_fraction(mut self, buffer_time_jitter_fraction: fn() -> f64) -> Self { @@ -127,7 +127,7 @@ impl LazyCacheBuilder { /// and buffer time jitter fraction is 0.2, then buffer time is adjusted to 8 seconds. /// Therefore, any requests made after 14 minutes and 52 seconds will load a new identity. /// - /// Defaults to a randomly generated value between 0.0 and 1.0. This setter is for testing only. + /// Defaults to a randomly generated value between 0.0 and 0.5. This setter is for testing only. #[allow(unused)] #[cfg(test)] fn set_buffer_time_jitter_fraction(