tolomea/django-auto-prefetch

N+1 issue in admin

sdementen opened this issue · 7 comments

I have a set of models for which auto-prefetch seems to fail as I observe an N+1 issue (via djdt and nplusone).
My simplified versions of models.py and admin.py for the app are the following

models.py

import auto_prefetch
from django.db import models


class Simulation(auto_prefetch.Model):
    pass


class SimulationSource(auto_prefetch.Model):
    name = models.CharField(max_length=10)
    simulation = auto_prefetch.ForeignKey(Simulation, on_delete=models.CASCADE, related_name="simulation_sources")


class Scenario(auto_prefetch.Model):
    simulation = auto_prefetch.ForeignKey(Simulation, on_delete=models.CASCADE, related_name="scenarios")

    choices = models.ManyToManyField(
        SimulationSource, through="ScenarioChoice", related_name="scenarios"
    )


class ScenarioChoice(auto_prefetch.Model):
    scenario = auto_prefetch.ForeignKey(
        Scenario, on_delete=models.CASCADE, related_name="scenario_choices"
    )
    simulation_source = auto_prefetch.ForeignKey(
        SimulationSource, on_delete=models.CASCADE, related_name="scenario_choices"
    )

admin.py

from django.contrib import admin

from .models import Simulation, SimulationSource, Scenario


class SimulationSourceInlineAdmin(admin.TabularInline):
    model = SimulationSource


class ScenarioChoiceInlineAdmin(admin.TabularInline):
    model = Scenario.choices.through


@admin.register(Simulation)
class SimulationAdmin(admin.ModelAdmin):
    inlines = [SimulationSourceInlineAdmin]


@admin.register(Scenario)
class ScenarioAdmin(admin.ModelAdmin):
    inlines = [ScenarioChoiceInlineAdmin]

Once the app is added to the django project, I go to the admin panel and:

  1. create a Simulation with 3 SimulationSource (via the TabularInline)
  2. create a Scenario
    => refreshing the admin page of the Scenario, I see already multiple lines like
    (0.000) SELECT "buggy_simulationsource"."id", "buggy_simulationsource"."name", "buggy_simulationsource"."simulation_id" FROM "buggy_simulationsource"; args=()
  3. in the Scenario admin, add ScenarioChoices
    => for each new ScenarioChoice, when refreshing the admin page of the Scenarion, there is an extra line
    (0.000) SELECT "buggy_simulationsource"."id", "buggy_simulationsource"."name", "buggy_simulationsource"."simulation_id" FROM "buggy_simulationsource"; args=()

Is my use of django-auto-prefetch correct ? is it some bug ?
I have also tried before using django-auto-prefetch to add manually the right select_related and prefetch_related but with no luck. I always had these weird simulationsource queries.

What Django version are you using?

Your queries are being caused by the dropdown fields, each one does a request to get the list of available options.

You will see one for every dropdown on the page.

Also there is an extra hidden inline that's used by the "add another scenario choice" javascript, so you will also see queries for any dropdowns on that.

Because these are all actual queryset invocations auto-prefetch can't do anything about them.

Does that explain all the queries you see?

In terms of figuring this out.
The Django debug toolbar will give you stack traces for where the queries are being evaluated which is quite handy, although it will at times lie about how many there are.
Based on that and prior experience I tried changing the admins / inlines to remove the drop down fields and observed that that made the queries go away.
Further experiments show that the number of queries also scales with the number of dropdowns.

thank you for pinpointing the issue!
indeed, if there is a different queryset for every dropdown, no prefetching will help...
it is nevertheless puzzling as default behavior in the django admin.
I close the issue as not linked to django-auto-prefetch.
and thank you for your great package! do you know if this feature is still considered to be integrated in django ?

There is interest in the idea but no one has had the time to champion it.

You are right that it's puzzling default behaviour. It's kinda a case of the easy solution being inefficient but no one getting round to implementing a better solution. It really needs to recognize where it can share the options across many drop downs.

Also as your total number of simulations grows the sheer data load of those drop downs, both at the DB and HTML levels will become a serious problem. The easiest solution to that is liberal application of autocomplete_fields. In past projects I've rejigged things so that FK's default to autocomplete and you have to explicitly opt in to drop downs where you know there won't be many rows.

With autocompletes you may even be able to remove the queries entirely as it only really needs the current value and current values are very amenable to prefetching. I haven't looked into that option, it will depend on exactly how the autocomplete widget works.

I did not know about the autocomplete_fields feature!

I have adapted my admin.py to

from django.contrib import admin

from .models import SimulationSource, Scenario


@admin.register(SimulationSource)
class SimulationSourceAdmin(admin.ModelAdmin):
    model = SimulationSource
    search_fields = ["id"]


class SimulationSourceInlineAdmin(admin.TabularInline):
    model = SimulationSource


class ScenarioChoiceInlineAdmin(admin.TabularInline):
    model = Scenario.choices.through
    autocomplete_fields = ["simulation_source"]


@admin.register(Scenario)
class ScenarioAdmin(admin.ModelAdmin):
    inlines = [ScenarioChoiceInlineAdmin]

which alleviates the issue. There are no more queries for the 3 extra dropdowns nor the hidden one (as far as I understand).
However, I still get a query for each ScenarioChoice.simulation_source already added in the Scenario. Would this be the last option you are refering to in your last message?

The 3 extra drop downs go away because they all have a value of None, so it doesn't need to do a query to generate the string for the current value.

Yes, in theory you should be able to prefetch the values used to render the strings for the current values, but as I said above the feasibility of that it will depend a lot on how the autocomplete widget is implemented.