Skip to content
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

refactor: remove setSpan() method from SessionImpl class. #3016

Closed
wants to merge 2 commits into from

Conversation

arpan14
Copy link
Collaborator

@arpan14 arpan14 commented Apr 6, 2024

Since multiplexed sessions can run more than one transaction concurrently, it shouldn't store a single span object. A new span object is created and stored in the context of the public method which is invoked. There are two changes in this PR

  • setSpan() method is removed from the SessionTransaction interface in SessionImpl class.
  • Modify setActive() method within SessionImpl interface.

setActive() method within SessionImpl does the below things

  1. Handles nested transaction use-case
  2. Invalidates active transactions - Disabling this will allow multiple transactions per session.
  3. Resets readyTransactionId - The value for readyTransactionId is only set in the prepareReadWriteTransaction method. Otherwise the value anyway remains null.
  4. Sets the span per active transaction - A session no longer can maintain a single span. Hence, we will require to change the way spans are added for multiplexed session. This is doable since for each new RPC, a new span object is created and stored in the local thread context. Instead of storing the span state in SessionImpl, we can fetch the span from context and set it.

For multiplexed session we are disabling #2, #3 and #4 for launching read-only transactions. The functionality of spans/traces for multiplexed session will be tested and added in a separate PR.

@arpan14 arpan14 requested a review from a team as a code owner April 6, 2024 09:15
@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: spanner Issues related to the googleapis/java-spanner API. labels Apr 6, 2024
@@ -485,9 +473,6 @@ <T extends SessionTransaction> T setActive(@Nullable T ctx) {
}
activeTransaction = ctx;
readyTransactionId = null;
if (activeTransaction != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be an issue.

@arpan14
Copy link
Collaborator Author

arpan14 commented Apr 15, 2024

Closing in favor of - #3027

@arpan14 arpan14 closed this Apr 15, 2024
@arpan14 arpan14 deleted the mux-pr-span-refactor branch April 15, 2024 12:19
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: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants