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

fix(bigtable): Use retry policy on streams with failing mutations #9706

Conversation

dbolduc
Copy link
Member

@dbolduc dbolduc commented Aug 18, 2022

Fixes #7479

Streams with an OK status, but failing mutations count towards the retry policy.

While the 4 tests don't look like each other, they are consistent with the other tests in their fixtures, namely the TooManyFailures or *Exhausted tests.


This change is Reviewable

@product-auto-label product-auto-label bot added the api: bigtable Issues related to the Bigtable API. label Aug 18, 2022
@google-cloud-cpp-bot
Copy link
Collaborator

Google Cloud Build Logs
For commit: ecd1ca6a1faa56b752fb63137e5caa916aedf786

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

@codecov
Copy link

codecov bot commented Aug 18, 2022

Codecov Report

Merging #9706 (ecd1ca6) into main (1abf599) will increase coverage by 0.00%.
The diff coverage is 96.12%.

@@           Coverage Diff            @@
##             main    #9706    +/-   ##
========================================
  Coverage   93.89%   93.89%            
========================================
  Files        1496     1496            
  Lines      139384   139503   +119     
========================================
+ Hits       130878   130993   +115     
- Misses       8506     8510     +4     
Impacted Files Coverage Δ
google/cloud/bigtable/table.h 98.54% <ø> (ø)
...oud/bigtable/internal/data_connection_impl_test.cc 96.66% <92.59%> (-0.11%) ⬇️
google/cloud/bigtable/table_bulk_apply_test.cc 98.25% <95.45%> (-0.30%) ⬇️
.../bigtable/internal/legacy_async_bulk_apply_test.cc 98.67% <97.05%> (-0.21%) ⬇️
...e/cloud/bigtable/internal/async_bulk_apply_test.cc 97.03% <97.43%> (+0.04%) ⬆️
google/cloud/bigtable/internal/async_bulk_apply.cc 100.00% <100.00%> (ø)
...le/cloud/bigtable/internal/data_connection_impl.cc 98.80% <100.00%> (ø)
...cloud/bigtable/internal/legacy_async_bulk_apply.cc 100.00% <100.00%> (ø)
google/cloud/bigtable/table.cc 99.42% <100.00%> (-0.01%) ⬇️
...e/cloud/pubsublite/internal/alarm_registry_impl.cc 97.05% <0.00%> (-2.95%) ⬇️
... and 5 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@dbolduc dbolduc marked this pull request as ready for review August 19, 2022 03:53
@dbolduc dbolduc requested a review from a team as a code owner August 19, 2022 03:53
@dbolduc dbolduc merged commit a4356c2 into googleapis:main Aug 23, 2022
@dbolduc dbolduc deleted the bigtable-bulk-apply-ok-streams-with-failed-mutations branch August 23, 2022 14:44
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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