Skip to content

Commit

Permalink
Cap the max jitter fraction for cache refresh buffer time to 0.5 (#3402)
Browse files Browse the repository at this point in the history
## 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
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [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 <jdisanti@amazon.com>
  • Loading branch information
ysaito1001 and jdisanti authored Feb 9, 2024
1 parent df1103d commit 166f0e2
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 5 deletions.
12 changes: 12 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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 {
Expand All @@ -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<Duration>) -> &mut Self {
Expand All @@ -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 {
Expand All @@ -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(
Expand Down

0 comments on commit 166f0e2

Please sign in to comment.