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

Refresh docs about transactions #5379

Merged
merged 4 commits into from
May 6, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 90 additions & 23 deletions docs/en/reference/transactions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,23 @@ is functionally equivalent to the previous one:
::

<?php
$conn->transactional(function($conn) {
$conn->transactional(function(Connection $conn): void {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The closure does not have to be a void, right? Isn't the returned value passed through by transactional()? Should we mention that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right sorry, I misread the code

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess since there's no return statement in this closure and the return value isn't used anywhere, it's void. Maybe it makes sense to have two examples: one with a return value and one without (both are valid).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a note paraphrasing what @derrabus said, not sure if you saw it before writing that comment and if you think two examples are better, in which case I'll be happy to oblige 🙂

Copy link
Member

@morozov morozov May 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I started writing my comment before you addressed his comment but I submitted it after I saw your update. I still believe that two examples are better than just an explanation, they complement each other, not exclude.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@derrabus Does this resolve your question/suggestion?

// do stuff
});

The ``Doctrine\DBAL\Connection`` also has methods to control the
Note that the closure above doesn't have to be a void, anything it
returns will be returned by ``transactional()``:

::

<?php
$one = $conn->transactional(function(Connection $conn): int {
// do stuff
return $conn->fetchOne('SELECT 1');
});


The ``Doctrine\DBAL\Connection`` class also has methods to control the
transaction isolation level as supported by the underlying
database. ``Connection#setTransactionIsolation($level)`` and
``Connection#getTransactionIsolation()`` can be used for that purpose.
Expand All @@ -48,27 +60,45 @@ constants:
TransactionIsolationLevel::SERIALIZABLE

The default transaction isolation level of a
``Doctrine\DBAL\Connection`` is chosen by the underlying platform
but it is always at least ``READ_COMMITTED``.
``Doctrine\DBAL\Connection`` instance is chosen by the underlying
platform but it is always at least ``READ_COMMITTED``.

Transaction Nesting
-------------------

A ``Doctrine\DBAL\Connection`` also adds support for nesting
transactions, or rather propagating transaction control up the call
stack. For that purpose, the ``Connection`` class keeps an internal
counter that represents the nesting level and is
Calling ``beginTransaction()`` while already in a transaction will
result in two very different behaviors depending on whether transaction
nesting with savepoints is enabled or not. In both cases though, there
won't be an actual transaction inside a transaction, even if your RDBMS
supports it. There is always only a single, real database transaction.

By default, transaction nesting at the SQL level with savepoints is
disabled. The value for that setting can be set on a per-connection
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why this is the default by the way, if someone does read the docs and nests transaction, I think they would be less surprised when seeing a savepoint than when seeing a transaction. By the way, it was a long time ago but I think I remember seeing savepoint in my logs, but I don't remember configuring them. Maybe there is (was?) a default to true in some other layer (I've checked the DoctrineBundle and the symfony recipes, and that does not seem to be the case).

Copy link
Member

@morozov morozov May 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think from the perspective of the API semantics, the default behavior is potentially harmful. If a given component starts and commits its transaction, it may expect that it's committed but it may be not depending on the configuration and the current nesting level. The same is about rollback. The more I think about how it works, the less sense it makes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do I sense an incoming deprecation? DB2 does not support savepoints, so that would mean DB2 users would no longer have the option of having nested transactions, and this documentation block should be restored just for them:

This also means that you cannot
successfully commit some changes in an outer transaction if an
inner transaction block fails and issues a rollback, even if this
would be the desired behavior (i.e. because the nested operation is
"optional" for the purpose of the outer transaction block). To
achieve that, you need to restructure your application logic so as
to avoid nesting transaction blocks. If this is not possible
because the nested transaction blocks are in a third-party API
you're out of luck.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default behavior should be definitely deprecated. If a platform doesn't support savepoints and there is an attempt to start a nested transaction, it should fail immediately.

The documentation block looks reasonable but it could use some cleanup in terms of language. Instead of focusing on the user ("you cannot [...] commit", "you're out of luck"), focus on the problems and the API.

Copy link
Member Author

@greg0ire greg0ire May 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do! (on 3.4.x)

basis, with
``Doctrine\DBAL\Connection#setNestTransactionsWithSavepoints()``.

Dummy mode
~~~~~~~~~~

When transaction nesting with savepoints is disabled, what happens is
not so much transaction nesting as propagating transaction control up
the call stack. For that purpose, the ``Connection`` class keeps an
internal counter that represents the nesting level and is
increased/decreased as ``beginTransaction()``, ``commit()`` and
``rollBack()`` are invoked. ``beginTransaction()`` increases the
nesting level whilst
``commit()`` and ``rollBack()`` decrease the nesting level. The nesting level starts at 0. Whenever the nesting level transitions from 0 to 1, ``beginTransaction()`` is invoked on the underlying driver connection and whenever the nesting level transitions from 1 to 0, ``commit()`` or ``rollBack()`` is invoked on the underlying driver, depending on whether the transition was caused by ``Connection#commit()`` or ``Connection#rollBack()``.
``rollBack()`` are invoked. ``beginTransaction()`` increases the nesting
level whilst ``commit()`` and ``rollBack()`` decrease the nesting level.
The nesting level starts at 0.
Whenever the nesting level transitions from 0 to 1,
``beginTransaction()`` is invoked on the underlying driver connection
and whenever the nesting level transitions from 1 to 0, ``commit()`` or
``rollBack()`` is invoked on the underlying driver, depending on whether
the transition was caused by ``Connection#commit()`` or
``Connection#rollBack()``.

What this means is that transaction control is basically passed to
code higher up in the call stack and the inner transaction block is
ignored, with one important exception that is described further
below. Do not confuse this with "real" nested transactions or
savepoints. These are not supported by Doctrine. There is always
only a single, real database transaction.
code higher up in the call stack and the inner transaction block does
not actually result in an SQL transaction. It is not ignored either
though.

To visualize what this means in practice, consider the following
example:
Expand Down Expand Up @@ -102,22 +132,20 @@ example:
throw $e;
}

However,
**a rollback in a nested transaction block will always mark the current transaction so that the only possible outcome of the transaction is to be rolled back**.
However, **a rollback in a nested transaction block will always mark the
current transaction so that the only possible outcome of the transaction
is to be rolled back**.
That means in the above example, the rollback in the inner
transaction block marks the whole transaction for rollback only.
Even if the nested transaction block would not rethrow the
exception, the transaction is marked for rollback only and the
commit of the outer transaction would trigger an exception, leading
to the final rollback. This also means that you can not
to the final rollback. This also means that you cannot
successfully commit some changes in an outer transaction if an
inner transaction block fails and issues a rollback, even if this
would be the desired behavior (i.e. because the nested operation is
"optional" for the purpose of the outer transaction block). To
achieve that, you need to restructure your application logic so as
to avoid nesting transaction blocks. If this is not possible
because the nested transaction blocks are in a third-party API
you're out of luck.
achieve that, you need to resort to transaction nesting with savepoint.

All that is guaranteed to the inner transaction is that it still
happens atomically, all or nothing, the transaction just gets a
Expand All @@ -142,6 +170,45 @@ wider scope and the control is handed to the outer scope.
nesting level, causing errors with broken transaction boundaries
that may be hard to debug.

Emulated Transaction Nesting with Savepoints
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Let's now examine what happens when transaction nesting with savepoints
is enabled, with the same example as above

::

<?php
// $conn instanceof Doctrine\DBAL\Connection
$conn->beginTransaction(); // 0 => 1, "real" transaction started
try {

...

// nested transaction block, this might be in some other API/library code that is
// unaware of the outer transaction.
$conn->beginTransaction(); // 1 => 2, savepoint created
try {
...

$conn->commit(); // 2 => 1
} catch (\Exception $e) {
$conn->rollBack(); // 2 => 1, rollback to savepoint
throw $e;
}

...

$conn->commit(); // 1 => 0, "real" transaction committed
} catch (\Exception $e) {
$conn->rollBack(); // 1 => 0, "real" transaction rollback
throw $e;
}

This time, everything is handled at the SQL level: the main transaction
is not marked for rollback only, but the inner emulated transaction is
rolled back to the savepoint.

Auto-commit mode
----------------

Expand Down