[Django] #31263: remote_field model caching issues in RenameModel migration
Django
-------------------------------------+-------------------------------------
Reporter: Sean | Owner: nobody
Esterkin |
Type: | Status: new
Uncategorized |
Component: | Version: 2.2
Migrations | Keywords: migration, caching,
Severity: Normal | remote_field, RenameModel
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
I am encountering a bug when renaming a model with RenameModel. My project
is fairly large with multiple applications and many migrations. With the
project in its current state, the migrations run correctly. I have added a
new migration with some earlier dependencies, which changed the order that
some of the migrations ran in, and now one of the Many to Many tables
foreign key column is not being renamed correctly.
I have tracked this issue down to the function
`_populate_directed_relation_graph` within `django/db/models/options.py`.
This function loops through all of the models and populates a
related_objects_graph with each model pointing to a list of related
fields.
Related fields are populated here
{{{
for f in fields_with_relations:
if not isinstance(f.remote_field.model, str):
related_objects_graph[f.remote_field.model._meta.concrete_model._meta].append(f)
}}}
and accessed here:
{{{
related_objects = related_objects_graph[model._meta.concrete_model._meta]
model._meta.__dict__['_relation_tree'] = related_objects
}}}
The bug seems to be that `f.remote_field.model._meta.concrete_model._meta`
is not the same for every field pointing to what should be the same model.
For example if my code looked like:
{{{
class M1(models.Model):
field1 = models.BooleanField()
class M2(models.Model):
m1 = models.ForeignKey(M1, on_delete=models.CASCADE)
class M3(models.Model):
m1s = models.ManyToManyField(M1, related_name="m3s")
}}}
then the populated `related_objects` for M1 might look like
{{{
defaultdict(<class 'list'>, {<Options for M1>:
[<django.db.models.fields.related.ForeignKey: m1>], <Options for M1>:
[<django.db.models.fields.related.ManyToManyField: m1s>]}
}}}
where each <Options for M1> is slightly different.
This is very likely a caching bug. When I disable the @cached_property
(see below) the bug does not occur. Additionally the bug only occurs when
I run the migrations start to finish. If I run the migrations one at a
time in the same order, this bug does not happen.
For diagnostics, I have found two changes to the Django code that cause
this bug to stop occurring. Neither of these are good changes to the
Django code.
The first is to change
{{{
related_objects_graph[f.remote_field.model._meta.concrete_model._meta].append(f)
}}}
to
{{{
related_objects_graph[f.remote_field.model._meta.concrete_model._meta.__str__()].append(f)
}}}
and
{{{
related_objects = related_objects_graph[model._meta.concrete_model._meta]
}}}
to
{{{
related_objects =
related_objects_graph[model._meta.concrete_model._meta.__str__()]
}}}
The other fix involved editing the ` __get__` function on
`cached_property` in `django/utils/functional.py`.
{{{
if instance is None:
return self
res = instance.__dict__[self.name] = self.func(instance)
return res
}}}
becomes
{{{
if instance is None:
return self
return self.func(instance)
}}}
crippling the caching but fixing the caching bug.
--
Ticket URL: <https://code.djangoproject.com/ticket/31263>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Migrations | Version: 2.2
Severity: Normal | Resolution:
Keywords: migration, caching, | Triage Stage:
remote_field, RenameModel | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* type: Uncategorized => Bug
--
Ticket URL: <https://code.djangoproject.com/ticket/31263#comment:1>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Migrations | Version: 2.2
Severity: Normal | Resolution:
Keywords: migration, caching, | Triage Stage:
remote_field, RenameModel | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
That's likely an issue with `RenameModel.state_forwards`
Could you set `delay=False` in the `reload_models` calls and see if it
helps
That should trigger an immediate re-rerender of all `to_reload` models
instead of ''deferring'' on demand which seems to be the issue here.
In order to address this issue we'll need a way to reproduce it so if you
could reduce your large project to a simplified set of apps/migrations
that would be greatly appreciated.
--
Ticket URL: <https://code.djangoproject.com/ticket/31263#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Sean Esterkin | Owner: nobody
Component: Migrations | Version: 2.2
Severity: Normal | Resolution: needsinfo
Keywords: migration, caching, | Triage Stage:
remote_field, RenameModel | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => needsinfo
--
Ticket URL: <https://code.djangoproject.com/ticket/31263#comment:3>