Skip to content
This repository has been archived by the owner on Dec 8, 2021. It is now read-only.

Sessions must only execute one Transaction at a time #464

Closed
mr-salty opened this issue Aug 28, 2019 · 0 comments · Fixed by #465
Closed

Sessions must only execute one Transaction at a time #464

mr-salty opened this issue Aug 28, 2019 · 0 comments · Fixed by #465
Assignees
Labels
api: spanner Issues related to the googleapis/google-cloud-cpp-spanner API. priority: p1 Medium priority. Will be fixed prior to next release. 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@mr-salty
Copy link
Contributor

Sessions can only execute one transaction at a time (as stated here and elsewhere). However, our current implementation does not ensure that. For each operation (call on Client), the ConnectionImpl takes a Session from the pool, makes the RPC, then returns the Session to the pool. The pool re-uses Sessions in LIFO order. So, as long as the calls to the ConnectionImpl use the same Transaction repeatedly, we will get the same Session, but in a multi-threaded program that may not be the case.

@mr-salty mr-salty added priority: p1 Medium priority. Will be fixed prior to next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Aug 28, 2019
@mr-salty mr-salty added this to the Spanner: Alpha milestone Aug 28, 2019
@mr-salty mr-salty self-assigned this Aug 28, 2019
mr-salty added a commit that referenced this issue Aug 28, 2019
mr-salty added a commit that referenced this issue Aug 28, 2019
`Transaction` now has a `SessionHolder` which is populated on-demand during
the first operation involving that `Transaction`.

The session is (normally) returned to the pool it was allocated from when
the `Transaction` is destroyed, but in the interim it will not be used by
any other `Transaction`.

- `Transaction::Visit` passes a reference to its `SessionHolder` to its
  functor. These are usually `ConnectionImpl` methods, which now use the
  session a `Transaction` already holds, or get a new session from the pool
  if needed.
- For Partition Read/Query cases:
  - Allow constructing a `Transaction` with session and transaction IDs.
  - Remove session from *Params structs (fixes #445).
  - Partitioned `Transactions` do not return their sessions to the pool, nor
    do the `PartitionRead` or `PartitionQuery` operations that created them.
- Update tests; add a matcher to check the `Transaction` session and
  transaction IDs.

Fixes #464
Fixes #445
@google-cloud-label-sync google-cloud-label-sync bot added the api: spanner Issues related to the googleapis/google-cloud-cpp-spanner API. label Jan 31, 2020
@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Apr 6, 2020
devjgm pushed a commit to devjgm/google-cloud-cpp that referenced this issue May 7, 2020
…ud-cpp-spanner#465)

`Transaction` now has a `SessionHolder` which is populated on-demand during
the first operation involving that `Transaction`.

The session is (normally) returned to the pool it was allocated from when
the `Transaction` is destroyed, but in the interim it will not be used by
any other `Transaction`.

- `Transaction::Visit` passes a reference to its `SessionHolder` to its
  functor. These are usually `ConnectionImpl` methods, which now use the
  session a `Transaction` already holds, or get a new session from the pool
  if needed.
- For Partition Read/Query cases:
  - Allow constructing a `Transaction` with session and transaction IDs.
  - Remove session from *Params structs (fixes googleapis/google-cloud-cpp-spanner#445).
  - Partitioned `Transactions` do not return their sessions to the pool, nor
    do the `PartitionRead` or `PartitionQuery` operations that created them.
- Update tests; add a matcher to check the `Transaction` session and
  transaction IDs.

Fixes googleapis/google-cloud-cpp-spanner#464
Fixes googleapis/google-cloud-cpp-spanner#445
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api: spanner Issues related to the googleapis/google-cloud-cpp-spanner API. priority: p1 Medium priority. Will be fixed prior to next release. 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants