Code

#18434 closed Cleanup/optimization (needsinfo)

Limit ModelFormSet queryset parameter

Reported by: sbaechler Owned by: nobody
Component: Forms Version: 1.2
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

This error happened to us a while ago (with Django 1.2.3) and I don't know if the issue has been fixed since. But I didn't find anything in the history.

The issue was this. One of our servers was starting to not react for several seconds a few times every day. The processor load went up to 200% and it had to use disk cache because it run out of RAM. It took us a while until we found the issue:

A few ModelFormSets were instantiated without the queryset parameter. Just POST data, which was sufficient for the app to work. There was no performance issue while the database had only a few entries but once it reached a few thousand, it brought the server down.

The issue was resolved once I added the queryset parameter. I don't know if it was caused because of the missing queryset parameter or the fact that the default manager returned a few thousand entries.

But it should be possible to add a fail-safe or warning if the queryset and data that comes in via POST don't match. Isn't it possible to get the primary keys from the POST data?

The formsets were created using the modelformset_factory. I have attached the view code.

Attachments (1)

view.txt (5.4 KB) - added by sbaechler 23 months ago.
view for ticket

Download all attachments as: .zip

Change History (2)

Changed 23 months ago by sbaechler

view for ticket

comment:1 Changed 22 months ago by aaugustin

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to needsinfo
  • Status changed from new to closed

The intended way to use ModelFormSets is:

  • 1 - on a GET request, instantiate a ModelFormSet and render it in a template
  • 2 - on a POST request, instantiate the same ModelFormSet with request.POST, process it and redirect

If evaluating the queryset causes excessive load, things should blow up at step 1, not at step 2. The problem will be obvious: your page will contain a form for each row in the database. I don't understand exactly what you're doing, but it seems to me that your code for steps 1 and 2 wasn't in sync.

I don't think it's worth attempting to magically hide such errors by restricting the queryset to POSTed objects.

If that doesn't answer your question, could you provide a bit more information? In particular, explain why step 1 works but not step 2. Thanks!


PS: I see that you've added some hardcoded filters to your view code. This isn't a maintainable solution because you just added tight coupling between your code and existing data in your production database. I don't know enough about your code base to suggest a better solution, but I recommend refactoring this.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.