Skip to content

feat(storage): experimental options to tune stall timeouts#9593

Merged
coryan merged 2 commits intogoogleapis:mainfrom
coryan:feat-storage-minimum-progress-control
Jul 29, 2022
Merged

feat(storage): experimental options to tune stall timeouts#9593
coryan merged 2 commits intogoogleapis:mainfrom
coryan:feat-storage-minimum-progress-control

Conversation

@coryan
Copy link
Copy Markdown
Contributor

@coryan coryan commented Jul 29, 2022 •
edited
Loading

Some applications need more progress than just "at least 1 byte per
second for K seconds". These new options allow them to require "at
least N bytes per second for K seconds".

This change is Reviewable

@product-auto-label product-auto-label Bot added the api: storage Issues related to the Cloud Storage API. label Jul 29, 2022
@google-cloud-cpp-bot
Copy link
Copy Markdown
Contributor

google-cloud-cpp-bot commented Jul 29, 2022

Google Cloud Build Logs
For commit: d0f11af72ee9b003e733e7df9eeb2dd983c6ad45

ℹ️ NOTE: Kokoro logs are linked from "Details" below.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 29, 2022 •
edited
Loading

Codecov Report

Merging #9593 (f5f371f) into main (3c0781d) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #9593      +/-   ##
==========================================
- Coverage   94.35%   94.34%   -0.01%     
==========================================
  Files        1491     1491              
  Lines      137714   137750      +36     
==========================================
+ Hits       129935   129967      +32     
- Misses       7779     7783       +4
Impacted Files Coverage Δ
google/cloud/internal/curl_impl.h 100.00% <ø> (ø)
...gle/cloud/storage/internal/curl_download_request.h 100.00% <ø> (ø)
google/cloud/storage/internal/curl_request.h 100.00% <ø> (ø)
...ogle/cloud/storage/internal/curl_request_builder.h 53.57% <ø> (ø)
google/cloud/internal/curl_impl.cc 89.73% <100.00%> (-0.14%) ⬇️
google/cloud/storage/client_options.cc 88.26% <100.00%> (+0.40%) ⬆️
google/cloud/storage/client_options_test.cc 100.00% <100.00%> (ø)
...le/cloud/storage/internal/curl_download_request.cc 87.62% <100.00%> (-0.97%) ⬇️
google/cloud/storage/internal/curl_request.cc 100.00% <100.00%> (ø)
...gle/cloud/storage/internal/curl_request_builder.cc 92.07% <100.00%> (+0.50%) ⬆️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3c0781d...f5f371f. Read the comment docs.

@coryan coryan marked this pull request as ready for review July 29, 2022 19:15
@coryan coryan requested a review from a team July 29, 2022 19:15
devbww
devbww approved these changes Jul 29, 2022
View reviewed changes
Comment thread google/cloud/internal/rest_options.h Outdated
* then the transfer is aborted.
*/
struct TransferStallMinimumRateOption {
using Type = std::uint32_t;
Copy link
Copy Markdown
Contributor

@devbww devbww Jul 29, 2022

Choose a reason for hiding this comment

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

#include <cstdint>

[Aside: Do we support L32 platforms? FWIW, uint32_t -> long would then be implementation-defined on large values.]

Copy link
Copy Markdown
Contributor Author

@coryan coryan Jul 29, 2022

Choose a reason for hiding this comment

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

Fixed.

Yes, we support Windows in 64-bit mode, which is LLP64 (long is 32-bits), and Windows in 32-bit mode. I forgot about the sign bit, fixed.

coryan added 2 commits July 29, 2022 20:38
@coryan
feat(storage): experimental option to tune stall timeouts
1c3cf3d 
Some applications need more progress than just "at least 1 byte per
second for K seconds".  These new options allow them to require "at
least N bytes per second for K seconds".
@coryan
Address review comments
f5f371f
@coryan coryan force-pushed the feat-storage-minimum-progress-control branch from d0f11af to f5f371f Compare July 29, 2022 20:38
@google-cloud-cpp-bot
Copy link
Copy Markdown
Contributor

google-cloud-cpp-bot commented Jul 29, 2022

Google Cloud Build Logs
For commit: f5f371f116643822898c974704eff8e6a1282baa

ℹ️ NOTE: Kokoro logs are linked from "Details" below.

@coryan coryan enabled auto-merge (squash) July 29, 2022 20:52
@coryan coryan merged commit e17ce3c into googleapis:main Jul 29, 2022
@coryan coryan deleted the feat-storage-minimum-progress-control branch July 29, 2022 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@devbww devbww devbww approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

api: storage Issues related to the Cloud Storage API.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@coryan @google-cloud-cpp-bot @devbww