Opened 2 years ago

Closed 6 months ago

#22330 closed Cleanup/optimization (needsinfo)

Model.__reduce__() includes cached lookups

Reported by: patrys 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

Description

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 timo

  • Component changed from Uncategorized to Database layer (models, ORM)
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Cleanup/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 6 months ago by akaariai

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 6 months ago by timgraham

  • Resolution set to needsinfo
  • Status changed from new to closed

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