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

Consider using retry policy for OK streams with failing mutations in bigtable::BulkApply() #7479

Closed
dbolduc opened this issue Oct 16, 2021 · 1 comment · Fixed by #9706
Closed
Assignees
Labels
api: bigtable Issues related to the Bigtable API. type: cleanup An internal cleanup or hygiene concern.

Comments

@dbolduc
Copy link
Member

dbolduc commented Oct 16, 2021

The current functionality is described here:

* @note The retry policy is only impacted by the result of the gRPC stream.
* Let's say you have a `LimitedErrorCountRetryPolicy` of 2. If an
* idempotent mutation fails with a retryable error and the stream itself
* succeeds, it may be retried more than 2 times. Only when the stream
* fails twice will we give up and consider the mutation to be failed.

But I think that a LimitedErrorCountRetryPolicy of 2 should mean we only ever send 2 RPCs, regardless of whether the errors are from the overall stream or from individual mutations. For example, this test currently passes, but I do not think it should.

There is some ambiguity in the case where a mutation fails and the stream fails with different status codes. I think in this case, we should continue to use the stream's status code for the retry policy. There is also ambiguity if two mutations fail with different transient errors, but the stream succeeds. Which error is passed to the retry policy?

@dbolduc dbolduc added api: bigtable Issues related to the Bigtable API. type: cleanup An internal cleanup or hygiene concern. labels Oct 16, 2021
@dbolduc dbolduc changed the title Bigtable BulkApply() individual mutations should be subject to the retry policy Bigtable BulkApply() failing mutations on OK streams should be subject to the retry policy Oct 16, 2021
@dbolduc dbolduc changed the title Bigtable BulkApply() failing mutations on OK streams should be subject to the retry policy Consider using retry policy for OK streams with failing mutations in bigtable::BulkApply() Oct 16, 2021
@dbolduc
Copy link
Member Author

dbolduc commented Aug 10, 2022

Bigtable team says this is a bug. We should backoff in between stream attempts, and count OK streams with failing mutations towards the retry policy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigtable Issues related to the Bigtable API. type: cleanup An internal cleanup or hygiene concern.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant