Make TransferredBytes be the top of the list in BinLabel

.changelog/1728489433.md

@@ -0,0 +1,14 @@
+---
+applies_to:
+- client
+- aws-sdk-rust
+authors:
+- ysaito1001
+references:
+- smithy-rs#3871
+- aws-sdk-rust#1202
+breaking: false
+new_feature: false
+bug_fix: true
+---
+Fix minimum throughput detection for downloads to avoid incorrectly raising an error while the user is consuming data at a slow but steady pace.

rust-runtime/aws-smithy-runtime/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "aws-smithy-runtime"
-version = "1.7.1"
+version = "1.7.2"
 authors = ["AWS Rust SDK Team <[email protected]>", "Zelda Hessler <[email protected]>"]
 description = "The new smithy runtime crate"
 edition = "2021"

rust-runtime/aws-smithy-runtime/src/client/http/body/minimum_throughput/throughput.rs

@@ -101,21 +101,19 @@ impl From<(u64, Duration)> for Throughput {
 #[derive(Copy, Clone, Debug, Ord, PartialOrd, Eq, PartialEq)]
 enum BinLabel {
     // IMPORTANT: The order of these enums matters since it represents their priority:
-    // Pending > TransferredBytes > NoPolling > Empty
+    // TransferredBytes > Pending > NoPolling > Empty
     //
     /// There is no data in this bin.
     Empty,
 
     /// No polling took place during this bin.
     NoPolling,
 
-    /// This many bytes were transferred during this bin.
-    TransferredBytes,
-
     /// The user/remote was not providing/consuming data fast enough during this bin.
-    ///
-    /// The number is the number of bytes transferred, if this replaced TransferredBytes.
     Pending,
+
+    /// This many bytes were transferred during this bin.
+    TransferredBytes,
 }
 
 /// Represents a bin (or a cell) in a linear grid that represents a small chunk of time.
@@ -139,8 +137,8 @@ impl Bin {
 
     fn merge(&mut self, other: Bin) -> &mut Self {
         // Assign values based on this priority order (highest priority higher up):
-        //   1. Pending
-        //   2. TransferredBytes
+        //   1. TransferredBytes
+        //   2. Pending
         //   3. NoPolling
         //   4. Empty
         self.label = if other.label > self.label {
@@ -410,6 +408,14 @@ mod test {
     use super::*;
     use std::time::Duration;
 
+    #[test]
+    fn test_log_buffer_bin_label_priority() {
+        use BinLabel::*;
+        assert!(Empty < NoPolling);
+        assert!(NoPolling < Pending);
+        assert!(Pending < TransferredBytes);
+    }
+
     #[test]
     fn test_throughput_eq() {
         let t1 = Throughput::new(1, Duration::from_secs(1));
@@ -521,7 +527,7 @@ mod test {
         assert_eq!(ThroughputReport::NoPolling, report);
     }
 
-    // Transferred bytes MUST take priority over pending
+    // Transferred bytes MUST take priority over pending when reporting throughput
     #[test]
     fn mixed_bag_mostly_pending() {
         let start = SystemTime::UNIX_EPOCH;
@@ -571,4 +577,27 @@ mod test {
 
         tl.push_pending(t0);
     }
+
+    #[test]
+    fn test_label_transferred_bytes_should_not_be_overwritten_by_pending() {
+        let start = SystemTime::UNIX_EPOCH;
+        // Each `Bin`'s resolution is 100ms (1s / BIN_COUNT), where `BIN_COUNT` is 10
+        let mut logs = ThroughputLogs::new(Duration::from_secs(1), start);
+
+        // push `TransferredBytes` and then `Pending` in the same first `Bin`
+        logs.push_bytes_transferred(start + Duration::from_millis(10), 10);
+        logs.push_pending(start + Duration::from_millis(20));
+
+        let BinCounts {
+            empty,
+            no_polling,
+            transferred,
+            pending,
+        } = logs.buffer.counts();
+
+        assert_eq!(9, empty);
+        assert_eq!(0, no_polling);
+        assert_eq!(1, transferred); // `transferred` should still be there
+        assert_eq!(0, pending); // while `pending` should cease to exist, failing to overwrite `transferred`
+    }
 }