Transaction support #2586

alcaeus · 2023-11-24T08:17:26Z

Q	A
Type	feature
BC Break	no
Fixed issues

Summary

This pull request introduces support for transactions. When calling DocumentManager::commit, a transaction will be used depending on a configuration setting. Alternatively, transaction usage can also be changed using a commit option.

Usage

A new useTransactionalFlush configuration option can be enabled when running on the following deployments:

Replica sets starting with MongoDB 4.0
Sharded clusters starting with MongoDB 4.2
Serverless deployments on MongoDB Atlas
Loadbalanced deployments on MongoDB Atlas

To enable this feature globally, enable transactional flushes in the configuration:

$config = new Configuration();
$config->setUseTransactionalFlush(true);
// Other configuration

$dm = DocumentManager::create(null, $config);

Alternatively, the withTransaction commit option can be specified to override this default value and selectively enable or disable transactions:

// To explicitly enable transaction for this write
$dm->flush(['withTransaction' => true]);

// To disable transaction usage for a write, regardless of the ``useTransactionalFlush`` config:
$dm->flush(['withTransaction' => false]);

This pull request has been reviewed in smaller chunks in the following pull requests:

This PR is also accompanied by a pull request in the Symfony Bundle to expose the transactional flush setting in the bundle configuration.

* Add tests for commit consistency showing wrong behaviour * Clear scheduled document changes at the end of a commit operation * Rename private variables to communicate intent * Remove obsolete comment * Use different error code * Use single mongos in consistency tests This ensures that the fail points are created on the same server that the write operations take place on, which can't be guaranteed in a sharded cluster with multiple mongoses. * Rename test methods for clarity * Explain reasoning for index error * Extract helper method to create failpoint

* Add configuration setting for transactional flush * Use classic setters for transactional flush setting

* Extract commit logic * Support transactional commit operations * Always use transactional flush if supported With this commit, all tests using the document manager use transactional flush as long as transactions are supported. Certain tests can use the static $allowsTransactions variable to disable this behaviour. * Test with MongoDB 7.0 * Update test names * Update phpstan baseline * Fix query selection in shard key tests * Flip transaction options constant by default * Use supportsTransaction method when skipping tests * Apply review feedback to tests * Add separate test to check write concern in commit options * Strip write options when in transaction * Use majority write concern in test

* Pass session and transaction information to event args * Only dispatch lifecycle events once per commit operation * Remove isInTransaction property in event args * Split method signature for readability * Use property promotion for event args classes * Extract construction of eventArgs * Inline spl_object_hash calls * Avoid injecting test instance * Add session to commitOptions in persister * Add session assertions in LifecycleEventManager

* Only retry transaction once * Rename variable

* Remove references to transactions where not applicable * Update transaction documentation * Apply suggestions from code review Co-authored-by: Jeremy Mikola <jmikola@gmail.com> * Change title level in event documentation * Add documentation about transactions to event docs --------- Co-authored-by: Jeremy Mikola <jmikola@gmail.com>

.github/workflows/coding-standards.yml

.github/workflows/continuous-integration.yml

lib/Doctrine/ODM/MongoDB/Configuration.php

lib/Doctrine/ODM/MongoDB/UnitOfWork.php

lib/Doctrine/ODM/MongoDB/Utility/LifecycleEventManager.php

jmikola · 2024-01-17T16:40:19Z

tests/Doctrine/ODM/MongoDB/Tests/BaseTestCase.php

+     * Removes any hosts beyond the first in a URI. This function should only be
+     * used with a sharded cluster URI, but that is not enforced.
+     */
+    protected static function removeMultipleHosts(string $uri): string


This is technically also lifted from PHPLIB but I have no qualms about copying it here. 🤫

jmikola · 2024-01-17T16:43:02Z

tests/Doctrine/ODM/MongoDB/Tests/BaseTestCase.php

+    {
+        $manager = new Manager(self::getUri());
+
+        return $manager->selectServer()->getType() !== Server::TYPE_STANDALONE;


Remind me if we discussed this before, but transactions require either a 4.0+ RS or 4.2+ sharded cluster. Since ODM only tests as low as 4.4, we don't need to concern ourselves with a version check.

We didn't discuss this before, so good point bringing it up. We haven't discussed version support in ODM in a while, but I'd say we can ignore anything < 4.4.

We already do

mongodb-odm/.github/workflows/continuous-integration.yml

Lines 23 to 26 in c9b7f6b

mongodb-version:

- "6.0"

- "5.0"

- "4.4"

I meant for the purpose of skipping other topologies. In the library, we check server versions for replica sets and sharded clusters to ensure we only test on supported platforms. This isn't necessary here.

malarzm

I admit I've skimmed through as I tried to do all partial PRs, I did not catch more than @jmikola's eagle eye already did ;)

jmikola

LGTM, but note the Psalm failures.

jmikola · 2024-01-19T14:05:41Z

lib/Doctrine/ODM/MongoDB/UnitOfWork.php

@@ -1268,11 +1273,7 @@ private function executeDeletions(ClassMetadata $class, array $documents, array
                $value->clearSnapshot();
            }

