Skip to content

Commit 3bbcbdf

Browse files
feat(bigtable): Rerouted MutateRows to use the data client (#18195)
Migrating over @gkevinzheng PR from bigtable monorepo googleapis/python-bigtable#1290 ### Original description: > **Changes Made:** > > - Rerouted MutateRows to use the data client's `Table.bulk_mutate_rows` function. > - Reworked error handling in `Table.mutate_rows`. Since we must return a list of `Status` objects, we process the list of errors obtained from the `_MutateRowsOperation` back into a corresponding list of error statuses. > - Reworked retries in `Table.mutate_rows` as follows: > - `retry.deadline` -> `operation_timeout` > - `timeout`/`mutation_timeout` -> `attempt_timeout` > - A null retry deadline now defaults to the table default mutate rows operation timeout instead of being defined as no timeout. The docstring has been updated to reflect that. A retry deadline of 0.0 will still be a no-retry option, and is implemented by setting the list of retriable errors to the empty list, and the operation timeout to the default mutate rows operation timeout. > - A `timestamp` of `None` in `set_cell`, which is the default, will now generate a client-side timestamp representing the current time, rather than defaulting to using the server-side timestamp. The docstrings for this have been updated to reflect this. > Note to reviewers: This PR has already been reviewed and merged to a staging branch, with the intention of doing a single merge to main. We are now planning to slowly rollout these changes back to the main branch. Minimal re-review should be necessary --------- Co-authored-by: Kevin Zheng <147537668+gkevinzheng@users.noreply.github.com>
1 parent 400f465 commit 3bbcbdf

3 files changed

Lines changed: 340 additions & 914 deletions

File tree

packages/google-cloud-bigtable/google/cloud/bigtable/table.py

Lines changed: 94 additions & 208 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,18 @@
1717
import warnings
1818
from typing import Set
1919

20-
from google.api_core import timeout
2120
from google.api_core.exceptions import (
2221
Aborted,
2322
DeadlineExceeded,
23+
GoogleAPICallError,
2424
InternalServerError,
2525
NotFound,
26-
RetryError,
2726
ServiceUnavailable,
2827
)
2928
from google.api_core.gapic_v1.method import DEFAULT
3029
from google.api_core.retry import Retry, if_exception_type
3130
from google.cloud._helpers import _to_bytes # type: ignore
31+
from google.rpc import code_pb2, status_pb2
3232

3333
from google.cloud.bigtable import enums
3434
from google.cloud.bigtable.backup import Backup
@@ -38,13 +38,18 @@
3838
MutationsBatcher,
3939
)
4040
from google.cloud.bigtable.column_family import ColumnFamily, _gc_rule_from_pb
41+
from google.cloud.bigtable.data._helpers import TABLE_DEFAULT
42+
from google.cloud.bigtable.data.exceptions import (
43+
MutationsExceptionGroup,
44+
RetryExceptionGroup,
45+
)
46+
from google.cloud.bigtable.data.mutations import RowMutationEntry
4147
from google.cloud.bigtable.encryption_info import EncryptionInfo
4248
from google.cloud.bigtable.policy import Policy
4349
from google.cloud.bigtable.row import AppendRow, ConditionalRow, DirectRow
4450
from google.cloud.bigtable.row_data import (
4551
DEFAULT_RETRY_READ_ROWS,
4652
PartialRowsData,
47-
_retriable_internal_server_error,
4853
)
4954
from google.cloud.bigtable.row_set import RowRange, RowSet
5055
from google.cloud.bigtable_admin_v2 import BaseBigtableTableAdminClient
@@ -563,7 +568,7 @@ def read_row(self, row_key, filter_=None, retry=DEFAULT_RETRY_READ_ROWS):
563568
default value :attr:`DEFAULT_RETRY_READ_ROWS` can be used and
564569
modified with the :meth:`~google.api_core.retry.Retry.with_delay`
565570
method or the :meth:`~google.api_core.retry.Retry.with_deadline`
566-
method.
571+
method. Custom on_error and predicate values will be ignored.
567572
568573
:rtype: :class:`.PartialRowData`, :data:`NoneType <types.NoneType>`
569574
:returns: The contents of the row if any chunks were returned in
@@ -634,7 +639,7 @@ def read_rows(
634639
default value :attr:`DEFAULT_RETRY_READ_ROWS` can be used and
635640
modified with the :meth:`~google.api_core.retry.Retry.with_delay`
636641
method or the :meth:`~google.api_core.retry.Retry.with_deadline`
637-
method.
642+
method. Custom on_error and predicate values will be ignored.
638643
639644
:rtype: :class:`.PartialRowsData`
640645
:returns: A :class:`.PartialRowsData` a generator for consuming
@@ -715,6 +720,9 @@ def mutate_rows(self, rows, retry=DEFAULT_RETRY, timeout=DEFAULT):
715720
specify a ``retry`` strategy of "do-nothing", a deadline of ``0.0``
716721
can be specified.
717722
723+
If a deadline of ``None`` is specified, the deadline defaults to
724+
a table-default of 600 seconds (10 minutes).
725+
718726
:type rows: list
719727
:param rows: List or other iterable of :class:`.DirectRow` instances.
720728
@@ -723,7 +731,8 @@ def mutate_rows(self, rows, retry=DEFAULT_RETRY, timeout=DEFAULT):
723731
(Optional) Retry delay and deadline arguments. To override, the
724732
default value :attr:`DEFAULT_RETRY` can be used and modified with
725733
the :meth:`~google.api_core.retry.Retry.with_delay` method or the
726-
:meth:`~google.api_core.retry.Retry.with_deadline` method.
734+
:meth:`~google.api_core.retry.Retry.with_deadline` method. Custom
735+
on_error and predicate values will be ignored.
727736
728737
:type timeout: float
729738
:param timeout: number of seconds bounding retries for the call
@@ -732,18 +741,90 @@ def mutate_rows(self, rows, retry=DEFAULT_RETRY, timeout=DEFAULT):
732741
:returns: A list of response statuses (`google.rpc.status_pb2.Status`)
733742
corresponding to success or failure of each row mutation
734743
sent. These will be in the same order as the `rows`.
744+
745+
:raise: ValueError: If a row entry has no mutations, or too many mutations
735746
"""
736747
if timeout is DEFAULT:
737748
timeout = self.mutation_timeout
738749

739-
retryable_mutate_rows = _RetryableMutateRowsWorker(
740-
self._instance._client,
741-
self.name,
742-
rows,
743-
app_profile_id=self._app_profile_id,
744-
timeout=timeout,
750+
retryable_errors = RETRYABLE_MUTATION_ERRORS
751+
752+
# The data client cannot take in zero or null values for deadline, so we set it to
753+
# the default if that is the case.
754+
if retry is None:
755+
operation_timeout = TABLE_DEFAULT.MUTATE_ROWS
756+
retryable_errors = []
757+
elif retry.deadline is None:
758+
operation_timeout = TABLE_DEFAULT.MUTATE_ROWS
759+
760+
# To adhere to the retry strategy of do-nothing being achievable with a deadline
761+
# of 0.0, we modify the retryable errors to be empty if such a deadline is passed.
762+
elif retry.deadline == 0:
763+
operation_timeout = TABLE_DEFAULT.MUTATE_ROWS
764+
retryable_errors = []
765+
else:
766+
operation_timeout = retry.deadline
767+
768+
attempt_timeout = timeout
769+
mutation_entries = []
770+
for row in rows:
771+
if not isinstance(row, DirectRow):
772+
raise TypeError(
773+
"Bulk processing can not be applied for conditional or append mutations."
774+
)
775+
if row.table is not None and row.table.name != self.name:
776+
raise TableMismatchError(
777+
"Row %s is a part of %s table. Current table: %s"
778+
% (row.row_key, row.table.name, self.name)
779+
)
780+
mutation_entries.append(RowMutationEntry(row.row_key, row._get_mutations()))
781+
return_statuses = [
782+
status_pb2.Status(code=code_pb2.OK) for _ in range(len(mutation_entries))
783+
] # By default, return status OKs for everything
784+
785+
try:
786+
self._table_impl.bulk_mutate_rows(
787+
mutation_entries,
788+
operation_timeout=operation_timeout,
789+
attempt_timeout=attempt_timeout,
790+
retryable_errors=retryable_errors,
791+
)
792+
except MutationsExceptionGroup as mut_exc_group:
793+
# We exception handle as follows:
794+
#
795+
# 1. Each exception in the error group is a FailedMutationEntryError, and its
796+
# cause is either a singular exception or a RetryExceptionGroup consisting of
797+
# multiple exceptions.
798+
#
799+
# 2. In the case of a singular exception, if the error does not have a gRPC status
800+
# code, we return a status code of UNKNOWN.
801+
#
802+
# 3. In the case of a RetryExceptionGroup, we use terminal exception in the exception
803+
# group and process that.
804+
for error in mut_exc_group.exceptions:
805+
cause = error.__cause__
806+
if isinstance(cause, RetryExceptionGroup):
807+
return_statuses[error.index] = self._get_status(
808+
cause.exceptions[-1]
809+
)
810+
else:
811+
return_statuses[error.index] = self._get_status(cause)
812+
813+
return return_statuses
814+
815+
@staticmethod
816+
def _get_status(error):
817+
if isinstance(error, GoogleAPICallError) and error.grpc_status_code is not None:
818+
return status_pb2.Status(
819+
code=error.grpc_status_code.value[0],
820+
message=error.message,
821+
details=error.details,
822+
)
823+
824+
return status_pb2.Status(
825+
code=code_pb2.UNKNOWN,
826+
message=str(error),
745827
)
746-
return retryable_mutate_rows(retry=retry)
747828

748829
def sample_row_keys(self):
749830
"""Read a sample of row keys in the table.
@@ -1078,133 +1159,6 @@ def restore(self, new_table_id, cluster_id=None, backup_id=None, backup_name=Non
10781159
)
10791160

10801161

1081-
class _RetryableMutateRowsWorker(object):
1082-
"""A callable worker that can retry to mutate rows with transient errors.
1083-
1084-
This class is a callable that can retry mutating rows that result in
1085-
transient errors. After all rows are successful or none of the rows
1086-
are retryable, any subsequent call on this callable will be a no-op.
1087-
"""
1088-
1089-
def __init__(self, client, table_name, rows, app_profile_id=None, timeout=None):
1090-
self.client = client
1091-
self.table_name = table_name
1092-
self.rows = rows
1093-
self.app_profile_id = app_profile_id
1094-
self.responses_statuses = [None] * len(self.rows)
1095-
self.timeout = timeout
1096-
1097-
def __call__(self, retry=DEFAULT_RETRY):
1098-
"""Attempt to mutate all rows and retry rows with transient errors.
1099-
1100-
Will retry the rows with transient errors until all rows succeed or
1101-
``deadline`` specified in the `retry` is reached.
1102-
1103-
:rtype: list
1104-
:returns: A list of response statuses (`google.rpc.status_pb2.Status`)
1105-
corresponding to success or failure of each row mutation
1106-
sent. These will be in the same order as the ``rows``.
1107-
"""
1108-
mutate_rows = self._do_mutate_retryable_rows
1109-
if retry:
1110-
mutate_rows = retry(self._do_mutate_retryable_rows)
1111-
1112-
try:
1113-
mutate_rows()
1114-
except (_BigtableRetryableError, RetryError):
1115-
# - _BigtableRetryableError raised when no retry strategy is used
1116-
# and a retryable error on a mutation occurred.
1117-
# - RetryError raised when retry deadline is reached.
1118-
# In both cases, just return current `responses_statuses`.
1119-
pass
1120-
1121-
return self.responses_statuses
1122-
1123-
@staticmethod
1124-
def _is_retryable(status):
1125-
return status is None or status.code in RETRYABLE_CODES
1126-
1127-
def _do_mutate_retryable_rows(self):
1128-
"""Mutate all the rows that are eligible for retry.
1129-
1130-
A row is eligible for retry if it has not been tried or if it resulted
1131-
in a transient error in a previous call.
1132-
1133-
:rtype: list
1134-
:return: The responses statuses, which is a list of
1135-
:class:`~google.rpc.status_pb2.Status`.
1136-
:raises: One of the following:
1137-
1138-
* :exc:`~.table._BigtableRetryableError` if any
1139-
row returned a transient error.
1140-
* :exc:`RuntimeError` if the number of responses doesn't
1141-
match the number of rows that were retried
1142-
"""
1143-
retryable_rows = []
1144-
index_into_all_rows = []
1145-
for index, status in enumerate(self.responses_statuses):
1146-
if self._is_retryable(status):
1147-
retryable_rows.append(self.rows[index])
1148-
index_into_all_rows.append(index)
1149-
1150-
if not retryable_rows:
1151-
# All mutations are either successful or non-retryable now.
1152-
return self.responses_statuses
1153-
1154-
entries = _compile_mutation_entries(self.table_name, retryable_rows)
1155-
data_client = self.client.table_data_client
1156-
1157-
kwargs = {}
1158-
if self.timeout is not None:
1159-
kwargs["timeout"] = timeout.ExponentialTimeout(deadline=self.timeout)
1160-
1161-
try:
1162-
responses = data_client.mutate_rows(
1163-
table_name=self.table_name,
1164-
entries=entries,
1165-
app_profile_id=self.app_profile_id,
1166-
retry=None,
1167-
**kwargs,
1168-
)
1169-
except RETRYABLE_MUTATION_ERRORS as exc:
1170-
# If an exception, considered retryable by `RETRYABLE_MUTATION_ERRORS`, is
1171-
# returned from the initial call, consider
1172-
# it to be retryable. Wrap as a Bigtable Retryable Error.
1173-
# For InternalServerError, it is only retriable if the message is related to RST Stream messages
1174-
if _retriable_internal_server_error(exc) or not isinstance(
1175-
exc, InternalServerError
1176-
):
1177-
raise _BigtableRetryableError
1178-
else:
1179-
# re-raise the original exception
1180-
raise
1181-
1182-
num_responses = 0
1183-
num_retryable_responses = 0
1184-
for response in responses:
1185-
for entry in response.entries:
1186-
num_responses += 1
1187-
index = index_into_all_rows[entry.index]
1188-
self.responses_statuses[index] = entry.status
1189-
if self._is_retryable(entry.status):
1190-
num_retryable_responses += 1
1191-
if entry.status.code == 0:
1192-
self.rows[index].clear()
1193-
1194-
if len(retryable_rows) != num_responses:
1195-
raise RuntimeError(
1196-
"Unexpected number of responses",
1197-
num_responses,
1198-
"Expected",
1199-
len(retryable_rows),
1200-
)
1201-
1202-
if num_retryable_responses:
1203-
raise _BigtableRetryableError
1204-
1205-
return self.responses_statuses
1206-
1207-
12081162
class ClusterState(object):
12091163
"""Representation of a Cluster State.
12101164
@@ -1351,71 +1305,3 @@ def _create_row_request(
13511305
row_set._update_message_request(message)
13521306

13531307
return message
1354-
1355-
1356-
def _compile_mutation_entries(table_name, rows):
1357-
"""Create list of mutation entries
1358-
1359-
:type table_name: str
1360-
:param table_name: The name of the table to write to.
1361-
1362-
:type rows: list
1363-
:param rows: List or other iterable of :class:`.DirectRow` instances.
1364-
1365-
:rtype: List[:class:`data_messages_v2_pb2.MutateRowsRequest.Entry`]
1366-
:returns: entries corresponding to the inputs.
1367-
:raises: :exc:`~.table.TooManyMutationsError` if the number of mutations is
1368-
greater than the max ({})
1369-
""".format(_MAX_BULK_MUTATIONS)
1370-
entries = []
1371-
mutations_count = 0
1372-
entry_klass = data_messages_v2_pb2.MutateRowsRequest.Entry
1373-
1374-
for row in rows:
1375-
_check_row_table_name(table_name, row)
1376-
_check_row_type(row)
1377-
mutations = row._get_mutation_pbs()
1378-
entries.append(entry_klass(row_key=row.row_key, mutations=mutations))
1379-
mutations_count += len(mutations)
1380-
1381-
if mutations_count > _MAX_BULK_MUTATIONS:
1382-
raise TooManyMutationsError(
1383-
"Maximum number of mutations is %s" % (_MAX_BULK_MUTATIONS,)
1384-
)
1385-
return entries
1386-
1387-
1388-
def _check_row_table_name(table_name, row):
1389-
"""Checks that a row belongs to a table.
1390-
1391-
:type table_name: str
1392-
:param table_name: The name of the table.
1393-
1394-
:type row: :class:`~google.cloud.bigtable.row.Row`
1395-
:param row: An instance of :class:`~google.cloud.bigtable.row.Row`
1396-
subclasses.
1397-
1398-
:raises: :exc:`~.table.TableMismatchError` if the row does not belong to
1399-
the table.
1400-
"""
1401-
if row.table is not None and row.table.name != table_name:
1402-
raise TableMismatchError(
1403-
"Row %s is a part of %s table. Current table: %s"
1404-
% (row.row_key, row.table.name, table_name)
1405-
)
1406-
1407-
1408-
def _check_row_type(row):
1409-
"""Checks that a row is an instance of :class:`.DirectRow`.
1410-
1411-
:type row: :class:`~google.cloud.bigtable.row.Row`
1412-
:param row: An instance of :class:`~google.cloud.bigtable.row.Row`
1413-
subclasses.
1414-
1415-
:raises: :class:`TypeError <exceptions.TypeError>` if the row is not an
1416-
instance of DirectRow.
1417-
"""
1418-
if not isinstance(row, DirectRow):
1419-
raise TypeError(
1420-
"Bulk processing can not be applied for conditional or append mutations."
1421-
)

0 commit comments

Comments
 (0)