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

feat(GCS+gRPC): more efficient WriteObject() stall timeouts #9576

Conversation

coryan
Copy link
Contributor

@coryan coryan commented Jul 27, 2022

This changes the implementation of WriteObject() to use the blocking
API and a watchdog timer to implement stall timeouts. If the timer
expires before the Write() operation completes a background thread
cancels the RPC. If the operation completes, we cancel the timer.

Fixes #9558


This change is Reviewable

This changes the implementation of `WriteObject()` to use the blocking
API and a watchdog timer to implement stall timeouts. If the timer
expires before the `Write()` operation completes a background thread
cancels the RPC.  If the operation completes, we cancel the timer.
@google-cloud-cpp-bot
Copy link
Collaborator

Google Cloud Build Logs
For commit: 71b28ccba8ee4ef1ef7be2c4767a5261d7660c6e

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

@codecov
Copy link

codecov bot commented Jul 27, 2022

Codecov Report

Merging #9576 (9c813e1) into main (9c813e1) will not change coverage.
The diff coverage is n/a.

❗ Current head 9c813e1 differs from pull request most recent head 71b28cc. Consider uploading reports for the commit 71b28cc to get more accurate results

@@           Coverage Diff           @@
##             main    #9576   +/-   ##
=======================================
  Coverage   94.37%   94.37%           
=======================================
  Files        1491     1491           
  Lines      137590   137590           
=======================================
  Hits       129844   129844           
  Misses       7746     7746           

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 9c813e1...71b28cc. Read the comment docs.

@coryan coryan marked this pull request as ready for review July 27, 2022 22:38
@coryan coryan requested a review from a team as a code owner July 27, 2022 22:38
grpc::WriteOptions().set_last_message());
watchdog.cancel();
if (watchdog.get()) {
writer->Close();
Copy link
Member

Choose a reason for hiding this comment

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

do we need another watchdog that might cancel this Close() operation? we use one a few lines below here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. If we get to this point the streaming RPC is already canceled. Setting up another watchdog, just to cancel the request again seems unlikely to speed up things.

@coryan coryan merged commit c58299d into googleapis:main Jul 28, 2022
@coryan coryan deleted the feat-GCS+gRPC-more-efficient-WriteObject-stall-timeout branch July 28, 2022 01:41
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.

Implement stall timeouts using watchdog timers
3 participants