chore(spanner): fix mutation only case in rw mux with aborted error #3571
Conversation
product-auto-label
bot
added
size: l
harshachinta
changed the title
harshachinta
changed the title
harshachinta
changed the title
Is this something that can actually happen? |
| the mutations are not sent, and the precommit token is not received, resulting in | ||
| an INVALID_ARGUMENT error (missing precommit token) during commit. | ||
| */ | ||
| useInlinedBegin = txn.mutationsOnlyTransaction || txn.transactionId != null; |
There was a problem hiding this comment.
nit: maybe we should rename this variable to something like shouldEnsureTxnAtStart? Now this variable is being set to true for a case that never uses inlinedBegin, which could cause confusion to new engineers who read this code for the first time.
| @@ -83,6 +83,7 @@ public class SessionPoolOptions { | |||
|
|
|||
| // TODO: Change to use java.time.Duration. | |||
| private final Duration multiplexedSessionMaintenanceDuration; | |||
| private final boolean skipVerifyingBeginTransactionForMuxRW; | |||
There was a problem hiding this comment.
Is this temporary just to be able to test with/without this fix?
| // request | ||
| // and precommit token is set in Commit request. |
There was a problem hiding this comment.
super-nit: fix formatting
| // is cleared | ||
| // after the first call, so the retry should succeed. |
| Spanner spanner1 = | ||
| SpannerOptions.newBuilder() | ||
| .setProjectId("test-project") | ||
| .setChannelProvider(channelProvider) | ||
| .setCredentials(NoCredentials.getInstance()) | ||
| .setSessionPoolOption( | ||
| SessionPoolOptions.newBuilder() | ||
| .setUseMultiplexedSession(true) | ||
| .setUseMultiplexedSessionForRW(true) | ||
| .setSkipVerifyingBeginTransactionForMuxRW(true) | ||
| .build()) | ||
| .build() | ||
| .getService(); |
There was a problem hiding this comment.
nit: it seems like a very large part of these tests are equal (e.g. setting up Spanner and verifying the requests). Can we combine those parts into shared methods that are used by the various test methods?
This PR fixes the following scenario.
In a mutation-only scenario for a read-write transaction using multiplexed sessions, an explicit BeginTransaction RPC is triggered after the user operation completes.
However, if an ABORT occurs during this BeginTransaction RPC, the client retry mechanism performs an explicit BeginTransaction before the user operation begins. This behavior causes issues in multiplexed sessions because, in mutation-only cases, the BeginTransaction requires a mutation key to generate a precommit token. This precommit token is essential for committing the transaction.
Currently, since the BeginTransaction during a retry is executed before the user operation, no mutation key is available, and consequently, no precommit token is included in the transaction response. This leads to commit failures with an error as shown below
The fix involves performing a BeginTransaction RPC for mutation-only cases with multiplexed sessions after the user operation completes, regardless of whether it is an initial operation or a retry.