[Django] #30601: ATOMIC_REQUESTS=True and caching might be dangerous

[Django]

#30601: ATOMIC_REQUESTS=True and caching might be dangerous
-------------------------------------+-------------------------------------
Reporter: | Owner: nobody
sebastian-philipp |
Type: | Status: new
Uncategorized |
Component: Database | Version: 2.2
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
ATOMIC_REQUESTS=True essentially creates nested transactions. And nested
transactions might not play well with custom caching.

A small example of how to use nested transactions and caching to get
inconsistent data:

{{{
my_mem_cache_a = ...
my_mem_cache_b = ...

def set_a(x):
with transaction.atomic():
a = ...
a.x = x
a.save()
my_mem_cache_a = x

def set_a_b(x, y):
with transaction.atomic():
set_a(x)
b = ...
b.y = y
raise Exception()
# at this point, my_mem_cache_a is out of sync with the database.
b.save()
my_mem_cache_b = x

set_a_b(...)
}}}

(this example also works without transaction.atomic() )

Would be nice to add a warning to

https://docs.djangoproject.com/en/2.2/topics/db/transactions/#tying-
transactions-to-http-requests

that ATOMIC_REQUESTS=True might be harmful, if code

* uses caching
* proactively updates data in the cache
* makes the assumption that values are updated in the database after a
transaction is finished.

--
Ticket URL: <https://code.djangoproject.com/ticket/30601>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#30601: Extend documentation about transaction rollbacks to mention global state
mutations such as caching
--------------------------------------+------------------------------------
Reporter: sebastian-philipp | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 2.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 1

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

--------------------------------------+------------------------------------
Changes (by Simon Charette):

* component: Database layer (models, ORM) => Documentation
* needs_docs: 0 => 1
* type: Uncategorized => Cleanup/optimization
* stage: Unreviewed => Accepted

Comment:

I'm not sure how related to `ATOMIC_REQUEST` this issue actually is, it
seems like a common pitfall of mixing database transactions and caching
and a good reason to rely on `transaction.on_commit` to defer cache
alterations instead.

The `atomic` documentation already mentions that model state should be
reverted (#28479) so maybe we could adjust this block to mention that all
form of state, including global one such as caching, should be reverted
and that using `transaction.on_commit` can be used to deal with global
state alterations.

Tentatively accepting based on the premise you might be interested in
submitting a patch yourself.

--
Ticket URL: <https://code.djangoproject.com/ticket/30601#comment:1>

[Django]

#30601: Extend documentation about transaction rollbacks to mention global state
mutations such as caching

-------------------------------------+-------------------------------------
Reporter: Sebastian Wagner | Owner:
Type: | angelicaferlin
Cleanup/optimization | Status: assigned

Component: Documentation | Version: 2.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 1

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by angelicaferlin):

* owner: nobody => angelicaferlin
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/30601#comment:2>

[Django]

#30601: Extend documentation about transaction rollbacks to mention global state
mutations such as caching
-------------------------------------+-------------------------------------
Reporter: Sebastian Wagner | Owner:
Type: | angelicaferlin
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 2.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 1

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by angelicaferlin):

* has_patch: 0 => 1

Comment:

Link to PR: https://github.com/django/django/pull/16848

--
Ticket URL: <https://code.djangoproject.com/ticket/30601#comment:3>

[Django]

#30601: Extend documentation about transaction rollbacks to mention global state
mutations such as caching

--------------------------------------+------------------------------------
Reporter: Sebastian Wagner | Owner: (none)
Type: Cleanup/optimization | Status: new

Component: Documentation | Version: 2.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

--------------------------------------+------------------------------------
Changes (by Angelica Ferlin):

* owner: Angelica Ferlin => (none)
* status: assigned => new

--
Ticket URL: <https://code.djangoproject.com/ticket/30601#comment:4>

[Django]

#30601: Extend documentation about transaction rollbacks to mention global state
mutations such as caching
--------------------------------------+------------------------------------
Reporter: Sebastian Wagner | Owner: (none)
Type: Cleanup/optimization | Status: new

Component: Documentation | Version: dev

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

--------------------------------------+------------------------------------
Changes (by Natalia Bidart):

* needs_docs: 1 => 0
* has_patch: 1 => 0
* version: 2.2 => dev

--
Ticket URL: <https://code.djangoproject.com/ticket/30601#comment:5>

[Django]

#30601: Extend documentation about transaction rollbacks to mention global state
mutations such as caching

-------------------------------------+-------------------------------------
Reporter: Sebastian Wagner | Owner: Lufafa
Type: | Joshua
Cleanup/optimization | Status: assigned

Component: Documentation | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Lufafa Joshua):

* owner: (none) => Lufafa Joshua

* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/30601#comment:6>

[Django]

#30601: Extend documentation about transaction rollbacks to mention global state
mutations such as caching
-------------------------------------+-------------------------------------
Reporter: Sebastian Wagner | Owner: Lufafa
Type: | Joshua
Cleanup/optimization | Status: assigned
Component: Documentation | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Lufafa Joshua):

Link to PR: https://github.com/django/django/pull/17413