-            // Document with this $oid after deletion treated as NEW, even if the $oid
-            // is obtained by a new document because the old one went out of scope.
-            $this->documentStates[$oid] = self::STATE_NEW;


Was just curious about this but traced the explanation back to https://github.com/doctrine/mongodb-odm/pull/2580/files#r1386613660

alcaeus · 2024-01-19T14:20:02Z

note the Psalm failures.

Thanks for pointing those out. I didn't have those locally, until I realised that I was running on Psalm 5.17. 5.20 has these errors, but they're unrelated to this code so I've skipped them via the baseline.

* 2.7.x: Bump ramsey/composer-install from 2 to 3 Bump actions/upload-artifact from 3 to 4 (#2601) Bump actions/cache from 3 to 4 (#2609) Transaction support (#2586) make doctrine/annotations dependency optional Decouple AttributeDriver from AnnotationDriver (#2502) Require doctrine/persistence 3 (#2603) Bump doctrine/.github from 3.1.0 to 4.0.0 Remove BC break issue type

alcaeus added the Feature label Nov 24, 2023

alcaeus added this to the 2.7.0 milestone Nov 24, 2023

alcaeus self-assigned this Nov 24, 2023

alcaeus mentioned this pull request Dec 14, 2023

Expose transaction setting doctrine/DoctrineMongoDBBundle#814

Merged

alcaeus force-pushed the feature/transactions branch 2 times, most recently from 18a61c2 to e728816 Compare December 19, 2023 13:22

alcaeus and others added 4 commits January 8, 2024 14:21

Run CI workflows on feature branches

95cf7df

Add configuration setting for transactional flush (#2587)

f02bf21

* Add configuration setting for transactional flush * Use classic setters for transactional flush setting

alcaeus force-pushed the feature/transactions branch from e728816 to 601714f Compare January 8, 2024 13:22

alcaeus and others added 3 commits January 8, 2024 15:36

Only retry transaction once (#2604)

1fc212a

* Only retry transaction once * Rename variable

alcaeus marked this pull request as ready for review January 17, 2024 08:07

alcaeus requested review from jmikola, GromNaN and malarzm January 17, 2024 08:07

GromNaN approved these changes Jan 17, 2024

View reviewed changes

.github/workflows/coding-standards.yml Show resolved Hide resolved

jmikola reviewed Jan 17, 2024

View reviewed changes

.github/workflows/continuous-integration.yml Outdated Show resolved Hide resolved

jmikola reviewed Jan 17, 2024

View reviewed changes

lib/Doctrine/ODM/MongoDB/Configuration.php Outdated Show resolved Hide resolved

jmikola reviewed Jan 17, 2024

View reviewed changes

malarzm approved these changes Jan 17, 2024

View reviewed changes

alcaeus added 2 commits January 19, 2024 09:58

Address code review items

f44a3ed

Test highest dependencies on MongoDB 7.0

ef5e8f0

alcaeus force-pushed the feature/transactions branch from 3323e50 to ef5e8f0 Compare January 19, 2024 09:00

alcaeus requested a review from jmikola January 19, 2024 09:00

jmikola approved these changes Jan 19, 2024

View reviewed changes

Regenerate psalm baseline to silence unrelated errors

6d655ea

alcaeus merged commit d688161 into 2.7.x Jan 19, 2024
40 checks passed

alcaeus deleted the feature/transactions branch January 19, 2024 14:33

alcaeus mentioned this pull request Feb 27, 2024

Transaction support #1888

Closed

alcaeus linked an issue Feb 27, 2024 that may be closed by this pull request

Transaction support #1888

Closed

franmomu mentioned this pull request Mar 19, 2024

Run tests without doctrine/annotations installed doctrine-extensions/DoctrineExtensions#2781

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Transaction support #2586

Transaction support #2586

alcaeus commented Nov 24, 2023 •

edited

Loading

jmikola Jan 17, 2024

jmikola Jan 17, 2024

alcaeus Jan 19, 2024

malarzm Jan 19, 2024 •

edited

Loading

alcaeus Jan 19, 2024

malarzm left a comment

jmikola left a comment

jmikola Jan 19, 2024

alcaeus commented Jan 19, 2024

Transaction support #2586

Transaction support #2586

Conversation

alcaeus commented Nov 24, 2023 • edited Loading

Summary

Usage

jmikola Jan 17, 2024

Choose a reason for hiding this comment

jmikola Jan 17, 2024

Choose a reason for hiding this comment

alcaeus Jan 19, 2024

Choose a reason for hiding this comment

malarzm Jan 19, 2024 • edited Loading

Choose a reason for hiding this comment

alcaeus Jan 19, 2024

Choose a reason for hiding this comment

malarzm left a comment

Choose a reason for hiding this comment

jmikola left a comment

Choose a reason for hiding this comment

jmikola Jan 19, 2024

Choose a reason for hiding this comment

alcaeus commented Jan 19, 2024

alcaeus commented Nov 24, 2023 •

edited

Loading

malarzm Jan 19, 2024 •

edited

Loading