Skip to content

perf(spanner): avoid allocating empty RequestOptions on read and query paths #18422

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.

Sign up for GitHub

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

Open
olavloite wants to merge 1 commit into
base: main
Choose a base branch
from
Open

perf(spanner): avoid allocating empty RequestOptions on read and query paths #18422

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 28 additions & 28 deletions packages/google-cloud-spanner/google/cloud/spanner_v1/_async/snapshot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,20 +409,20 @@ async def read(
)
request_options = _merge_request_options(request_options, client_context)

if request_options is None:
request_options = RequestOptions()
elif type(request_options) is dict:
request_options = RequestOptions(request_options)

if self._read_only:
request_options.transaction_tag = None
if (
directed_read_options is None
and database._directed_read_options is not None
):
directed_read_options = database._directed_read_options
elif self.transaction_tag is not None:
request_options.transaction_tag = self.transaction_tag
if request_options is not None:
if self._read_only:
request_options.transaction_tag = None
elif self.transaction_tag is not None:
request_options.transaction_tag = self.transaction_tag
Comment thread
olavloite marked this conversation as resolved.
Comment thread
olavloite marked this conversation as resolved.
elif not self._read_only and self.transaction_tag is not None:
request_options = RequestOptions(transaction_tag=self.transaction_tag)
Comment thread
olavloite marked this conversation as resolved.
Comment thread
olavloite marked this conversation as resolved.

if (
self._read_only
and directed_read_options is None
and database._directed_read_options is not None
):
directed_read_options = database._directed_read_options

