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

update first byte timeout algo #461

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Conversation

TingDaoK
Copy link
Contributor

@TingDaoK TingDaoK commented Nov 15, 2024

Issue #, if available:

  • Some customer saw the error from the first byte timeout being too restrict and lead to fail the retries.

Description of changes:

  • Update the first byte timeout to be more reasonable.

Current main benchmark 1x30GiB file upload with c5n.18xlarge:

main branch
[ERROR] [2024-11-15T23:10:23Z] [00007f54532e3440] [AuthCredentialsProvider] - Failed to resolve role arn during sts web identity provider initialization.
Run:1 Secs:8.023596 Gb/s:32.117522
Run:2 Secs:7.781778 Gb/s:33.115574
Run:3 Secs:7.520261 Gb/s:34.267168
Run:4 Secs:7.507220 Gb/s:34.326692
Run:5 Secs:7.904503 Gb/s:32.601423
Run:6 Secs:7.487668 Gb/s:34.416329
Run:7 Secs:7.583257 Gb/s:33.982500
Run:8 Secs:7.044802 Gb/s:36.579886
Run:9 Secs:6.976853 Gb/s:36.936141
Run:10 Secs:10.211120 Gb/s:25.237000
Overall Throughput (Gb/s) Median:34.124834 Mean:33.358023 Min:25.237000 Max:36.936141 Variance:9.457925 StdDev:3.075374
Overall Duration (Secs) Median:7.551759 Mean:7.804106 Min:6.976853 Max:10.211120 Variance:0.743098 StdDev:0.862031
Peak RSS:4065.613281 MiB

With this change same workload:

new timeout branch
[ERROR] [2024-11-15T23:11:51Z] [00007f2602455440] [AuthCredentialsProvider] - Failed to resolve role arn during sts web identity provider initialization.
Run:1 Secs:7.350504 Gb/s:35.058551
Run:2 Secs:7.112591 Gb/s:36.231247
Run:3 Secs:7.219721 Gb/s:35.693629
Run:4 Secs:8.043577 Gb/s:32.037741
Run:5 Secs:8.452846 Gb/s:30.486542
Run:6 Secs:8.600675 Gb/s:29.962535
Run:7 Secs:6.505653 Gb/s:39.611405
Run:8 Secs:7.331239 Gb/s:35.150678
Run:9 Secs:7.568643 Gb/s:34.048116
[ERROR] [2024-11-15T23:13:01Z] [00007f258ffff640] [S3MetaRequest] - id=0x2484a0f0 Request failed from error 14341 (Response code indicates internal server error). (request=0x7f25a80d61b0, response status=500). Try to setup a retry.
Run:10 Secs:7.183231 Gb/s:35.874947
Overall Throughput (Gb/s) Median:35.104615 Mean:34.415539 Min:29.962535 Max:39.611405 Variance:7.606629 StdDev:2.758012
Overall Duration (Secs) Median:7.340872 Mean:7.536868 Min:6.505653 Max:8.600675 Variance:0.377454 StdDev:0.614373
Peak RSS:4071.847656 MiB

Overall there is no performance downgrade from this change on the benchmark.

Meanwhile, if I just disable the timeout, the same workload result in:

disable timeout branch
Run:1 Secs:10.158413 Gb/s:25.367941
Run:2 Secs:10.453124 Gb/s:24.652730
Run:3 Secs:10.343186 Gb/s:24.914763
Run:4 Secs:8.831093 Gb/s:29.180763
Run:5 Secs:10.161891 Gb/s:25.359261
Run:6 Secs:9.006103 Gb/s:28.613711
Run:7 Secs:8.713228 Gb/s:29.575496
Run:8 Secs:8.070316 Gb/s:31.931592
Run:9 Secs:9.802968 Gb/s:26.287756
Run:10 Secs:9.996880 Gb/s:25.777847
Overall Throughput (Gb/s) Median:26.032801 Mean:27.166186 Min:24.652730 Max:31.931592 Variance:5.525187 StdDev:2.350572
Overall Duration (Secs) Median:9.899924 Mean:9.553720 Min:8.070316 Max:10.453124 Variance:0.615521 StdDev:0.784552
Peak RSS:4064.597656 MiB

there is an around 22% performance downgrade.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.67%. Comparing base (5d8d420) to head (3d7dfd3).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #461      +/-   ##
==========================================
+ Coverage   89.63%   89.67%   +0.04%     
==========================================
  Files          20       20              
  Lines        6137     6162      +25     
==========================================
+ Hits         5501     5526      +25     
  Misses        636      636              
Files with missing lines Coverage Δ
source/s3_client.c 90.31% <100.00%> (+0.22%) ⬆️

... and 2 files with indirect coverage changes

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