codeconsole commented on code in PR #16415:
URL: https://github.com/apache/grails-core/pull/16415#discussion_r4123656671
##########
grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/transactions/DatastoreTransactionManager.java:
##########
@@ -224,17 +280,52 @@ protected void doSetRollbackOnly(DefaultTransactionStatus
status) throws Transac
TransactionObject txObject = (TransactionObject)
status.getTransaction();
status.setRollbackOnly();
txObject.getSessionHolder().setRollbackOnly();
+ // A joined transaction failed, so the whole transaction will roll
back. Discard what is queued
+ // now: flushed explicitly before then, it would be written, and
without a server-side
+ // transaction nothing could take it back.
+ Session session = txObject.getSessionHolder().getSession();
+ if (session != null) {
+ session.clear();
+ }
+ }
+
+ /**
+ * A committed or rolled back transaction can no longer be joined: code
run from its
+ * {@code afterCommit} or {@code afterCompletion} callbacks begins one of
its own, rather than joining
+ * a transaction that will not commit again and losing its writes.
+ */
+ private static void ended(TransactionObject txObject) {
+
txObject.getSessionHolder().setTransactionActive(txObject.getTransactionSession(),
false);
+ }
+
+ /**
+ * The session the transaction began on, which its commit flushes and its
rollback clears, even if
+ * another has been bound on top of it since; the holder's current session
for a transaction begun
+ * by a subclass that does not record one.
+ */
+ private static Session transactionSession(TransactionObject txObject) {
+ Session session = txObject.getTransactionSession();
+ return session != null ? session :
txObject.getSessionHolder().getSession();
}
@Override
protected void doCleanupAfterCompletion(Object transaction) {
TransactionObject txObject = (TransactionObject) transaction;
+ SessionHolder sessionHolder = txObject.getSessionHolder();
- // Un-bind the session holder from the thread.
- if (txObject.isNewSessionHolder()) {
-
DatastoreUtils.closeSession(txObject.getSessionHolder().getSession());
+ ended(txObject);
+ // A joined transaction that failed marked the holder rollback-only. A
pre-bound holder outlives
+ // this transaction, and the next one begun on it must not inherit
that mark.
+ sessionHolder.resetRollbackOnly();
Review Comment:
Reproduced as you describe: at `5f98897e8a` the TCK scenario committed `C`
on the simple datastore. Fixed in 3cd24a38ac the way you suggested.
`SessionHolder` now keeps the mark per session (`setRollbackOnly(Session)`,
`isRollbackOnly(Session)`, `resetRollbackOnly(Session)`). `doSetRollbackOnly`
marks the session the participant joined. `TransactionObject.isRollbackOnly()`
asks about its own transaction's session, which for a participant is the
holder's current session. Cleanup resets only that session. The holder-wide
methods are unchanged, and the manager no longer uses them.
Your scenario is now the sixth `NestedTransactionSpec` feature.
`MongoTransactionSpec` runs it with transactions on, through both
`withTransaction` and `TransactionTemplate`: the inner transaction commits, and
the outer throws `UnexpectedRollbackException`.
`DatastoreTransactionManagerSpec` checks it at the manager level.
All six TCK features now pass on simple, MongoDB, Neo4j, Hibernate 5 and 7,
with nothing pending:
- 141d57cf73: the simple datastore counted identifiers per session, so a
`withNewSession` insert took the identifier of one still queued in the outer
session, and was overwritten when that flushed.
- ff3dc8cef6: on Neo4j, a failed participant rolled back and closed the
transaction on the spot, and the outer code's later writes then ran in
auto-commit, which is how `C` was committed there. The transaction is now only
marked rollback-only.
Testing `withNewSession` also turned up two related bugs in
`GormSharedSessionMongoTransactionManager`, fixed in ffcf25d3a3 and 8244fca127.
A transaction begun in a new session ran its `MongoTemplate` calls in the outer
transaction's `ClientSession`, or, if read-only, still in it. Its cleanup
unbound the outer transaction's Spring Data holder. The manager now sets aside
whatever Spring Data holder is bound when a transaction begins, and puts it
back when that transaction completes.
##########
grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/transactions/TransactionObject.java:
##########
@@ -77,7 +118,7 @@ public boolean isNewSession() {
@Override
public boolean isRollbackOnly() {
Review Comment:
Fixed in d91b55e1c7. `flush()` now uses the transaction's session through
the same accessor as commit and rollback, which now carries the fallback to the
holder's current session. `DatastoreTransactionManagerSpec` covers
`status.flush()` with a session bound on top, and from a joined transaction.
##########
grails-data-mongodb/docs/src/docs/asciidoc/introduction/upgradeNotes.adoc:
##########
@@ -53,6 +53,23 @@ class UserProfile {
See <<idGeneration,Identity Generation>> for the full description of
`storedAs`.
+==== A Transaction Started Inside Another Joins It
+
+A transactional service method called from another, or `withTransaction`
inside `withTransaction`, used to begin a second transaction on the same
session: it committed on its own, and the surrounding transaction's commit then
did nothing, so the surrounding transaction's later writes could be lost. It
now joins the surrounding transaction, and everything commits once when that
one does, as on Hibernate.
+
+The effect most applications will notice: GORM for MongoDB does not flush
before a query while a transaction is in progress, so a write a joined call
leaves queued is not visible to a later query in the same transaction. It used
to be, because the inner transaction flushed it when it returned:
Review Comment:
Done here rather than in an issue (5f65cc0914), since
`grails.mongodb.transactional` is new in 8.0. `MongoQuery.flushBeforeQuery` now
also flushes when the session `hasActiveTransaction()`. A read-only transaction
still doesn't flush: it has no server-side transaction, and its session is in
`COMMIT` mode. `MongoTransactionSpec` checks that a joined call's queued write
is visible to a later query, and that a write flushed before a query rolls back
with the transaction. `MongoTransactionDisabledSpec` checks that without
transactions the query still doesn't see it. This note, `transactions.adoc` and
`upgrading80x.adoc` now limit the "not visible" wording to the
non-transactional setup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
