Opened 3 years ago

Closed 12 months ago

#22330 closed Cleanup/optimization (needsinfo)

Model.__reduce__() includes cached lookups

Reported by: Patryk Zawadzki Owned by: nobody
Component: Database layer (models, ORM) Version: 1.6
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


If you try to pickle an object after accessing any of its related instances, its __dict__ will contain references to those instances and will cause them to be pickled as part of your object.

Pickling them can cause additional instances to be traversed and can result in an unexpectedly large number of objects being included. These will be pickled/unpickled as a single chunk and I assume the unpickled instance will continue to use its cached instances which may lead to surprising results (values not reflecting current DB state).

Pickling additional instances can cause relation traversal to cache additional reverse lookups thus modifying an instance's __dict__ while it is being pickled. This results in:

RuntimeError: dictionary changed size during iteration

Change History (3)

comment:1 Changed 2 years ago by Tim Graham

Component: UncategorizedDatabase layer (models, ORM)
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

I am not sure of the expected behavior. If we don't make a change here, we should at least document like we've done for QuerySet pickling.

comment:2 Changed 12 months ago by Anssi Kääriäinen

If nobody offers a good reason why this must be changed, I think we should opt for keeping the current behavior. Sometimes you might actually want to pickle the related instances, too. And, changes here might result in performance changes for deployed applications, which are nasty to detect when updating Django. One example is that caching results of select_related queryset would not cache the related selections after the change.

Above, the RuntimeError thrown by pickling looks like a good argument for fixing. But as long as we don't have a test to reproduce this issue, it is hard to say if the problem is in Django or how to fix the problem if it is in Django.

comment:3 Changed 12 months ago by Tim Graham

Resolution: needsinfo
Status: newclosed

It might be related to issues like #24381, but I agree we can close this until we get a test case to reproduce the problem.

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