Deletions in admin panel create N + 1 queries

[Mike Lissner]

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.

[Simon Charette]

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 ​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 ​to report the accurate number of deleted objects. ​It's even a documented anti-pattern in our docs.

[Natalia Bidart]

​PR

[Natalia Bidart]

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.

[Akash Kumar Sen]

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

[GitHub <noreply@…>]

In 851b6879:

Refs #34462 -- Fixed queryset antipattern when processing object deletion.

[Akash Kumar Sen]

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

[Akash Kumar Sen]

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

[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 ;)

[Akash Kumar Sen]

Test added at : ​New testcase
Getting some weird and untraceable failures, some review will be helpful: ​PR

[Mariusz Felisiak <felisiak.mariusz@…>]

In 45e0c589:

Refs #34462 -- Moved ModelAdmin.log_deletion() test to a separate test case.

[Mariusz Felisiak]

In 40b3975e:

Fixed #34462 -- Made admin log actions in bulk.

This also deprecates ModelAdmin.log_deletion() and
LogEntryManager.log_action().

[Sarah Boyce <42296566+sarahboyce@…>]

In 6c120508:

Refs #34462 -- Removed ModelAdmin.log_deletion() and LogEntryManager.log_action() per deprecation timeline.