read_request = ReadRequest(
session=session.name,
Expand Down Expand Up @@ -603,20 +603,20 @@ async def execute_sql(
)
request_options = _merge_request_options(request_options, client_context)

if request_options is None:
request_options = RequestOptions()
elif type(request_options) is dict:
request_options = RequestOptions(request_options)

if self._read_only:
request_options.transaction_tag = None
if (
directed_read_options is None
and database._directed_read_options is not None
):
directed_read_options = database._directed_read_options
elif self.transaction_tag is not None:
request_options.transaction_tag = self.transaction_tag
if request_options is not None:
if self._read_only:
request_options.transaction_tag = None
elif self.transaction_tag is not None:
request_options.transaction_tag = self.transaction_tag
Comment thread
olavloite marked this conversation as resolved.
Comment thread
olavloite marked this conversation as resolved.
elif not self._read_only and self.transaction_tag is not None:
request_options = RequestOptions(transaction_tag=self.transaction_tag)
Comment thread
olavloite marked this conversation as resolved.
Comment thread
olavloite marked this conversation as resolved.

if (
self._read_only
and directed_read_options is None
and database._directed_read_options is not None
):
directed_read_options = database._directed_read_options

execute_sql_request = ExecuteSqlRequest(
session=session.name,
Expand Down
18 changes: 8 additions & 10 deletions packages/google-cloud-spanner/google/cloud/spanner_v1/_async/transaction.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -524,11 +524,10 @@ async def execute_update(
)
request_options = _merge_request_options(request_options, client_context)

if request_options is None:
request_options = RequestOptions()
elif type(request_options) is dict:
request_options = RequestOptions(request_options)
request_options.transaction_tag = self.transaction_tag
if request_options is not None:
request_options.transaction_tag = self.transaction_tag
Comment thread
olavloite marked this conversation as resolved.
elif self.transaction_tag is not None:
request_options = RequestOptions(transaction_tag=self.transaction_tag)
Comment thread
olavloite marked this conversation as resolved.
Comment thread
olavloite marked this conversation as resolved.

trace_attributes = {
"db.statement": dml,
Expand Down Expand Up @@ -684,11 +683,10 @@ async def batch_update(
)
request_options = _merge_request_options(request_options, client_context)

if request_options is None:
request_options = RequestOptions()
elif type(request_options) is dict:
request_options = RequestOptions(request_options)
request_options.transaction_tag = self.transaction_tag
if request_options is not None:
request_options.transaction_tag = self.transaction_tag
Comment thread
olavloite marked this conversation as resolved.
elif self.transaction_tag is not None:
request_options = RequestOptions(transaction_tag=self.transaction_tag)
Comment thread
olavloite marked this conversation as resolved.
Comment thread
olavloite marked this conversation as resolved.

trace_attributes = {
# Get just the queries from the DML statement batch
Expand Down
13 changes: 5 additions & 8 deletions packages/google-cloud-spanner/google/cloud/spanner_v1/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,9 +320,7 @@ def __init__(
raise ValueError(
"Both username and password must be specified for Omni authentication"
)
from google.cloud.spanner_v1.omni.credentials import (
SpannerOmniCredentials,
)
from google.cloud.spanner_v1.omni.credentials import SpannerOmniCredentials

if has_username and has_password:
credentials = SpannerOmniCredentials(
Expand All @@ -339,11 +337,10 @@ def __init__(
disable_builtin_metrics = True
elif isinstance(credentials, AnonymousCredentials):
self._emulator_host = self._client_options.api_endpoint
else:
if username is not None or password is not None:
raise ValueError(
"username and password can only be used when instance_type='omni'."
)
elif username is not None or password is not None:
raise ValueError(
"username and password can only be used when instance_type='omni'."
)
super(Client, self).__init__(
project=project,
credentials=credentials,
Expand Down
52 changes: 26 additions & 26 deletions packages/google-cloud-spanner/google/cloud/spanner_v1/snapshot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,19 +362,19 @@ def read(
database._instance._client._client_context, self._client_context
)
request_options = _merge_request_options(request_options, client_context)
if request_options is None:
request_options = RequestOptions()
elif type(request_options) is dict:
request_options = RequestOptions(request_options)
if self._read_only:
request_options.transaction_tag = None
if (
directed_read_options is None
and database._directed_read_options is not None
):
directed_read_options = database._directed_read_options
elif self.transaction_tag is not None:
request_options.transaction_tag = self.transaction_tag
if request_options is not None:
if self._read_only:
request_options.transaction_tag = None
elif self.transaction_tag is not None:
request_options.transaction_tag = self.transaction_tag
Comment thread
olavloite marked this conversation as resolved.
Comment thread
olavloite marked this conversation as resolved.
elif not self._read_only and self.transaction_tag is not None:
request_options = RequestOptions(transaction_tag=self.transaction_tag)
Comment thread
olavloite marked this conversation as resolved.
Comment thread
olavloite marked this conversation as resolved.
if (
self._read_only
and directed_read_options is None
and (database._directed_read_options is not None)
):
directed_read_options = database._directed_read_options
read_request = ReadRequest(
session=session.name,
table=table,
Expand Down Expand Up @@ -543,19 +543,19 @@ def execute_sql(
database._instance._client._client_context, self._client_context
)
request_options = _merge_request_options(request_options, client_context)
if request_options is None:
request_options = RequestOptions()
elif type(request_options) is dict:
request_options = RequestOptions(request_options)
if self._read_only:
request_options.transaction_tag = None
if (
directed_read_options is None
and database._directed_read_options is not None
):
directed_read_options = database._directed_read_options
elif self.transaction_tag is not None:
request_options.transaction_tag = self.transaction_tag
if request_options is not None:
if self._read_only:
request_options.transaction_tag = None
elif self.transaction_tag is not None:
request_options.transaction_tag = self.transaction_tag
Comment thread
olavloite marked this conversation as resolved.
Comment thread
olavloite marked this conversation as resolved.
elif not self._read_only and self.transaction_tag is not None:
request_options = RequestOptions(transaction_tag=self.transaction_tag)
Comment thread
olavloite marked this conversation as resolved.
Comment thread
olavloite marked this conversation as resolved.
if (
self._read_only
and directed_read_options is None
and (database._directed_read_options is not None)
):
directed_read_options = database._directed_read_options
execute_sql_request = ExecuteSqlRequest(
session=session.name,
sql=sql,
Expand Down
18 changes: 8 additions & 10 deletions packages/google-cloud-spanner/google/cloud/spanner_v1/transaction.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,11 +436,10 @@ def execute_update(
database._instance._client._client_context, self._client_context
)
request_options = _merge_request_options(request_options, client_context)
if request_options is None:
request_options = RequestOptions()
elif type(request_options) is dict:
request_options = RequestOptions(request_options)
request_options.transaction_tag = self.transaction_tag
if request_options is not None:
request_options.transaction_tag = self.transaction_tag
Comment thread
olavloite marked this conversation as resolved.
elif self.transaction_tag is not None:
request_options = RequestOptions(transaction_tag=self.transaction_tag)
Comment thread
olavloite marked this conversation as resolved.
Comment thread
olavloite marked this conversation as resolved.
trace_attributes = {"db.statement": dml, "request_options": request_options}
is_inline_begin = False
if self._transaction_id is None:
Expand Down Expand Up @@ -575,11 +574,10 @@ def batch_update(
database._instance._client._client_context, self._client_context
)
request_options = _merge_request_options(request_options, client_context)
if request_options is None:
request_options = RequestOptions()
elif type(request_options) is dict:
request_options = RequestOptions(request_options)
request_options.transaction_tag = self.transaction_tag
if request_options is not None:
request_options.transaction_tag = self.transaction_tag
Comment thread
olavloite marked this conversation as resolved.
elif self.transaction_tag is not None:
request_options = RequestOptions(transaction_tag=self.transaction_tag)
Comment thread
olavloite marked this conversation as resolved.
Comment thread
olavloite marked this conversation as resolved.
trace_attributes = {
"db.statement": ";".join([statement.sql for statement in parsed]),
"request_options": request_options,
Expand Down
Loading
Loading