Opened 18 years ago

Closed 18 years ago

#2638 closed defect (duplicate)

[patch] default ForeignKey field results in terrible scaling perfomance

Reported by: heckj@… Owned by: Malcolm Tredinnick
Component: Validators Version: dev
Severity: critical Keywords:
Cc: Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Normally I'm not sure I'd call this a defect, but the default values for models.ForiegnKey (specifically raw_admin_id) have such a negative performance impact that I couldn't avoid it.

The discussion on the django-user's mailing list at http://groups.google.com/group/django-users/browse_thread/thread/ca0640e1cd0149a4/484695e3983c1858?lnk=gst&q=AddManipulator+performance&rnum=1#484695e3983c1858 caught my attention.

While I really like the behavior that's enabled without raw_admin_id, I think the default value should favor the scaling/performance side of Django, with the additional functionality of returning potential values from the validator being the opt in.

I've posted the test case code I used at http://www.rhonabwy.com/django_perf_test.zip

Attachments (2)

joetest.zip (12.4 KB ) - added by heckj@… 18 years ago.
test project illustrating the performance degredation
2638.diff (1.8 KB ) - added by heckj@… 18 years ago.
attaching patch (from trunk) for a suggested resolution. Includes update to documentation

Download all attachments as: .zip

Change History (8)

by heckj@…, 18 years ago

Attachment: joetest.zip added

test project illustrating the performance degredation

comment:1 by Karen Tracey <graybark@…>, 18 years ago

Yes, I think I've run into this also. I actually thought it was this problem (list_display containing ForeignKey create a dead loop):

http://groups.google.com/group/django-users/browse_thread/thread/1d73cec767f42c38/84538ca609dceee2?lnk=gst&q=infinite+loop&rnum=1

but in my case I didn't have a loop in my ForeignKey relations, just over 100,000 potential values for the ForeignKey field. Trying to turn on edit_inline in the admin for a model that pointed to one of these things via ForeignKey resulted in what appeared to be an infinite loop. I only realized later that the problem was some code in admin was trying to populate a list of choices for these things, which isn't feasible for my DB. Malcolm alerted me that he thought I would also hit the problem outside of admin if I just tried to create a manipulator for form processing, but since I haven't gotten to needing to do that yet and I don't need the edit_inline functionality, I hadn't gotten around to worrying about/investigating that yet. I'm glad to hear there is already a way to turn this behavior off; for my DB it would be better if "off" was the default.

Just figured I'd mention that I did run afoul of this, in case it influences what you decide to do with it.

by heckj@…, 18 years ago

Attachment: 2638.diff added

attaching patch (from trunk) for a suggested resolution. Includes update to documentation

comment:2 by heckj@…, 18 years ago

Summary: default ForeignKey field results in terrible scaling perfomance[patch] default ForeignKey field results in terrible scaling perfomance
Version: 0.95SVN

Added patch for a suggested resolution. I also included an update to the documentation associated with the code change.

comment:3 by Malcolm Tredinnick, 18 years ago

This isn't the right solution, although your diagnosis of the problem is more or less accurate. A better solution, I suspect, is to make the get_choices() and/or get_default_choices() functions in the manipulators sufficiently lazy so that they don't load all the values unless they are needed (and they usually aren't needed in things like AddManipulator and ChangeManipulator settings).

comment:4 by Malcolm Tredinnick, 18 years ago

Owner: changed from Adrian Holovaty to Malcolm Tredinnick

comment:5 by Chris Beaven, 18 years ago

Triage Stage: UnreviewedDesign decision needed

comment:6 by Malcolm Tredinnick, 18 years ago

Resolution: duplicate
Status: newclosed

Marking this as a dupe of #3436, since that ticket has a patch that I believe should fix the root cause.

Note: See TracTickets for help on using tickets.
Back to Top