Skip to content

Reuse checksum on streaming retries + refactor ChecksumInterceptor#3500

Merged
sbaluja merged 15 commits intomainfrom
ReuseStreamingChecksum
Aug 6, 2025
Merged

Reuse checksum on streaming retries + refactor ChecksumInterceptor#3500
sbaluja merged 15 commits intomainfrom
ReuseStreamingChecksum

Conversation

@sbaluja
Copy link
Contributor

@sbaluja sbaluja commented Jul 31, 2025

Issue #, if available:

Description of changes:

  • On streaming operations, if the request payload was fully consumed and request checksum calculation was complete we will save the request checksum algorithm and value in memory to be reused for retry.
  • Refactored the ChecksumInterceptor logic for setting checksum headers/values to be split across separate functions to follow single responsibility and allow for easier addition of checksum algorithms in the future.

Check all that applies:

  • Did a review by yourself.
  • Added proper tests to cover this PR. (If tests are not applicable, explain.)
    • Tests already exist to validate these changes.
  • Checked if this PR is a breaking (APIs have been changed) change.
  • Checked if this PR will not introduce cross-platform inconsistent behavior.
  • Checked if this PR would require a ReadMe/Wiki update.

Check which platforms you have built SDK on to verify the correctness of this PR.

  • Linux
  • Windows
  • Android
  • MacOS
  • IOS
  • Other Platforms

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@sbaluja sbaluja force-pushed the ReuseStreamingChecksum branch from 291e457 to f2dbd60 Compare July 31, 2025 16:59
typedef std::function<void(const Aws::Http::HttpRequest&)> RequestSignedHandler;

struct RetryContext {
std::pair<Aws::String, std::shared_ptr<Aws::Utils::Crypto::Hash>> m_requestHash;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will initialize to
("", nullptr),
which is why you do
if (request.m_retryContext.m_requestHash.second == nullptr) {
later on to check for "absence".
would be better to have this as
std::shared_ptr<std::pair<Aws::String, std::shared_ptr<Aws::Utils::Crypto::Hash>>> m_requestHash
or
Aws::Crt::Optional<std::pair<Aws::String, std::shared_ptr<Aws::Utils::Crypto::Hash>>> m_requestHash

to better call out "absent" or "present"

Copy link
Contributor Author

@sbaluja sbaluja Jul 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough, was keeping it consistent with how we check for absence on the httpRequest itself - httpRequest->GetRequestHash().second == nullptr, but this sounds like a good change, will implement

@sbaluja sbaluja merged commit faa33a6 into main Aug 6, 2025
5 of 6 checks passed
@sbaluja sbaluja deleted the ReuseStreamingChecksum branch October 27, 2025 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants