Opened 5 years ago

Closed 5 years ago

#31263 closed Bug (needsinfo)

remote_field model caching issues in RenameModel migration.

Reported by: Sean Esterkin Owned by: nobody
Component: Migrations Version: 2.2
Severity: Normal Keywords: migration, caching, remote_field, RenameModel
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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.

Change History (3)

comment:1 by Sean Esterkin, 5 years ago

Type: UncategorizedBug

comment:2 by Simon Charette, 5 years ago

That's likely an issue with RenameModel.state_forwards

https://github.com/django/django/blob/1712a76b9dfda1ef220395e62ea87079da8c9f6c/django/db/migrations/operations/models.py#L304-L343

Could you set delay=False in the reload_models calls and see if it helps

https://github.com/django/django/blob/1712a76b9dfda1ef220395e62ea87079da8c9f6c/django/db/migrations/operations/models.py#L339-L343

That should trigger an immediate re-rerender of all to_reload models instead of deferring on demand which seems to be the issue here.

It's also possible that this is due to ManyToManyField auto-created models (when no explicit through is specified like in your example) not being reloaded as it seems we only deal with the through is not None case

https://github.com/django/django/blob/1712a76b9dfda1ef220395e62ea87079da8c9f6c/django/db/migrations/operations/models.py#L325-L333

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.

Last edited 5 years ago by Simon Charette (previous) (diff)

comment:3 by Mariusz Felisiak, 5 years ago

Resolution: needsinfo
Status: newclosed
Summary: remote_field model caching issues in RenameModel migrationremote_field model caching issues in RenameModel migration.
Note: See TracTickets for help on using tickets.
Back to Top