Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a debug log entry for retries #1352

Merged
merged 3 commits into from
May 2, 2022
Merged

Add a debug log entry for retries #1352

merged 3 commits into from
May 2, 2022

Conversation

jdisanti
Copy link
Collaborator

@jdisanti jdisanti commented Apr 29, 2022

Motivation and Context

This PR logs a debug event when a retry is going to be performed so that it's easier to tell retry is working by looking at logs (prompted by awslabs/aws-sdk-rust#524).

Checklist

  • I have updated CHANGELOG.next.toml if I made changes to the smithy-rs codegen or runtime crates
  • 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.

@jdisanti jdisanti requested a review from a team as a code owner April 29, 2022 17:47
@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

Rust Wrk benchmark report:

Duration: 90 sec, Connections: 32, Threads: 2

Measurement Deviation Current Old
Requests/sec -8.05% 60290.99 65570.48
Total requests -8.05% 5430219 5905413
Total errors NaN% 0 0
Total successes -8.05% 5430219 5905413
Average latency ms -11.59% 0.61 0.69
Minimum latency ms 0.00% 0.02 0.02
Maximum latency ms 16.40% 21.22 18.23
Stdev latency ms -28.16% 0.74 1.03
Transfer Mb -8.05% 564.47 613.87
Connect errors NaN% 0 0
Read errors NaN% 0 0
Write errors NaN% 0 0
Status errors (not 2xx/3xx) NaN% 0 0
Timeout errors NaN% 0 0

Copy link
Collaborator

@rcoh rcoh left a comment

Choose a reason for hiding this comment

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

LGTM, but seems like we might as well add a little more info?

@@ -313,6 +313,8 @@ impl RetryHandler {
return None;
}
};

tracing::debug!("retrying after {:?}", dur);
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we include more info here? retry kind, number of retries, etc.?

@jdisanti jdisanti enabled auto-merge (squash) May 2, 2022 17:45
@github-actions
Copy link

github-actions bot commented May 2, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

Rust Wrk benchmark report:

Duration: 90 sec, Connections: 32, Threads: 2

Measurement Deviation Current Old
Requests/sec 16.70% 72005.71 61698.93
Total requests 16.72% 6482332 5553748
Total errors NaN% 0 0
Total successes 16.72% 6482332 5553748
Average latency ms -3.51% 0.55 0.57
Minimum latency ms 0.00% 0.02 0.02
Maximum latency ms -1.33% 17.04 17.27
Stdev latency ms 36.67% 0.82 0.6
Transfer Mb 16.72% 673.84 577.31
Connect errors NaN% 0 0
Read errors NaN% 0 0
Write errors NaN% 0 0
Status errors (not 2xx/3xx) NaN% 0 0
Timeout errors NaN% 0 0

@jdisanti jdisanti merged commit 3aab827 into main May 2, 2022
@jdisanti jdisanti deleted the jdisanti-retry-log branch May 2, 2022 18:02
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