codeconsole commented on PR #16415: URL: https://github.com/apache/grails-core/pull/16415#issuecomment-5850577416
Thanks for the careful review and the full runs. All of it is addressed in 8ca8ba2c1a. **1. A flush inside a read-only scope.** Agreed. `transactions.adoc` and `upgrading80x.adoc` now lead with calling the method from read-write code or giving it `PROPAGATION_REQUIRES_NEW`, and say that a write flushed there is saved outside any transaction, so a later failure cannot roll it back. The new `MongoTransactionSpec` feature "a write flushed in a read-write transaction inside a read-only one is not part of any transaction" pins it: the read-only outer throws after the inner flush, and the write stays. **2. Commit and rollback on the recorded session.** `doCommit`, `doRollback` and cleanup now all use the session the transaction began on, falling back to the holder's current session when none was recorded. A new `DatastoreTransactionManagerSpec` feature binds a second session on top and checks that the commit flushes, and the rollback clears, the transaction's own session. **3. Neo4j's `doBegin`.** It was a copy of the base one from when the base did not pass the definition to `beginTransaction`. Now that it does, I removed the copy, so Neo4j takes the base failure path (flush mode put back, rollback on a pre-bound session), and the "set to NEVER" comment goes with it. `grails-data-neo4j-core`: 592 tests, 99 skipped as before, no failures. **4. Minor** - `transactionSessions` is now a synchronized set, matching the deque. - I dropped the `validateExistingTransaction` sentence from the guide rather than document a `BeanPostProcessor`; the class javadoc still names the property. - The protected `doSuspend`/`doResume` calls in the spec are left as they are. - Squash message: I haven't rewritten the branch. A suggestion describing the final behaviour is below. **Release notes.** The GORM for MongoDB upgrade notes have a new entry, "A Transaction Started Inside Another Joins It", second after the `ObjectId` change. Its main point is the one you raised, with an example: a joined call's unflushed writes are not seen by later queries in the same transaction, including in `@Rollback` tests. Suggested squash message: ``` Join a transaction started inside another, as Hibernate does DatastoreTransactionManager never overrode isExistingTransaction, so a nested PROPAGATION_REQUIRED call began a second transaction on the same session and spent it. The outer commit then did nothing: on MongoDB the outer transaction's later writes were lost, and with multi-document transactions its earlier ones too. Fixes #14391. A transaction in progress on the session the thread is using is now joined, and a read-write transaction inside a read-only one joins it and is read-only, as on Hibernate. REQUIRES_NEW suspends the surrounding transaction and runs in a session of its own. A completed transaction cannot be joined, so afterCommit code begins its own. Commit and rollback complete the transaction begun for them, on the session it began on, and a commit whose transaction is no longer active throws. Cleanup resets rollback-only and puts back the flush mode a read-only transaction changed. On MongoDB, beginning a transaction over an active server-side one throws, and a read-only transaction reads without one. The Spring Data unified manager suspends Spring Data's session holder with GORM's. ``` Re-run for this commit: `grails-datastore-core` (296), `grails-data-neo4j-core` (592), the MongoDB transaction specs and the TCK transaction specs on MongoDB, all passing, with `codeStyle` clean for the three modules. -- 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]
