[Django] #21171: Skip transaction creation for single statement operations
Django
----------------------------------------------+--------------------
Reporter: akaariai | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Database layer (models, ORM) | Version: 1.5
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
There are a couple of cases where Django will execute just a single data
modifying statement for the operation done. Examples include single table
.save(), .delete() that doesn't cascade, bulk_create() and .update().
Atomicity of single statements guarantees that these operations work
without creating / releasing transaction. This could increase performance
of these statements when used in autocommit mode.
The save() method should be easy to handle - it is easy to check if the
table has parents or not. Bulk create is also somewhat easy, although
there are cases where multiple queries need to be executed. Model deletion
might be hard to handle, signals are sent inside the same transaction and
it isn't that far fetched to think that signal handler will do additional
data modifications (log changes for example).
It might be there are more operations that could benefit from this -
get_or_create() and update_or_create() for example.
I got the idea while inspecting if cache db backend could use ORM
directly. Currently the db backend doesn't wrap all data modifications in
atomic blocks, but if using ORM directly that would happen and this could
lead to performance problems in the backend.
--
Ticket URL: <https://code.djangoproject.com/ticket/21171>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: akaariai | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.5
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by timo):
* needs_better_patch: => 0
* needs_docs: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/21171#comment:1>
Django
Reporter: akaariai | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.5
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by shai):
I must be missing something -- in current master, get_or_create() and
update_or_create() are not single-statement operations.
Also, as far as I see, the signals argument applies to save() just as much
as it applies to delete().
Perhaps add a keyword argument? `subtransaction=True`, so if you want to
save a bit you pass in False?
--
Ticket URL: <https://code.djangoproject.com/ticket/21171#comment:2>
Django
Reporter: akaariai | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.5
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by akaariai):
For save() signals are sent outside the transaction but for delete inside
the transaction. So, for delete, if you skip the transaction creation,
then queries ran from signals aren't inside transaction control. But for
save() this isn't the case - any DML is ran outside transaction in any
case.
The delete() case is a change in behaviour.
You are right about get_or_create() and update_or_create().
All in all, I'm beginning to wonder if this is really worth it...
--
Ticket URL: <https://code.djangoproject.com/ticket/21171#comment:3>
Django
Reporter: Anssi Kääriäinen | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.5
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/10448 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/21171#comment:4>
Django
Reporter: Anssi Kääriäinen | Owner: nobody
Cleanup/optimization |
Component: Database layer | Version: 1.5
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Florian Apolloner <apollo13@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"bc7dd8490b882b2cefdc7faf431dc64c532b79c9" bc7dd849]:
{{{
#!CommitTicketReference repository=""
revision="bc7dd8490b882b2cefdc7faf431dc64c532b79c9"
Fixed #21171 -- Avoided starting a transaction when a single (or atomic
queries) are executed.
Checked the following locations:
* Model.save(): If there are parents involved, take the safe way and use
transactions since this should be an all or nothing operation.
If the model has no parents:
* Signals are executed before and after the previous existing
transaction -- they were never been part of the transaction.
* if `force_insert` is set then only one query is executed -> atomic
by definition and no transaction needed.
* same applies to `force_update`.
* If a primary key is set and no `force_*` is set Django will try an
UPDATE and if that returns zero rows it tries an INSERT. The first
case is completly save (single query). In the second case a
transaction should not produce different results since the update
query is basically a no-op then (might miss something though).
* QuerySet.update(): no signals issued, single query -> no transaction
needed.
* Model/Collector.delete(): This one is fun due to the fact that is
does many things at once.
Most importantly though: It does send signals as part of the
transaction, so for maximum backwards compatibility we need to be
conservative.
To ensure maximum compatibility the transaction here is removed only
if the following holds true:
* A single instance is being deleted.
* There are no signal handlers attached to that instance.
* There are no deletions/updates to cascade.
* There are no parents which also need deletion.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21171#comment:5>
Django
Reporter: Anssi Kääriäinen | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: 1.5
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by GitHub <noreply@…>):
In [changeset:"f8ef5f2c86683bef3b200fd864efc14f1fbbc23b" f8ef5f2c]:
{{{
#!CommitTicketReference repository=""
revision="f8ef5f2c86683bef3b200fd864efc14f1fbbc23b"
Refs #21171 -- Made Collector.delete() rollback in the correct database.
Regression in c7dd8490b882b2cefdc7faf431dc64c532b79c9.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21171#comment:6>
Django
Reporter: Anssi Kääriäinen | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: 1.5
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"aca675ad33c1673a464172182bc66d8cd16efaa5" aca675a]:
{{{
#!CommitTicketReference repository=""
revision="aca675ad33c1673a464172182bc66d8cd16efaa5"
[3.1.x] Refs #21171 -- Made Collector.delete() rollback in the correct
database.
Regression in c7dd8490b882b2cefdc7faf431dc64c532b79c9.
Backport of f8ef5f2c86683bef3b200fd864efc14f1fbbc23b from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21171#comment:7>