Skip to content

chore: fallback for Partitioned Operations with multiplexed session #3710

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

Merged

Conversation

pratickchokhani
Copy link
Contributor

@pratickchokhani pratickchokhani commented Mar 26, 2025

If PartitionQueryRequest or PartitionReadRequest with multiplexed session return unimplemented error, then an implicit fallback to regular session will occur.

…tiplexed session. If PartitionQueryRequest or PartitionReadRequest with multiplexed session return unimplemented error, then an implicit fallback to regular session will occur.
@pratickchokhani pratickchokhani requested review from a team as code owners March 26, 2025 07:48
@product-auto-label product-auto-label bot added size: l Pull request size is large. api: spanner Issues related to the googleapis/java-spanner API. labels Mar 26, 2025
@olavloite olavloite changed the title feat(spanner): Implement fallback for Partitioned Operations with multiplexed session. If PartitionQueryRequest or PartitionReadRequest with multiplexed session return unimplemented error, then an implicit fallback to regular session will occur. chore: fallback for Partitioned Operations with multiplexed session Mar 26, 2025
Copy link
Collaborator

@olavloite olavloite left a comment

Choose a reason for hiding this comment

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

LGTM, with some minor nits

throw e;
}
}

void maybeMarkUnimplementedForPartitionedOps(SpannerException spannerException) {
boolean maybeMarkUnimplementedForPartitionedOps(SpannerException spannerException) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that we should mark this entire method as synchronized. In theory, the partitionRead and partitionQuery methods could be executed in parallel, and if an UNIMPLEMENTED error is returned, we would only want to create the fallback transaction once.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. Have synchronized on session.

Comment on lines 338 to 340
session.setFallbackSessionReference(sessionClient.createSession().getSessionReference());
sessionName = session.getName();
initFallbackTransaction();
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should only be executed if it had not already been executed (see also the comment above on making the method synchronized)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. Added a validation of unimplementedForPartitionedOps before executing fallback.

@pratickchokhani pratickchokhani merged commit 9940b66 into googleapis:main Mar 27, 2025
30 of 32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the googleapis/java-spanner API. size: l Pull request size is large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants