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

Unable to rollback transaction with no activity or with only queued mutations #2103

Open
BobDickinson opened this issue Sep 9, 2024 · 0 comments
Labels
api: spanner Issues related to the googleapis/nodejs-spanner API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@BobDickinson
Copy link

Environment details

  • OS: MacOS Sonomia 14.6.1
  • Node.js version: 20.9.0
  • npm version: 10.1.0
  • @google-cloud/spanner version: 7.14.0

Steps to reproduce

  1. Create a transaction - I use getTransaction()
  2. Attempt transaction.rollback()

Or alternatively, execute any number of insert, upsert, delete, etc operations on the transaction and then attempt rollback().

When attempting to rollback() a transaction that hasn't had any activity, or that has only had queued mutation activity, calling rollback() on that transaction throws an error: 'Transaction ID is unknown, nothing to rollback.'

From here:

'Transaction ID is unknown, nothing to rollback.'

Expected behavior

Any transaction in a state where it can be successfully committed should also be able to be rolled back without error.

The notion of the transaction id being set when the transaction has actually begun with Spanner (and there is something to roll back), and the idea of queued mutations, are internal to the library (not explained anywhere in the docs that I could find), so requiring the user to understand that they cannot call rollback() under these certain (only known to the library authors) conditions does not seem correct.

The workaround is pretty straightforward, but required me to spend a couple of hours with the library source code to ensure that it was correct (essentially, if id is not set, call end() instead of rollback() to abort the transaction).

The correct behavior would be for the transaction.rollback() to succeed (not throw an error) in this case (no id set, and potentially outstanding queue mutations) - it's a logically valid usage and the result is a noop, maybe with an internal call to end(), so why you would throw an error in that case is beyond me, especially given that the semantics the SDK user would need to understand to avoid the error are not documented AFAICT.

Just trying to save anyone else the 2 hours I spend on this issue. Thanks.

@BobDickinson BobDickinson added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Sep 9, 2024
@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/nodejs-spanner API. label Sep 9, 2024
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. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

1 participant