-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(bigtable): Reroute Mutations Batcher to use data client #18200
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
Changes from all commits
5ba6d50
206af9b
3ae897f
78cc8c9
87f74b5
1bfb126
49e1dd2
6aae668
ac7f05a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -295,10 +295,11 @@ def __init__( | |
| self._exceptions_since_last_raise: int = 0 | ||
| # keep track of the first and last _exception_list_limit exceptions | ||
| self._exception_list_limit: int = 10 | ||
| self._oldest_exceptions: list[Exception] = [] | ||
| self._newest_exceptions: deque[Exception] = deque( | ||
| self._oldest_exceptions: list[FailedMutationEntryError] = [] | ||
| self._newest_exceptions: deque[FailedMutationEntryError] = deque( | ||
| maxlen=self._exception_list_limit | ||
| ) | ||
| # only used by the shim right now. | ||
| self._user_batch_completed_callback: ( | ||
| Callable[[list[status_pb2.Status]], Any] | None | ||
| ) = None | ||
|
|
@@ -385,7 +386,11 @@ async def _flush_internal(self, new_entries: list[RowMutationEntry]): | |
| new_entries list of RowMutationEntry objects to flush | ||
| """ | ||
| # flush new entries | ||
| in_process_requests: list[CrossSync.Future[list[FailedMutationEntryError]]] = [] | ||
| in_process_requests: list[ | ||
| tuple[ | ||
| CrossSync.Future[list[FailedMutationEntryError]], list[RowMutationEntry] | ||
| ] | ||
| ] = [] | ||
| async for batch, metric in self._flow_control.add_to_flow_with_metrics( | ||
| new_entries, self._target.client._metrics | ||
| ): | ||
|
|
@@ -395,7 +400,7 @@ async def _flush_internal(self, new_entries: list[RowMutationEntry]): | |
| metric, | ||
| sync_executor=self._sync_rpc_executor, | ||
| ) | ||
| in_process_requests.append(batch_task) | ||
| in_process_requests.append((batch_task, batch)) | ||
| # wait for all inflight requests to complete | ||
| found_exceptions = await self._wait_for_batch_results(*in_process_requests) | ||
| # update exception data to reflect any new errors | ||
|
|
@@ -453,7 +458,7 @@ async def _execute_mutate_rows( | |
| ) | ||
| return [] | ||
|
|
||
| def _add_exceptions(self, excs: list[Exception]): | ||
| def _add_exceptions(self, excs: list[FailedMutationEntryError]): | ||
| """ | ||
| Add new list of exceptions to internal store. To avoid unbounded memory, | ||
| the batcher will store the first and last _exception_list_limit exceptions, | ||
|
|
@@ -553,26 +558,28 @@ def _on_exit(self): | |
| @staticmethod | ||
| @CrossSync.convert | ||
| async def _wait_for_batch_results( | ||
| *tasks: CrossSync.Future[list[FailedMutationEntryError]] | ||
| | CrossSync.Future[None], | ||
| ) -> list[Exception]: | ||
| *tasks: tuple[ | ||
| CrossSync.Future[list[FailedMutationEntryError]] | CrossSync.Future[None], | ||
| list[RowMutationEntry], | ||
| ], | ||
| ) -> list[FailedMutationEntryError]: | ||
| """ | ||
| Takes in a list of futures representing _execute_mutate_rows tasks, | ||
| waits for them to complete, and returns a list of errors encountered. | ||
|
|
||
| Args: | ||
| *tasks: futures representing _execute_mutate_rows or _flush_internal tasks | ||
| *tasks: Tuples of futures representing _execute_mutate_rows or | ||
| _flush_internal tasks, and their associated batches | ||
| Returns: | ||
| list[Exception]: | ||
| list of Exceptions encountered by any of the tasks. Errors are expected | ||
| to be FailedMutationEntryError, representing a failed mutation operation. | ||
| If a task fails with a different exception, it will be included in the | ||
| output list. Successful tasks will not be represented in the output list. | ||
| list[FailedMutationEntryError]: | ||
| list of FailedMutationEntryError encountered by any of the tasks, | ||
| representing a failed mutation operation. | ||
| Successful tasks will not be represented in the output list. | ||
| """ | ||
| if not tasks: | ||
| return [] | ||
| exceptions: list[Exception] = [] | ||
| for task in tasks: | ||
| exceptions: list[FailedMutationEntryError] = [] | ||
| for task, batch in tasks: | ||
| if CrossSync.is_async: | ||
| # futures don't need to be awaited in sync mode | ||
| await task | ||
|
|
@@ -584,6 +591,17 @@ async def _wait_for_batch_results( | |
| # strip index information | ||
| exc.index = None | ||
| exceptions.extend(exc_list) | ||
| except Exception as e: | ||
| except FailedMutationEntryError as e: | ||
| e.index = None | ||
| exceptions.append(e) | ||
| except Exception as e: | ||
| exceptions.extend( | ||
| [ | ||
| FailedMutationEntryError( | ||
| failed_idx=None, failed_mutation_entry=entry, cause=e | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what does field_index=None mean? should it be the index of the current entry?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| ) | ||
| for entry in batch | ||
| ] | ||
| ) | ||
|
|
||
| return exceptions | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would task raise a FailedMutationEntry? Or it always return a list?