[Django] #34462: Deletions in admin panel create N + 1 queries

[Django]

#34462: Deletions in admin panel create N + 1 queries
-------------------------------------+-------------------------------------
Reporter: Mike | Owner: nobody
Lissner |
Type: | Status: new
Cleanup/optimization |
Component: | Version: 3.2
contrib.admin |
Severity: Normal | Keywords: performance
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-------------------------------------+-------------------------------------
Sentry has a new feature that identifies when your app has an N+1 query:

https://docs.sentry.io/product/issues/issue-details/performance-issues/n
-one-queries/

(I'll pause here to note my love of Sentry.)

This new feature just identified that when you delete multiple items from
the admin panel, Django does an N+1 query for each of them, so if you
delete 100 items, as I just did, it generates 100 queries to your
database. The queries are small, but this isn't great.

According to Sentry, the queries it generates are of the form:

{{{
INSERT INTO "django_admin_log" ("action_time", "user_id",
"content_type_id", "object_id", "object_repr", "action_flag",
"change_message") VALUES (%s, %s, %s, %s, %s, %s, %s) RETURNING
"django_admin_log"."id"
}}}

I think a bulk_insert would fix this, right?

I don't have time to tackle this, but I thought I'd at least report it. Do
we care?

I can share the Sentry issue with anybody that wants to tackle this. So we
can find it later, it's BIGCASES2-V in our system.

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

[Django]

#34462: Deletions in admin panel create N + 1 queries

--------------------------------------+------------------------------------
Reporter: Mike Lissner | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: performance | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0

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

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

* stage: Unreviewed => Accepted

Comment:

In order to solve this issue we'd have to introduce a new
`ModelAdmin.log_deletions` method as `log_deletion` only allows a single
object to be passed which would require adding a
`LogEntryManager.log_actions` method. If we're going forward with this
change we should also make something for the changelist form view that
relies on `log_addition` and `log_change`. Any bypass of `log_deletion`
and friends for newly introduced bulk logging methods should be documented
in release notes and we should also possibly keep calling into the old
method for a deprecation period if they are overridden in subclasses.

The `delete_selected` action
[https://github.com/django/django/blob/38e63c9e61152682f3ff982c85a73793ab6d3267/django/contrib/admin/actions.py#L46-L57
has another issue that go beyond the one reported here]. It issues a
`count()` query and then issues a follow up query that retrieve each
objects which ultimately makes `count()` unnecessary since `len(queryset)`
can be used to reduce the number of queries but more importantly
[https://github.com/django/django/blob/38e63c9e61152682f3ff982c85a73793ab6d3267/django/contrib/admin/actions.py#L55
to report the accurate number of deleted objects].
[https://docs.djangoproject.com/en/4.2/topics/db/optimization/#don-t
-overuse-contains-count-and-exists It's even a documented anti-pattern in
our docs].

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

[Django]

#34462: Deletions in admin panel create N + 1 queries

-------------------------------------+-------------------------------------
Reporter: Mike Lissner | Owner: Akash
Type: | Kumar Sen
Cleanup/optimization | Status: assigned

Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: performance | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Akash Kumar Sen):

* owner: nobody => Akash Kumar Sen
* status: new => assigned

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

[Django]

#34462: Deletions in admin panel create N + 1 queries
-------------------------------------+-------------------------------------
Reporter: Mike Lissner | Owner: Akash
Type: | Kumar Sen
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: performance | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Natalia Bidart):

* needs_better_patch: 0 => 1
* has_patch: 0 => 1
* needs_tests: 0 => 1
* needs_docs: 0 => 1

Comment:

[[https://github.com/django/django/pull/16790|PR]]

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

[Django]

#34462: Deletions in admin panel create N + 1 queries
-------------------------------------+-------------------------------------
Reporter: Mike Lissner | Owner: Akash
Type: | Kumar Sen
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: performance | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Natalia Bidart):

* needs_better_patch: 1 => 0
* has_patch: 1 => 0
* needs_tests: 1 => 0
* needs_docs: 1 => 0

Comment:

The proposed PR was targetting the wrong branch, I closed it with an
explanatory comment and further guidance on needed tests and docs. I'm
unsetting all the flags to allow for the new work to be considered
independently.

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

[Django]

#34462: Deletions in admin panel create N + 1 queries
-------------------------------------+-------------------------------------
Reporter: Mike Lissner | Owner: Akash
Type: | Kumar Sen
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: performance | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Akash Kumar Sen):

PR https://github.com/django/django/pull/16792

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

[Django]

#34462: Deletions in admin panel create N + 1 queries
-------------------------------------+-------------------------------------
Reporter: Mike Lissner | Owner: Akash
Type: | Kumar Sen
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: performance | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by David Sanders):

* needs_better_patch: 0 => 1
* has_patch: 0 => 1

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

[Django]

#34462: Deletions in admin panel create N + 1 queries
-------------------------------------+-------------------------------------
Reporter: Mike Lissner | Owner: Akash
Type: | Kumar Sen
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: performance | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by GitHub <noreply@…>):

In [changeset:"851b6879565bfd8f7cc8005b3f92c0437de0bfff" 851b6879]:
{{{
#!CommitTicketReference repository=""
revision="851b6879565bfd8f7cc8005b3f92c0437de0bfff"
Refs #34462 -- Fixed queryset antipattern when processing object deletion.
}}}

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

[Django]