--
Ticket URL: <https://code.djangoproject.com/ticket/30601#comment:7>

[Django]

#30601: Extend documentation about transaction rollbacks to mention global state
mutations such as caching
-------------------------------------+-------------------------------------
Reporter: Sebastian Wagner | Owner: Lufafa
Type: | Joshua
Cleanup/optimization | Status: assigned
Component: Documentation | Version: dev
Severity: Normal | Resolution:

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Natalia Bidart):

* has_patch: 0 => 1
* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/30601#comment:8>

[Django]

#30601: Extend documentation about transaction rollbacks to mention global state
mutations such as caching
-------------------------------------+-------------------------------------
Reporter: Sebastian Wagner | Owner: Lufafa
Type: | Joshua

Cleanup/optimization | Status: closed
Component: Documentation | Version: dev
Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Natalia <124304+nessita@…>):

* status: assigned => closed
* resolution: => fixed

Comment:

In [changeset:"aa80b357fbef46e5b6faa08d63bcfd4fe21f3776" aa80b357]:
{{{
#!CommitTicketReference repository=""
revision="aa80b357fbef46e5b6faa08d63bcfd4fe21f3776"
Fixed #30601 -- Doc'd the need to manually revert all app state on
transaction rollbacks.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/30601#comment:9>

[Django]

#30601: Extend documentation about transaction rollbacks to mention global state
mutations such as caching
-------------------------------------+-------------------------------------
Reporter: Sebastian Wagner | Owner: Lufafa
Type: | Joshua
Cleanup/optimization | Status: closed
Component: Documentation | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Natalia <124304+nessita@…>):

In [changeset:"c8ac50c201ecacaa93f4b638a032f04e731acd03" c8ac50c]:
{{{
#!CommitTicketReference repository=""
revision="c8ac50c201ecacaa93f4b638a032f04e731acd03"
[5.0.x] Fixed #30601 -- Doc'd the need to manually revert all app state on
transaction rollbacks.

Backport of aa80b357fbef46e5b6faa08d63bcfd4fe21f3776 from main
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/30601#comment:10>

[Django]

#30601: Extend documentation about transaction rollbacks to mention global state
mutations such as caching
-------------------------------------+-------------------------------------
Reporter: Sebastian Wagner | Owner: Lufafa
Type: | Joshua
Cleanup/optimization | Status: closed
Component: Documentation | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Natalia <124304+nessita@…>):

In [changeset:"696fbc32d6185283859b3a84277f49937a3012fc" 696fbc3]:
{{{
#!CommitTicketReference repository=""
revision="696fbc32d6185283859b3a84277f49937a3012fc"
[4.2.x] Fixed #30601 -- Doc'd the need to manually revert all app state on
transaction rollbacks.

Backport of aa80b357fbef46e5b6faa08d63bcfd4fe21f3776 from main
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/30601#comment:11>

[Django]

#30601: Extend documentation about transaction rollbacks to mention global state
mutations such as caching
-------------------------------------+-------------------------------------
Reporter: Sebastian Wagner | Owner: Lufafa
Type: | Joshua
Cleanup/optimization | Status: closed
Component: Documentation | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Natalia <124304+nessita@…>):

In [changeset:"9b18af4f6f12b9d25157e0b5afc3dca198f6dd06" 9b18af4f]:
{{{
#!CommitTicketReference repository=""
revision="9b18af4f6f12b9d25157e0b5afc3dca198f6dd06"
Refs #30601 -- Fixed typos in docs/topics/db/transactions.txt.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/30601#comment:12>

[Django]

#30601: Extend documentation about transaction rollbacks to mention global state
mutations such as caching
-------------------------------------+-------------------------------------
Reporter: Sebastian Wagner | Owner: Lufafa
Type: | Joshua
Cleanup/optimization | Status: closed
Component: Documentation | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Natalia <124304+nessita@…>):

In [changeset:"acd4595ab0c9bbe852fda348fe0b10376cbafdbd" acd4595]:
{{{
#!CommitTicketReference repository=""
revision="acd4595ab0c9bbe852fda348fe0b10376cbafdbd"
[5.0.x] Refs #30601 -- Fixed typos in docs/topics/db/transactions.txt.

Backport of 9b18af4f6f12b9d25157e0b5afc3dca198f6dd06 from main
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/30601#comment:13>

[Django]

#30601: Extend documentation about transaction rollbacks to mention global state
mutations such as caching
-------------------------------------+-------------------------------------
Reporter: Sebastian Wagner | Owner: Lufafa
Type: | Joshua
Cleanup/optimization | Status: closed
Component: Documentation | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Natalia <124304+nessita@…>):

In [changeset:"3fae5d92da84382483fe48cc62c1f57789b47196" 3fae5d9]:
{{{
#!CommitTicketReference repository=""
revision="3fae5d92da84382483fe48cc62c1f57789b47196"
[4.2.x] Refs #30601 -- Fixed typos in docs/topics/db/transactions.txt.

Backport of 9b18af4f6f12b9d25157e0b5afc3dca198f6dd06 from main
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/30601#comment:14>