0

Ive added a prefetch to django admin as I noticed it was running nearly 1000 queries for a model.

However the prefetch seems to have had zero effect. as far as I can see the prefetch query is correct?

example duplicate:

SELECT "sites_sitedata"."id", "sites_sitedata"."location", "sites_sitedata"."
 ... 
FROM "sites_sitedata" WHERE "sites_sitedata"."id" = '7'
  Duplicated 314 times. 
0.09126367597049141%
0.24    
Sel Expl
Connection: default
/itapp/itapp/circuits/models.py in __str__(88)
  return '%s | %s | %s | %s ' % (self.site_data.location, \

there are also duplicates for, circuit providers, and circuit types glancing at a high level

admin.py

class SiteSubnetsAdmin(admin.ModelAdmin):
    search_fields = ['site_data','device_data','subnet','subnet_type','vlan_id','peer_desc'] 
    list_display = ('site_data','device_data','subnet','subnet_type','vlan_id','peer_desc')
    ordering = ('site_data','device_data',)

    def get_queryset(self, request):
        queryset = super(SiteSubnetsAdmin, self).get_queryset(request)
        queryset = SiteSubnets.objects \
                .prefetch_related( 
                    Prefetch(
                        'circuit',
                        queryset=Circuits.objects.prefetch_related('site_data').prefetch_related('service_contacts').prefetch_related('circuit_type').prefetch_related('provider'),
                    ),
                ) \
                .prefetch_related('site_data') \
                .prefetch_related('device_data') \
                .prefetch_related('subnet_type')
        return queryset

admin.site.register(SiteSubnets, SiteSubnetsAdmin)

subnets.models

class SiteSubnets(models.Model):
    device_data = models.ForeignKey(DeviceData, verbose_name="Device", \
                on_delete=models.PROTECT, blank=True, null=True)
    site_data = models.ForeignKey(SiteData, verbose_name="Location", \
                on_delete=models.PROTECT, blank=True, null=True)               
    subnet = models.GenericIPAddressField(protocol='IPv4', \
             verbose_name="Subnet", blank=True, null=True)
    subnet_type = models.ForeignKey(SubnetTypes, verbose_name="Subnet Type") 
    circuit = models.ForeignKey(Circuits, verbose_name="Link to circuit?", \
                on_delete=models.PROTECT, blank=True, null=True)
    vlan_id = models.IntegerField(verbose_name="Vlan ID", blank=True, null=True)
    peer_desc = models.IntegerField(verbose_name="Peer description", blank=True, null=True)
    class Meta:
        verbose_name = "Site Subnets"
        verbose_name_plural = "Site Subnets"

    def __str__(self):
        if self.device_data != None:
            return '{0} - {1} - {2}'.format(self.site_data,self.device_data, self.subnet)       
        else:
            return '{0} - {1}'.format(self.site_data, self.subnet) 

circuits.models

class Circuits(models.Model):
    site_data = models.ForeignKey(SiteData, verbose_name="Site", on_delete=models.PROTECT)
    order_no = models.CharField(max_length=200, verbose_name="Order No")
    expected_install_date = models.DateField()
    install_date = models.DateField(blank=True, null=True)
    circuit_type = models.ForeignKey(CircuitTypes, verbose_name="Circuit Type", on_delete=models.PROTECT)
    circuit_preference = models.CharField(max_length=20, verbose_name="Circuit Preference", \
                         choices=settings.CIRCUIT_PREFERENCE, blank=True, null=True)
    provider = models.ForeignKey(CircuitProviders, verbose_name="Circuit Provider", on_delete=models.PROTECT)
    service_contacts = models.ForeignKey(ServiceContacts, on_delete=models.PROTECT)
    subnet_mask = models.CharField(max_length=4, verbose_name="Subnet Mask", \
                  choices=settings.SUBNET_MASK_CHOICES, blank=True, null=True)
    decommissioned = models.BooleanField(default=False, verbose_name="Decomission this circuit?")
    active_link = models.BooleanField(default=False, verbose_name="Active Link?")


    class Meta:
        verbose_name = "Circuit Data"
        verbose_name_plural = "Circuit Data"
        permissions = (
            ("can_view_financial", "Can View Financial"),
            ("can_view_orders", "Can View Orders"),
        )

    def __str__(self):
        return '%s | %s | %s | %s ' % (self.site_data.location, \
                                       self.provider, self.circuit_type, self.ref_no) 

EDIT would this be the method stored separately? in the same model or does it need to be external to the model?

def full_info(self): 
    return '{} | {} | {} | {}'.format(self.site_data.location, \
                                   self.provider, self.circuit_type, self.ref_no)     

def __str__(self):
    return '{} | {} | {}'.format(self.provider, self.circuit_type, self.ref_no)
AlexW
  • 2,843
  • 12
  • 74
  • 156

2 Answers2

2

You should use select_related for the forward foreign keys. prefetch_related prefetches reverse foreign key and many-to-many relations:

# ...
queryset=Circuits.objects\
    .select_related('site_data', 'service_contacts', 'circuit_type', 'provider'),
# ...
.select_related('site_data', 'device_data', 'subnet_type')

Displaying all this fk info in the __str__ can be troublesome as the __str__is used by default in several places in the admin, like the change list but also in form input dropdowns. You could display the pk of the related objects instead (if that is informative enough), as it is stored in the local table and won't create extra queries:

def __str__(self):
    return '%s | %s | %s | %s ' % (self.site_data_id, \
                                   self.provider_id, self.circuit_type_id, self.ref_no) 
user2390182
  • 72,016
  • 6
  • 67
  • 89
1

Firstly, I would try to avoid including foreign keys in __str__ methods. It can lead to huge numbers of queries like this. Keep the __str__ method as simple possible, and if you need to display related information then create another method and use that when necessary.

Secondly, I'm not sure that Django supports using a prefetch_related inside a Prefetch object. In your case, you don't even need to use prefetch_related. For foreign keys you can use select_related and Django will do a join to fetch the related objects rather than prefetching them in separate queries.

queryset = SiteSubnets.objects.select_related(
    'circuit', 'circuit__site_data', 'circuit__service_contacts', 'circuit__circuit_type', 'circuit__provider'
    ).select_related('site_data', 'device_data', subnet_type')

If the additional queries are coming from the select fields for foreign keys in the Django admin, then optimising get_queryset isn't going to help. You could either simplify the __str__ methods as I suggested, dig deeper into the Django admin internals to try to optimise the queries, use a ajax widget to prevent loading all the options, or put up with the large number of queries.

Alasdair
  • 298,606
  • 55
  • 578
  • 516
  • It's just a method that returns a string, e.g. `def full_info(self): return '%s | %s | %s | %s ...' ` – Alasdair Jan 24 '18 at 11:05
  • ive added my self method to the question, Im not sure its right? – AlexW Jan 24 '18 at 11:22
  • It's a method on the model, so you would call `self.full_info()`. However, if you call that method inside `__str__`, that would defeat the point of creating a separate method. The idea is to simplify `__str__` so that it doesn't use foreign keys, then use `full_info` when you really need to display all the information. – Alasdair Jan 24 '18 at 11:26
  • I think I understand, so like the above edit I have done? then if I want the site_data.location to be display I would then call self.full_info(), how would I get self.full_info into a django admin select box? – AlexW Jan 24 '18 at 12:07
  • That's a separate question. To start with you'll need a custom model choice field, see e.g. https://stackoverflow.com/questions/3167824/change-django-modelchoicefield-to-show-users-full-names-rather-than-usernames. Then use a custom model form, or override something like `formfield_for_foreignkey`. – Alasdair Jan 24 '18 at 12:10
  • ok I removed all the selfs that references other models and my queries are 9 now instead of 1000. so this was definitely the cause. – AlexW Jan 24 '18 at 14:21