Skip to content

Commit 3d6358f

Browse files
committed
perf(spanner): eliminate redundant tracing span in run_in_transaction
Eliminate the redundant inner 'CloudSpanner.Session.run_in_transaction' span and duplicate MetricsCapture context, leaving 'CloudSpanner.Database.run_in_transaction' as the single transaction lifecycle span (matching Java and Go client behavior). - Attach transaction retry, abort, and rollback events directly to the active span. - Guard span event creation, retry delay computation, and exception stringification behind span.is_recording(). - Refactor the run_in_transaction retry loop by extracting _create_transaction_for_attempt and _handle_aborted helpers.
1 parent f40cf77 commit 3d6358f

11 files changed

Lines changed: 937 additions & 257 deletions

File tree

packages/google-cloud-spanner/.cross_sync/generate.py

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,10 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414
from __future__ import annotations
15-
from typing import Sequence
15+
1616
import ast
17+
from typing import Sequence
18+
1719
"""
1820
Entrypoint for initiating an async -> sync conversion using CrossSync
1921
@@ -35,12 +37,13 @@ def extract_header_comments(file_path) -> str:
3537
header.append(line)
3638
else:
3739
break
38-
header.append("\n# This file is automatically generated by CrossSync. Do not edit manually.\n\n")
40+
header.append(
41+
"\n# This file is automatically generated by CrossSync. Do not edit manually.\n\n"
42+
)
3943
return "".join(header)
4044

4145

4246
class CrossSyncOutputFile:
43-
4447
def __init__(self, output_path: str, ast_tree, header: str | None = None):
4548
self.output_path = output_path
4649
self.tree = ast_tree
@@ -56,15 +59,19 @@ def render(self, with_formatter=True, save_to_disk: bool = True) -> str:
5659
"""
5760
full_str = self.header + ast.unparse(self.tree)
5861
if with_formatter:
59-
import black # type: ignore
60-
import autoflake # type: ignore
61-
62-
full_str = black.format_str(
63-
autoflake.fix_code(full_str, remove_all_unused_imports=True),
64-
mode=black.FileMode(),
65-
)
62+
try:
63+
import autoflake # type: ignore
64+
import black # type: ignore
65+
66+
full_str = black.format_str(
67+
autoflake.fix_code(full_str, remove_all_unused_imports=True),
68+
mode=black.FileMode(),
69+
)
70+
except ImportError:
71+
pass
6672
if save_to_disk:
6773
import os
74+
6875
os.makedirs(os.path.dirname(self.output_path), exist_ok=True)
6976
with open(self.output_path, "w") as f:
7077
f.write(full_str)
@@ -73,10 +80,15 @@ def render(self, with_formatter=True, save_to_disk: bool = True) -> str:
7380

7481
def convert_files_in_dir(directory: str) -> set[CrossSyncOutputFile]:
7582
import glob
83+
import os
84+
7685
from transformers import CrossSyncFileProcessor
7786

78-
# find all python files in the directory
79-
files = glob.glob(directory + "/**/*.py", recursive=True)
87+
# find all python files in the directory or use single file
88+
if os.path.isfile(directory):
89+
files = [directory]
90+
else:
91+
files = glob.glob(directory + "/**/*.py", recursive=True)
8092
# keep track of the output files pointed to by the annotated classes
8193
artifacts: set[CrossSyncOutputFile] = set()
8294
file_transformer = CrossSyncFileProcessor()

packages/google-cloud-spanner/google/cloud/spanner_v1/_async/session.py

Lines changed: 184 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -518,6 +518,107 @@ def transaction(self, client_context=None) -> Transaction:
518518

519519
return Transaction(self, client_context=client_context)
520520

