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

Outside of Transactions, always use single-use transactions #347

Closed
stephenplusplus opened this issue Sep 29, 2018 · 8 comments
Closed

Outside of Transactions, always use single-use transactions #347

stephenplusplus opened this issue Sep 29, 2018 · 8 comments
Assignees
Labels
api: spanner Issues related to the googleapis/nodejs-spanner API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@stephenplusplus
Copy link
Contributor

RE: #16 (comment)

And default strong: true to enable strong reads.

@snehashah16 -- correct?

@JustinBeckwith JustinBeckwith added the triage me I really want to be triaged. label Sep 30, 2018
@snehashah16
Copy link

i am not sure what is covered within "always use single-use transactions".. could u provide more information

@stephenplusplus
Copy link
Contributor Author

This refers back to #16 (comment)

@snehashah16
Copy link

reading through another bug, i noticed there is a heavy usage of singleUse within node library.
for all writes (insert|update|delete), it would best performed within an explicit txn ( begin txn -> modify data -> commit). This guarantees idempotency due to replays etc. docs here

@JustinBeckwith JustinBeckwith added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. and removed triage me I really want to be triaged. labels Oct 1, 2018
@stephenplusplus
Copy link
Contributor Author

For the write methods, I think that should be fine (@callmehiphop ?)

Regarding this question: #16 (comment) -- what do you think? database.run uses executeStreamingSql underneath.

@callmehiphop
Copy link
Contributor

callmehiphop commented Oct 3, 2018

@snehashah16 we could probably use a refactor to make the code clearer, but only the Table object makes heavy use of single use transactions. All writes done via Transactions actually use the begin > commit workflow.

As for Database#run, I was always under the impression that the backend created a single use transaction object for us. The ExecuteSqlRequest proto docs say

The transaction to use. If none is provided, the default is a temporary read-only transaction with strong concurrency.

@snehashah16
Copy link

@callmehiphop @crwilcox
Lets discuss this and have a design agreed upon.

@stephenplusplus
Copy link
Contributor Author

@snehashah16 I reviewed our methods to determine the ones that read and the ones that write:

Database

  • run() / runStream()
    • Spanner > executeStreamingSql

Table

  • createReadStream() / read()

    • Spanner > streamingRead
  • deleteRows()

    • Spanner > commit
  • insert()

    • Spanner > commit
  • replace()

    • Spanner > commit
  • update()

    • Spanner > commit
  • upsert()

    • Spanner > commit

I've sent a PR that will do 2 things when complete:

  1. Change all of the methods that call commit outside of a transaction, to instead create and commit the mutation inside of a transaction
  2. Change the read methods that call executeStreamingSql and streamingRead to use a single use, strong read (by default) transaction

That PR is in progress here: #361. Please let me know if the above plan is correct.

@stephenplusplus
Copy link
Contributor Author

The work is done: #361

Step 2 wasn't necessary. By default, single-use, read-only, strong-concurrency transactions are used for executeStreamingSql and streamingRead.

@google-cloud-label-sync google-cloud-label-sync bot added the api: spanner Issues related to the googleapis/nodejs-spanner API. label Jan 31, 2020
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/nodejs-spanner API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

4 participants