fix(common): revamp the async polling loop #7762
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Do not drop a cancellation request made before we receive the `longrunning::Operation`. Instead, remember that a cancellation request was made, and then cancel the operation once we know it has started. This (probabilistically) saves a significant chunk of time in the `spanner_cancel_backup_create` sample, as we now always cancel the `CreateBackup()`. * Do not fabricate a cancelled `Status`. The only way we really know an operation has been cancelled is by examining the `GetOperation()` response.
|
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
Codecov Report
@@ Coverage Diff @@
## main #7762 +/- ##
==========================================
- Coverage 95.15% 95.15% -0.01%
==========================================
Files 1271 1271
Lines 114603 114633 +30
==========================================
+ Hits 109054 109082 +28
- Misses 5549 5551 +2
Continue to review full report at Codecov.
|
coryan
approved these changes
Dec 19, 2021
devbww
added a commit
to devbww/google-cloud-cpp
that referenced
this pull request
Apr 6, 2022
Do not fabricate a deadline-exceeded `Status` when the polling policy says enough is enough. That left the operation in an unknowable state. Rather, cancel the operation and wait for the next poll to yield us an accurate status (which may end up being OK). That is, a polling limit is really a request for auto-cancellation. (See googleapis#7762 for the part-un work on ensuring an explicit cancel yielded an accurate status.) In the case of the Spanner backup tests, for example, this means we can do something meaningful after hitting the poll limit of a `CreateBackup()` call, as we'll know that the backup is no longer pending. Update the poll-loop mocking tests to account for the new sequence of RPCs.
devbww
added a commit
to devbww/google-cloud-cpp
that referenced
this pull request
Apr 14, 2022
Do not fabricate a deadline-exceeded `Status` when the polling policy says enough is enough. That left the operation in an unknowable state. Rather, cancel the operation and wait for the next poll to yield us an accurate status (which may end up being OK). That is, a polling limit is really a request for auto-cancellation. (See googleapis#7762 for the part-un work on ensuring an explicit cancel yielded an accurate status.) In the case of the Spanner backup tests, for example, this means we can do something meaningful after hitting the poll limit of a `CreateBackup()` call, as we'll know that the backup is no longer pending. Update the poll-loop mocking tests to account for the new sequence of RPCs.
devbww
added a commit
to devbww/google-cloud-cpp
that referenced
this pull request
Apr 14, 2022
Do not fabricate a deadline-exceeded `Status` when the polling policy says enough is enough. That left the operation in an unknowable state. Rather, cancel the operation and wait for the next poll to yield us an accurate status (which may end up being OK). That is, a polling limit is really a request for auto-cancellation. (See googleapis#7762 for the part-un work on ensuring an explicit cancel yielded an accurate status.) In the case of the Spanner backup tests, for example, this means we can do something meaningful after hitting the poll limit of a `CreateBackup()` call, as we'll know that the backup is no longer pending. Update the poll-loop mocking tests to account for the new sequence of RPCs.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Do not drop a cancellation request made before we receive
the
longrunning::Operation.Instead, remember that a cancellation request was made,
and then cancel the operation once we know it has started.
This (probabilistically) saves a significant chunk of time
in the
spanner_cancel_backup_createsample, as we nowalways cancel the
CreateBackup().Do not fabricate a cancelled
Status.The only way we really know an operation has been cancelled
is by examining the
GetOperation()response.This change is