521+
def _create_transaction_for_attempt(
522+
self,
523+
client_context=None,
524+
transaction_tag=None,
525+
exclude_txn_from_change_streams=None,
526+
isolation_level=None,
527+
read_lock_mode=None,
528+
previous_transaction_id=None,
529+
) -> Transaction:
530+
"""Create and configure a transaction instance for a single attempt.
531+
532+
:type client_context: :class:`~google.cloud.spanner_v1.client_context.ClientContext`
533+
:param client_context: (Optional) client context to use for the transaction.
534+
535+
:type transaction_tag: str
536+
:param transaction_tag: (Optional) transaction tag.
537+
538+
:type exclude_txn_from_change_streams: bool
539+
:param exclude_txn_from_change_streams: (Optional) whether to exclude from change streams.
540+
541+
:type isolation_level: int
542+
:param isolation_level: (Optional) isolation level.
543+
544+
:type read_lock_mode: int
545+
:param read_lock_mode: (Optional) read lock mode.
546+
547+
:type previous_transaction_id: bytes
548+
:param previous_transaction_id: (Optional) previous transaction id for multiplexed sessions.
549+
550+
:rtype: :class:`~google.cloud.spanner_v1.transaction.Transaction`
551+
:returns: A configured Transaction instance.
552+
"""
553+
transaction = self.transaction(client_context=client_context)
554+
transaction.transaction_tag = transaction_tag
555+
transaction.exclude_txn_from_change_streams = exclude_txn_from_change_streams
556+
transaction.isolation_level = isolation_level
557+
transaction.read_lock_mode = read_lock_mode
558+
559+
if self.is_multiplexed:
560+
transaction._multiplexed_session_previous_transaction_id = (
561+
previous_transaction_id
562+
)
563+
return transaction
564+
565+
@CrossSync.convert
566+
async def _handle_aborted(
567+
self,
568+
exception,
569+
span,
570+
event_name,
571+
attempts,
572+
deadline,
573+
default_retry_delay,
574+
include_cause=False,
575+
):
576+
"""Handle an Aborted error: record trace event and delay until next retry attempt.
577+
578+
:type exception: :class:`google.api_core.exceptions.Aborted`
579+
:param exception: The aborted exception.
580+
581+
:type span: :class:`opentelemetry.trace.Span`
582+
:param span: The current active span.
583+
584+
:type event_name: str
585+
:param event_name: The span event name to record.
586+
587+
:type attempts: int
588+
:param attempts: The retry attempt number.
589+
590+
:type deadline: float
591+
:param deadline: Timestamp deadline for retrying.
592+
593+
:type default_retry_delay: float
594+
:param default_retry_delay: Default delay between retries.
595+
596+
:type include_cause: bool
597+
:param include_cause: (Optional) Whether to include the exception string as cause.
598+
"""
599+
if span and span.is_recording():
600+
cause = (
601+
exception.errors[0] if getattr(exception, "errors", None) else exception
602+
)
603+
delay_seconds = _get_retry_delay(
604+
cause,
605+
attempts,
606+
default_retry_delay=default_retry_delay,
607+
)
608+
attributes = {
609+
"attempt": attempts,
610+
"delay_seconds": delay_seconds,
611+
}
612+
if include_cause:
613+
attributes["cause"] = str(exception)
614+
add_span_event(span, event_name, attributes)
615+
await _delay_until_retry(
616+
exception,
617+
deadline,
618+
attempts,
619+
default_retry_delay=default_retry_delay,
620+
)
621+
521622
@CrossSync.convert
522623
async def run_in_transaction(self, func, *args, **kw):
523624
"""Perform a unit of work in a transaction, retrying on abort.
@@ -568,126 +669,94 @@ async def run_in_transaction(self, func, *args, **kw):
568669
database = self._database
569670
log_commit_stats = database.log_commit_stats
570671

571-
extra_attributes = {}
572-
if transaction_tag:
573-
extra_attributes["transaction.tag"] = transaction_tag
574-
575-
with (
576-
trace_call(
577-
"CloudSpanner.Session.run_in_transaction",
578-
self,
579-
extra_attributes=extra_attributes,
580-
observability_options=getattr(database, "observability_options", None),
581-
) as span,
582-
MetricsCapture(self._resource_info),
583-
):
584-
attempts: int = 0
585-
586-
# If a transaction using a multiplexed session is retried after an aborted
587-
# user operation, it should include the previous transaction ID in the
588-
# transaction options used to begin the transaction. This allows the backend
589-
# to recognize the transaction and increase the lock order for the new
590-
# transaction that is created.
591-
# See :attr:`~google.cloud.spanner_v1.types.TransactionOptions.ReadWrite.multiplexed_session_previous_transaction_id`
592-
previous_transaction_id: Optional[bytes] = None
593-
594-
while True:
595-
txn = self.transaction(client_context=client_context)
596-
txn.transaction_tag = transaction_tag
597-
txn.exclude_txn_from_change_streams = exclude_txn_from_change_streams
598-
txn.isolation_level = isolation_level
599-
txn.read_lock_mode = read_lock_mode
600-
601-
if self.is_multiplexed:
602-
txn._multiplexed_session_previous_transaction_id = (
603-
previous_transaction_id
604-
)
672+
span = get_current_span()
673+
attempts: int = 0
674+
675+
# If a transaction using a multiplexed session is retried after an aborted
676+
# user operation, it should include the previous transaction ID in the
677+
# transaction options used to begin the transaction. This allows the backend
678+
# to recognize the transaction and increase the lock order for the new
679+
# transaction that is created.
680+
# See :attr:`~google.cloud.spanner_v1.types.TransactionOptions.ReadWrite.multiplexed_session_previous_transaction_id`
681+
previous_transaction_id: Optional[bytes] = None
682+
683+
while True:
684+
transaction = self._create_transaction_for_attempt(
685+
client_context=client_context,
686+
transaction_tag=transaction_tag,
687+
exclude_txn_from_change_streams=exclude_txn_from_change_streams,
688+
isolation_level=isolation_level,
689+
read_lock_mode=read_lock_mode,
690+
previous_transaction_id=previous_transaction_id,
691+
)
692+
attempts += 1
605693

606-
attempts += 1
607-
span_attributes = dict(attempt=attempts)
694+
try:
695+
return_value = await CrossSync.run_if_async(
696+
func, transaction, *args, **kw
697+
)
608698

609-
try:
610-
return_value = await CrossSync.run_if_async(func, txn, *args, **kw)
611-
612-
except Aborted as exc:
613-
previous_transaction_id = txn._transaction_id
614-
delay_seconds = _get_retry_delay(
615-
exc.errors[0],
616-
attempts,
617-
default_retry_delay=default_retry_delay,
618-
)
619-
attributes = dict(delay_seconds=delay_seconds, cause=str(exc))
620-
attributes.update(span_attributes)
621-
add_span_event(
622-
span,
623-
"Transaction was aborted in user operation, retrying",
624-
attributes,
625-
)
626-
await _delay_until_retry(
627-
exc,
628-
deadline,
629-
attempts,
630-
default_retry_delay=default_retry_delay,
631-
)
632-
continue
699+
except Aborted as exc:
700+
previous_transaction_id = transaction._transaction_id
701+
await self._handle_aborted(
702+
exc,
703+
span,
704+
"Transaction was aborted in user operation, retrying",
705+
attempts,
706+
deadline,
707+
default_retry_delay,
708+
include_cause=True,
709+
)
710+
continue
633711

634-
except GoogleAPICallError:
635-
add_span_event(
636-
span,
637-
"User operation failed due to GoogleAPICallError, not retrying",
638-
span_attributes,
639-
)
640-
raise
712+
except GoogleAPICallError:
713+
add_span_event(
714+
span,
715+
"User operation failed due to GoogleAPICallError, not retrying",
716+
{"attempt": attempts},
717+
)
718+
raise
641719

642-
except Exception:
643-
add_span_event(
644-
span,
645-
"User operation failed. Invoking Transaction.rollback(), not retrying",
646-
span_attributes,
647-
)
648-
await txn.rollback()
649-
raise
720+
except Exception:
721+
add_span_event(
722+
span,
723+
"User operation failed. Invoking Transaction.rollback(), not retrying",
724+
{"attempt": attempts},
725+
)
726+
await transaction.rollback()
727+
raise
728+
729+
try:
730+
await transaction.commit(
731+
return_commit_stats=log_commit_stats,
732+
request_options=commit_request_options,
733+
max_commit_delay=max_commit_delay,
734+
)
650735

651-
try:
652-
await txn.commit(
653-
return_commit_stats=log_commit_stats,
654-
request_options=commit_request_options,
655-
max_commit_delay=max_commit_delay,
656-
)
736+
except Aborted as exc:
737+
previous_transaction_id = transaction._transaction_id
738+
await self._handle_aborted(
739+
exc,
740+
span,
741+
"Transaction was aborted during commit, retrying",
742+
attempts,
743+
deadline,
744+
default_retry_delay,
745+
)
746+
continue
657747

658-
except Aborted as exc:
659-
previous_transaction_id = txn._transaction_id
660-
delay_seconds = _get_retry_delay(
661-
exc.errors[0],
662-
attempts,
663-
default_retry_delay=default_retry_delay,
664-
)
665-
attributes = dict(delay_seconds=delay_seconds)
666-
attributes.update(span_attributes)
667-
add_span_event(
668-
span,
669-
"Transaction was aborted during commit, retrying",
670-
attributes,
671-
)
672-
await _delay_until_retry(
673-
exc,
674-
deadline,
675-
attempts,
676-
default_retry_delay=default_retry_delay,
677-
)
748+
except GoogleAPICallError:
749+
add_span_event(
750+
span,
751+
"Transaction.commit failed due to GoogleAPICallError, not retrying",
752+
{"attempt": attempts},
753+
)
754+
raise
678755

679-
except GoogleAPICallError:
680-
add_span_event(
681-
span,
682-
"Transaction.commit failed due to GoogleAPICallError, not retrying",
683-
span_attributes,
756+
else:
757+
if log_commit_stats and transaction.commit_stats:
758+
database.logger.info(
759+
"CommitStats: {}".format(transaction.commit_stats),
760+
extra={"commit_stats": transaction.commit_stats},
684761
)
685-
raise
686-
687-
else:
688-
if log_commit_stats and txn.commit_stats:
689-
database.logger.info(
690-
"CommitStats: {}".format(txn.commit_stats),
691-
extra={"commit_stats": txn.commit_stats},
692-
)
693-
return return_value
762+
return return_value

packages/google-cloud-spanner/google/cloud/spanner_v1/_helpers.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -856,7 +856,7 @@ def _delay_until_retry(exc, deadline, attempts, default_retry_delay=None):
856856
:param attempts: number of call retries
857857
"""
858858

859-
cause = exc.errors[0]
859+
cause = exc.errors[0] if getattr(exc, "errors", None) else exc
860860
now = time.time()
861861
if now >= deadline:
862862
raise

0 commit comments

Comments
 (0)