feat: Multiplex sessions v1 #15108
feat: Multiplex sessions v1 #15108
Conversation
purva9413
force-pushed
the
multiplex-sessions-v1
branch
from
384ffcc to
26993f7
Compare
…plex sessions Keeping PooledSessio usage conditions in ResultStream
purva9413
force-pushed
the
multiplex-sessions-v1
branch
2 times, most recently
from
fc576ce to
c599d33
Compare
…ions test: Fix Mutation tests to be in keeping with mux sessions test: Fix unit tests test: Fix more unit tests
purva9413
force-pushed
the
multiplex-sessions-v1
branch
from
c599d33 to
c455405
Compare
…orted txn fix: Fix empty bytestring as default
purva9413
force-pushed
the
multiplex-sessions-v1
branch
3 times, most recently
from
c097273 to
7622623
Compare
purva9413
added
the
kokoro:force-run
purva9413
force-pushed
the
multiplex-sessions-v1
branch
from
7622623 to
d4fd1c0
Compare
test: Adding some logging for emulator testing test: Special conditioning for emulator test: Changes to support some tests on the emulator chore: adding some logging for emulator chore: try create new mux for emulator to see if tests get fixed
purva9413
force-pushed
the
multiplex-sessions-v1
branch
from
d4fd1c0 to
a88dc20
Compare
amanda-tarafa
left a comment
amanda-tarafa
left a comment
There was a problem hiding this comment.
Some initial comments
| public Timestamp ReadTimestamp => Interlocked.CompareExchange(ref _transaction, null, null)?.ReadTimestamp; | ||
|
|
||
| // internal for testing | ||
| internal MultiplexedSessionPrecommitToken PrecommitToken { get; set; } |
There was a problem hiding this comment.
Let's make this a variable of the class. I'll find tests that mock the service and check the request.
apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/ManagedTransaction.cs
Show resolved
Hide resolved
apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/ManagedTransaction.cs
Show resolved
Hide resolved
apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/ManagedTransaction.cs
Show resolved
Hide resolved
| } | ||
|
|
||
| // Fetch first mutation in list, we will return the first mutation by default if it meets all conditions | ||
| Mutation key = mutations.ElementAt(0); |
There was a problem hiding this comment.
Something like this:
| Mutation key = mutations.ElementAt(0); | |
| mutations.Where(m => m.KeySet.Keys.Count > 0 || m.Delete?.KeySet.Kets.Count > 0) |
apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/ManagedTransaction.cs
Show resolved
Hide resolved
| /// </summary> | ||
| public partial class ManagedTransaction | ||
| { | ||
| private readonly ManagedSession _multiplexSession; |
There was a problem hiding this comment.
I've been thinking of something that will benefit the following use cases:
- Detached transactions
- Transaction that are ongoing while a ManagedSessions refreshes.
Instead of keeping the ManagedSession reference here, let's keep just the Session (the Spanner session) refrerence and the SpannerClient reference.
Instead of checking/attempting to refresh the session from within the ManagedTransaction, we do it internally from the ManagedSession, we add there methods to obtain the ManagedTransaction and in those methods we check/refresh the session.
Let's chat about it.
| return _session != oldSession; | ||
| } | ||
|
|
||
| internal async Task MaybeRefreshWithTimePeriodCheck() |
There was a problem hiding this comment.
I forgot to mention, I think it's best if we save this task and check on it with a lock instead of using a semaphore (the same we do for creating a transaction in ManagedTransaction). The semaphore is stopping several refreshes to be "running" at once, but it's not stopping several refreshes to be "triggered" at once, meaning we may have a succession of refreshes one after the other.
3 participants
Creating a draft PR for early review.
Idea here is SessionPool translates to the ManagedSession class. And PooledSession translates to the ManagedTransaction wrapper in this new design.
Lifecycle of the MulitplexSession -- Each query/operation coming in as part of the transaction checks for the expiry of the session. There is a proactive 'refresh' of the session every 7 days. If an operation comes in when the session is past this 7 day period, it can still safely use the existing session and we fire a session refresh in the background asynchronously. If the session is past 28 days of expiry, this session can no longer be safely used since there is already cleanup on the backend. The transaction needs to block till we get a new Session in this case.
TODO: