#28462 closed Bug (fixed)
ModelAdmin.list_editable unusably slow and memory intensive with large datasets
| Reported by: | Ben Cole | Owned by: | nobody |
|---|---|---|---|
| Component: | contrib.admin | Version: | 1.10 |
| Severity: | Normal | Keywords: | |
| Cc: | josh.smeaton@… | Triage Stage: | Ready for checkin |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
Since 1.10 list_editable on ModelAdmin is unusable for Models with a large-ish dataset.
The problem is caused by a recent change to how the FormSet is generated for the admin. Previously it was generated from the ChangeList result list, but it has been changed to use the admin's get_queryset which will return more than the current "pageful" of results (potentially the entire dataset) causing Django to generate a form for each instance. This results in Django consuming all available RAM and in some cases the python instance crashing. My personal laptop became unresponsive and I had to force power off.
Attachments (1)
Change History (19)
comment:1 by , 7 years ago
| Summary: | list_editable unusable with large datasets → ModelAdmin.list_editable unusably slow and memory intensive with large datasets |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
comment:2 by , 7 years ago
| Has patch: | set |
|---|---|
| Needs tests: | set |
comment:3 by , 7 years ago
+1 Same error. We have overwritten the ModelAdmin for this problem. I think to solve a strange problem and getting a memory problem is not a good solution. I prefer a 500 error one time every three month that a serious memory problem every day.
This commit https://github.com/django/django/commit/917cc288a38f3c114a5440f0749b7e5e1086eb36#commitcomment-23412084 does not make any sense.
Four examples of this big problem:
Please consider revert this change, it is very dangerous for other people if they don't identify this bug/trap.
by , 7 years ago
| Attachment: | memory-problem.png added |
|---|
comment:4 by , 7 years ago
| Easy pickings: | set |
|---|
comment:5 by , 7 years ago
| Easy pickings: | unset |
|---|
I don't think reverting is a good idea as that reintroduces possible data loss.
comment:6 by , 7 years ago
I just made a PR to solve this problem: https://github.com/django/django/pull/9820
comment:7 by , 7 years ago
| Cc: | added |
|---|
comment:8 by , 7 years ago
| Needs tests: | unset |
|---|---|
| Triage Stage: | Accepted → Ready for checkin |
comment:9 by , 7 years ago
| Triage Stage: | Ready for checkin → Accepted |
|---|
"Ready for checkin" is set by the patch reviewer, not the author.
comment:10 by , 7 years ago
| Patch needs improvement: | set |
|---|
Hi AdamDonna. I'm going to mark this "Patch needs improvement" for now, pending the extra testing outlined on the ticket.
- Please uncheck when that's ready
- @carltongibson me on GitHub if you want/need me to input/help.
Thanks for your effort!
comment:11 by , 7 years ago
| Patch needs improvement: | unset |
|---|---|
| Triage Stage: | Accepted → Ready for checkin |
PR (without a test)