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: retry if failure occurs on initial call in MutateRows #123

Merged
merged 2 commits into from Sep 21, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 16 additions & 12 deletions google/cloud/bigtable/table.py
Expand Up @@ -14,11 +14,12 @@

"""User-friendly container for Google Cloud Bigtable Table."""

from grpc import StatusCode

from google.api_core import timeout
from google.api_core.exceptions import RetryError
from google.api_core.exceptions import Aborted
from google.api_core.exceptions import DeadlineExceeded
from google.api_core.exceptions import NotFound
from google.api_core.exceptions import RetryError
from google.api_core.exceptions import ServiceUnavailable
from google.api_core.retry import if_exception_type
from google.api_core.retry import Retry
from google.api_core.gapic_v1.method import wrap_method
Expand Down Expand Up @@ -986,15 +987,12 @@ class _RetryableMutateRowsWorker(object):
are retryable, any subsequent call on this callable will be a no-op.
"""

# pylint: disable=unsubscriptable-object
RETRY_CODES = (
StatusCode.DEADLINE_EXCEEDED.value[0],
StatusCode.ABORTED.value[0],
StatusCode.UNAVAILABLE.value[0],
Aborted.grpc_status_code.value[0],
DeadlineExceeded.grpc_status_code.value[0],
ServiceUnavailable.grpc_status_code.value[0],
)

# pylint: enable=unsubscriptable-object

def __init__(self, client, table_name, rows, app_profile_id=None, timeout=None):
self.client = client
self.table_name = table_name
Expand Down Expand Up @@ -1078,9 +1076,15 @@ def _do_mutate_retryable_rows(self):
client_info=data_client._client_info,
)

responses = data_client._inner_api_calls["mutate_rows"](
mutate_rows_request, retry=None
)
try:
responses = data_client._inner_api_calls["mutate_rows"](
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this already handled in read rows or does it need to be added there as well?

Copy link
Collaborator

Choose a reason for hiding this comment

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

all methods that changed: googleapis/googleapis@a175708

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Solid call out. this may need to be done broadly to manage the initial setup calls.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

both "read_rows" and "sample_rows" return the raw api call right now. So I think this is complete for now, as we do no special handling beyond gapic configs for these.

mutate_rows_request, retry=None
)
except (ServiceUnavailable, DeadlineExceeded, Aborted):
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this PR change the _is_retryable method on this class to us the imported exceptions, too (to DRY that bit out).

Copy link
Contributor

Choose a reason for hiding this comment

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

# If an exception, considered retryable by `RETRY_CODES`, is
# returned from the initial call, consider
# it to be retryable. Wrap as a Bigtable Retryable Error.
raise _BigtableRetryableError

num_responses = 0
num_retryable_responses = 0
Expand Down