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

feat: add new_transaction support #499

Merged
merged 47 commits into from Apr 4, 2024
Merged

feat: add new_transaction support #499

merged 47 commits into from Apr 4, 2024

Conversation

daniel-sanche
Copy link
Contributor

@daniel-sanche daniel-sanche commented Nov 28, 2023

This PR modifies the Transaction class to add a new begin_later argument.

When True, the transaction will not initialize itself at the start of the context manager __enter__ block. Instead, it will only begin at the first rpc. If the first rpc is a read call, this allows us to create the transaction at the same time as the call, saving a network call.

This new behaviour is enabled by default, and is aligned with changes in other languages


For reviewers

In Python, a transaction is created in a context manager, and typically interacted with through the client (which passes through to the Transaction object when needed):

with client.transaction(begin_later=True):
  client.query(query)
  client.get(entity)
  client.put(entity)
  client.delete(entity)

if begin_later is False, it will call begin automatically when entering that context manager block (using __enter__ method), retrieving a transaction_id from the server. Otherwise, begin is deferred until a get/query/commit call forces the transaction to talk to the backend

States

The state machine looks like this. Green lines denote transitions that are only valid when begin_later=True orange lines denote transitions that are valid when begin_later=False

Untitled Diagram drawio (2)

  • Starts out in INITIAL state
    • if begin_later is true, it allows you to immediately start collecting put and get mutations. Otherwise any rpcs in the INITIAL state will throw an exception
  • When begin is called and the transaction receives and ID from the server, it moves into IN_PROGRESS state
    • if begin_later is false, it will immediately call begin when enter is called (called by the with statement)
    • if begin_later is true, it will call begin when it encounters the first get or query call
    • user can also always call begin manually
  • a rollback from either state (or a commit with no content from INITIAL) will result in the terminal ABORTED state
  • a commit from IN_PROGRESS results in the terminal FINISHED state
  • any invalid attempted transitions will raise an exception

@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: datastore Issues related to the googleapis/python-datastore API. labels Nov 28, 2023
@product-auto-label product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Nov 28, 2023
@daniel-sanche daniel-sanche marked this pull request as ready for review November 29, 2023 00:00
@daniel-sanche daniel-sanche requested review from a team as code owners November 29, 2023 00:00
@daniel-sanche daniel-sanche changed the title [DRAFT] feat: add new_transaction support feat: add new_transaction support Nov 29, 2023
@daniel-sanche daniel-sanche requested a review from a team as a code owner February 9, 2024 21:59
@cindy-peng
Copy link

This PR modifies the Transaction class to add a new begin_later argument.

When True, the transaction will not initialize itself at the start of the context manager __enter__ block. Instead, it will only begin at the first rpc. If the first rpc is a read call, this allows us to create the transaction at the same time as the call, saving a network call.

This new behaviour is enabled by default, and is aligned with changes in other languages

Hi Daniel, it looks like begin_later is by default set to False so by default disabled? Do we want this behavior to be enabled by default?

@daniel-sanche
Copy link
Contributor Author

Yes, I believe it's intended to be off by default. Having it on or off may be more efficient depending on the circumstances, so we opted to keep it off by default (correct me if I'm misremembering @danieljbruce @bhshkh)

@daniel-sanche daniel-sanche merged commit 43855dd into main Apr 4, 2024
28 checks passed
@daniel-sanche daniel-sanche deleted the new_transaction branch April 4, 2024 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: datastore Issues related to the googleapis/python-datastore API. size: l Pull request size is large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants