Repository navigation
Use try-with-resources for Transactions #49
Description
Activity
Yes, the reason that we didn't make Transaction Closable is because close does not have enough context (as oppose to Python with clause) about success/failure of the operation.
I think having the need to explicitly commit but rather automatically rollback (if not committed) is not intuitive/expected and I would like to get some more feedback/opinions before going ahead with it.
For now we provided the DatastoreService#runInTransaction convenient method.I agree with Arie that it would be surprising for Transaction.close() to mean "abort the transaction if it is not committed". What you would want instead would be for it to mean "commit the transaction unless there was an exception, in which case abort it". But unfortunately there's no way for the close() method to know whether it is being invoked because of an exception or because execution of the
tryblock completed normally.I think I would be in favour of having only the
DatastoreService.runInTransactionmethod so that it is impossible to accidentally abandon a transaction due to an exception. The argument toTransactionCallablewould then need to be aTransaction, which is probably better than aDatastoreReaderWriteranyway? The resulting code would be slightly more verbose on Java 7 or before, but quite succinct on Java 8 with lambdas:result = dataStore.runInTransaction((t) -> { t.query()...t.put()...; return something; });
I agree that passing
Transactioninstead ofDatastoreReaderWritertoDatastoreService.runInTransactionsounds nicer but it has the side effect of also including a way
to manually/explicitlycommitorrollback(the main 2 methods thatTransactionadds toDatastoreReaderWriter).Though having an explicit commit/rollback option in the callback could be seen as advantage/feature, I feel that it may lead to more confusing user code (where the callback code can't tell for sure if it is still active) and thought we should better avoid it.
Few possible options:
- Leave it as is.
- Replace callback param with a new
TransactionCallbackinterface (similar to Transaction but without a way to explicitly commit/rollback). - Replace callback param with Transaction (and allow callback to explicitly commit/rollback).
Thoughts?
You need transactions to fail safe i.e. roll back by default. The user must specifically commit the work and know that the commit succeeded so that other work can rely on it. The nasty path is where the VM itself raises an Error (which can happen at any time) which must result the work getting rolled back.
The simplest way to ensure that behaviour is to have the user explicitly commit their work. The user can then assume that any error that prevented the commit operation completing successfully resulted in no work being applied.
For reference, where the JDBC spec says it is "implementation-defined" (doh!) whether Connection#close() commits or rolls back, the drivers from MySQL, Postgres, DB2 (non-mainframe) and SQLServer all roll back by default whereas only Oracle and DB2 (mainframe) commit. Similarly a UserTransaction in JavaEE will always rollback unless it is specifically committed.
Based on @eamonnmcmanus comment, Java 8's lambdas would make this intuitive:
interface DataStore { /** Simple way to run work in a transaction. */ <ResultT> ResultT runInTransaction(Supplier<ResultT> work); /** Run work in a transaction with ability to manually control outcome. */ <ResultT> ResultT runInTransaction(Function<Transaction, ResultT> work); }
The
Transactioninterface would only need to add one method for transaction control:interface Transaction extends DatastoreReaderWriter { void setRollbackOnly(); }
which would would like the same method in
javax.transaction.UserTransactionWork would be automatically committed if the function returned normally, or would be rolled back if the block threw a Throwable. Per Java8 functional APIs, the work would not be able to throw a checked Exception.
To run this on Java 7, we can backport the Supplier and Function interfaces. I realize equivalents exist in Guava but I suggest we avoid using Guava classes in the API.
- addedapi: datastoreIssues related to the Datastore API.Issues related to the Datastore API.type: feature request‘Nice-to-have’ improvement, new feature or different behavior or design.‘Nice-to-have’ improvement, new feature or different behavior or design.
on Nov 23, 2016 - addedpriority: p2Moderately-important priority. Fix may not be included in next release.Moderately-important priority. Fix may not be included in next release.and removed
on Jul 18, 2017 garrettjonesgoogle commented
on Aug 11, 2017 ContributorMore actionsThis has been added to our feature backlog: https://github.com/GoogleCloudPlatform/google-cloud-java/wiki/Feature-backlog . This issue will be closed but is linked in the backlog and can continue to be used for comment and discussion.
5 remaining items
- added a commit that references this issue
on Jul 14, 2022 - added a commit that references this issue
on Sep 15, 2022 - added 5 commits that reference this issue
on Oct 4, 2022 - added a commit that references this issue
on Jan 6, 2026 - added a commit that references this issue
on Jan 22, 2026 - added a commit that references this issue
on Mar 30, 2026 - added a commit that references this issue
on Apr 1, 2026
It would be nice to be able to use the resource pattern around a Transaction:
Default behaviour of close() would be to roll back a transaction. This is to avoid potential errors caused by the application failing to catch unexpected Throwables causing incomplete work to be auto-committed.