Skip to content

[Feature] Add clock skew computation and unit test - #3898

Open
kai-ion wants to merge 1 commit into
mainfrom
timechange
Open

[Feature] Add clock skew computation and unit test#3898
kai-ion wants to merge 1 commit into
mainfrom
timechange

Conversation

@kai-ion

@kai-ion kai-ion commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Issue #, if available:

Description of changes:
Add clock skew computation and unit test

Check all that applies:

  • Did a review by yourself.
  • Added proper tests to cover this PR. (If tests are not applicable, explain.)
  • 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.

@kai-ion kai-ion changed the title Add clock skew computation and unit test [Feature] Add clock skew computation and unit test Aug 24, 2026
@kai-ion
kai-ion marked this pull request as ready for review August 26, 2026 19:43
Comment thread src/aws-cpp-sdk-core/include/aws/core/internal/ClockSkew.h Outdated
Comment thread src/aws-cpp-sdk-core/include/aws/core/internal/ClockSkew.h Outdated
Comment thread src/aws-cpp-sdk-core/include/aws/core/internal/ClockSkew.h Outdated
Comment thread src/aws-cpp-sdk-core/include/aws/core/internal/ClockSkew.h Outdated
Comment thread src/aws-cpp-sdk-core/include/aws/core/internal/ClockSkew.h
Comment thread src/aws-cpp-sdk-core/include/aws/core/internal/ClockSkew.h Outdated
Comment thread tests/aws-cpp-sdk-core-tests/utils/ClockSkewTest.cpp Outdated
@kai-ion
kai-ion force-pushed the timechange branch 2 times, most recently from c299fbb to 6214afe Compare August 28, 2026 14:51
Comment thread src/aws-cpp-sdk-core/include/aws/core/internal/ClockSkew.h Outdated
// Retryable when the applied skew is off from the observed skew by more than the threshold.
if (candidate.has_value())
{
const std::chrono::milliseconds delta = attemptSkew - candidate.value();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ok im sort of split on this, nice to have wont block it, the reason you are doing all of this is because std::chrono::abs is in c++17. i sort of hate we are forced to do this the wrong way. so a suggestion. in aws/core/utils/local you could add Chrono.h and have something like

namespace Aws {
#if defined(__cpp_lib_chrono) && __cpp_lib_chrono >= 201510L
using std::chrono::abs;
using std::chrono::ceil;
using std::chrono::floor;
using std::chrono::round;
#else
// TODO: delete this branch once the minimum standard is C++17.
template <class Rep, class Period>
constexpr std::enable_if_t<std::numeric_limits<Rep>::is_signed,
                           std::chrono::duration<Rep, Period>>
abs(std::chrono::duration<Rep, Period> d) {
    return d >= d.zero() ? d : -d;
}
#endif
}

then here you could just call Aws::chrono::abs, and if you compile with 17 you get the feature, if you compile without you get the shim implementation.

Add clock skew correction computation and unit test
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