#34462: Deletions in admin panel create N + 1 queries
-------------------------------------+-------------------------------------
Reporter: Mike Lissner | Owner: Akash
Type: | Kumar Sen
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: performance | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 1

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

Changes (by Mariusz Felisiak):

* needs_docs: 0 => 1

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

[Django]

#34462: Deletions in admin panel create N + 1 queries
-------------------------------------+-------------------------------------
Reporter: Mike Lissner | Owner: Akash
Type: | Kumar Sen
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: performance | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Akash Kumar Sen):

PR ready for review https://github.com/django/django/pull/16792

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

[Django]

#34462: Deletions in admin panel create N + 1 queries
-------------------------------------+-------------------------------------
Reporter: Mike Lissner | Owner: Akash
Type: | Kumar Sen
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: performance | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Akash Kumar Sen):

Updated the PR : https://github.com/django/django/pull/16792

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

[Django]

#34462: Deletions in admin panel create N + 1 queries
-------------------------------------+-------------------------------------
Reporter: Mike Lissner | Owner: Akash
Type: | Kumar Sen
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: performance | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by David Sanders):

Akash the mergers are busy so a little patience is required. They have a
list of PRs that they work through and yours is on the list ;)

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

[Django]

#34462: Deletions in admin panel create N + 1 queries
-------------------------------------+-------------------------------------
Reporter: Mike Lissner | Owner: Akash
Type: | Kumar Sen
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: performance | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Akash Kumar Sen):

* needs_better_patch: 1 => 0
* needs_docs: 1 => 0

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

[Django]

#34462: Deletions in admin panel create N + 1 queries
-------------------------------------+-------------------------------------
Reporter: Mike Lissner | Owner: Akash
Type: | Kumar Sen
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: performance | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by David Wobrock):

* cc: David Wobrock (added)

* needs_tests: 0 => 1

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

[Django]

#34462: Deletions in admin panel create N + 1 queries
-------------------------------------+-------------------------------------
Reporter: Mike Lissner | Owner: Akash
Type: | Kumar Sen
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: performance | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Akash Kumar Sen):

* needs_tests: 1 => 0

Comment:

Test added at :
[https://github.com/django/django/blob/497d6f54695422b829f997952e6721a38282337b/tests/admin_utils/test_logentry.py#L291
New testcase]
Getting some weird and untraceable failures, some review will be helpful:
[https://github.com/django/django/pull/16792 PR]

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

[Django]

#34462: Deletions in admin panel create N + 1 queries
-------------------------------------+-------------------------------------
Reporter: Mike Lissner | Owner: Akash
Type: | Kumar Sen
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: performance | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak):

* needs_better_patch: 0 => 1

* needs_docs: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/34462#comment:15>

[Django]

#34462: Deletions in admin panel create N + 1 queries
-------------------------------------+-------------------------------------
Reporter: Mike Lissner | Owner: Akash
Type: | Kumar Sen
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: performance | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Akash Kumar Sen):

* needs_better_patch: 1 => 0
* needs_docs: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/34462#comment:16>

[Django]

#34462: Deletions in admin panel create N + 1 queries
-------------------------------------+-------------------------------------
Reporter: Mike Lissner | Owner: Akash
Type: | Kumar Sen
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: performance | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/34462#comment:17>

[Django]

#34462: Deletions in admin panel create N + 1 queries
-------------------------------------+-------------------------------------
Reporter: Mike Lissner | Owner: Akash
Type: | Kumar Sen
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: performance | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Akash Kumar Sen):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/34462#comment:18>

[Django]

#34462: Deletions in admin panel create N + 1 queries
-------------------------------------+-------------------------------------
Reporter: Mike Lissner | Owner: Akash
Type: | Kumar Sen
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:
Keywords: performance | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"45e0c5892f84b5bb47999cbe16eabb5a13293d85" 45e0c589]:
{{{
#!CommitTicketReference repository=""
revision="45e0c5892f84b5bb47999cbe16eabb5a13293d85"
Refs #34462 -- Moved ModelAdmin.log_deletion() test to a separate test
case.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/34462#comment:19>

[Django]

#34462: Deletions in admin panel create N + 1 queries
-------------------------------------+-------------------------------------
Reporter: Mike Lissner | Owner: Akash
Type: | Kumar Sen
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution:

Keywords: performance | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/34462#comment:20>

[Django]

#34462: Deletions in admin panel create N + 1 queries
-------------------------------------+-------------------------------------
Reporter: Mike Lissner | Owner: Akash
Type: | Kumar Sen

Cleanup/optimization | Status: closed
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution: fixed

Keywords: performance | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

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

Comment:

Fixed in 40b3975e7d3e1464a733c69171ad7d38f8814280.

--
Ticket URL: <https://code.djangoproject.com/ticket/34462#comment:21>

[Django]

#34462: Deletions in admin panel create N + 1 queries
-------------------------------------+-------------------------------------
Reporter: Mike Lissner | Owner: Akash
Type: | Kumar Sen
Cleanup/optimization | Status: closed
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution: fixed
Keywords: performance | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Sarah Boyce <42296566+sarahboyce@…>):

In [changeset:"6c120508b6445cb0d6198b4eacccd411960686d2" 6c120508]:
{{{#!CommitTicketReference repository=""
revision="6c120508b6445cb0d6198b4eacccd411960686d2"
Refs #34462 -- Removed ModelAdmin.log_deletion() and
LogEntryManager.log_action() per deprecation timeline.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34462#comment:22>