From patchwork Sat Nov 19 16:51:20 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Finucane X-Patchwork-Id: 696861 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [103.22.144.68]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3tLgtB2pmjz9t1T for ; Sun, 20 Nov 2016 03:53:38 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=fail reason="key not found in DNS" (0-bit key; unprotected) header.d=that.guru header.i=@that.guru header.b="dq8a6RQc"; dkim-atps=neutral Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 3tLgtB1CqvzDvjX for ; Sun, 20 Nov 2016 03:53:38 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; dkim=fail reason="key not found in DNS" (0-bit key; unprotected) header.d=that.guru header.i=@that.guru header.b="dq8a6RQc"; dkim-atps=neutral X-Original-To: patchwork@lists.ozlabs.org Delivered-To: patchwork@lists.ozlabs.org Received: from camel.maple.relay.mailchannels.net (camel.maple.relay.mailchannels.net [23.83.214.29]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3tLgsR6gS0zDvmh for ; Sun, 20 Nov 2016 03:52:59 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; dkim=fail reason="key not found in DNS" (0-bit key; unprotected) header.d=that.guru header.i=@that.guru header.b="dq8a6RQc"; dkim-atps=neutral X-Sender-Id: mxroute|x-authuser|stephen@that.guru Received: from relay.mailchannels.net (localhost [127.0.0.1]) by relay.mailchannels.net (Postfix) with ESMTP id D0D57A0CF9; Sat, 19 Nov 2016 16:52:55 +0000 (UTC) Received: from one.mxroute.com (ip-10-220-3-24.us-west-2.compute.internal [10.220.3.24]) by relay.mailchannels.net (Postfix) with ESMTPA id 18CF8A0A0A; Sat, 19 Nov 2016 16:52:53 +0000 (UTC) X-Sender-Id: mxroute|x-authuser|stephen@that.guru Received: from one.mxroute.com ([UNAVAILABLE]. [10.95.17.29]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384) by 0.0.0.0:2500 (trex/5.7.8); Sat, 19 Nov 2016 16:52:55 +0000 X-MC-Relay: Neutral X-MailChannels-SenderId: mxroute|x-authuser|stephen@that.guru X-MailChannels-Auth-Id: mxroute X-MC-Loop-Signature: 1479574375195:1766260898 X-MC-Ingress-Time: 1479574375195 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=that.guru; s=default; h=References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Sender:Reply-To:MIME-Version:Content-Type:Content-Transfer-Encoding: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=Emjec/5DJBekQwb/f+JS5ZPpxAGYr4rVDlm7CMMThr4=; b=dq8a6RQcvWbcSLg7AX/+q92xeY gqWWCT28hceL7GQDF3M5a5B7x/Iv0mWLsnWhWBJc2SkiKn2SUQR9lEW1qzLFjH2L7jzfvJdUtcq4F Yh0SWjix1Xxx22kcxYwdzOzUlvc0JHE4EsesQv9QYZOFwwccT4rGlBDbg+dx9uiN+HVnWDeNwIZ7n x92tjFyPEu6JMFpoWKvLHK7QIaPOUyJhh0lRKXPh5nIyeOHzpP8jKiRrsYO9K/ic/UHU5alvkX2FR w3fnFE751MIhuxdKotAbohQsoec4RJZuDufB/Vsm2uWfXyY7Z5JCQpqklKJqHnrPmV4xCIBKNsEn3 KK1FbMqA==; From: Stephen Finucane To: patchwork@lists.ozlabs.org Subject: [PATCH v2 05/13] REST: Resolve performance issues with tags Date: Sat, 19 Nov 2016 16:51:20 +0000 Message-Id: <1479574288-24171-6-git-send-email-stephen@that.guru> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1479574288-24171-1-git-send-email-stephen@that.guru> References: <1479574288-24171-1-git-send-email-stephen@that.guru> X-OutGoing-Spam-Status: No, score=-10.0 X-AuthUser: stephen@that.guru X-BeenThere: patchwork@lists.ozlabs.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Patchwork development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: patchwork-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Patchwork" The list comprehension used to generate a list of tags resulted in a significant number of duplicated queries. Resolve this by copying the approach taken to minimize patch queries in the UI. This changes the output of the response in two ways. First, counts for all tag patches are now shown, even if the count is 0. Secondly, a dictionary is returned, with the tag name as the key, rather than a list. As such, the output for a typical patch is transformed from: [ { 'name': 'Reviewed-by', 'count': 1 } ] to { 'Reviewed-by': 1 'Acked-by': 0, 'Tested-by': 0 } How this is achieved is a little hacky, but reworked tags are on the post-v2.0 which will allow this to be resolved. Signed-off-by: Stephen Finucane Cc: Andy Doan --- patchwork/api/patch.py | 12 +++++------- patchwork/models.py | 13 ++++++++++--- patchwork/tests/test_rest_api.py | 5 ++--- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py index bfd8fe3..bc4cb5c 100644 --- a/patchwork/api/patch.py +++ b/patchwork/api/patch.py @@ -52,9 +52,8 @@ class PatchSerializer(HyperlinkedModelSerializer): return request.build_absolute_uri(instance.get_mbox_url()) def get_tags(self, instance): - # TODO(stephenfin): I don't think this is correct - too many queries - return [{'name': x.tag.name, 'count': x.count} - for x in instance.patchtag_set.all()] + return {x.name: getattr(instance, x.attr_name) + for x in instance.project.tags} def get_headers(self, instance): if instance.headers: @@ -85,10 +84,9 @@ class PatchViewSet(PatchworkViewSet): serializer_class = PatchSerializer def get_queryset(self): - qs = super(PatchViewSet, self).get_queryset( - ).prefetch_related( - 'check_set', 'patchtag_set' - ).select_related('state', 'submitter', 'delegate') + qs = super(PatchViewSet, self).get_queryset().with_tag_counts()\ + .prefetch_related('check_set')\ + .select_related('state', 'submitter', 'delegate') if 'pk' not in self.kwargs: # we are doing a listing, we don't need these fields qs = qs.defer('content', 'diff', 'headers') diff --git a/patchwork/models.py b/patchwork/models.py index a27dda6..d1d0857 100644 --- a/patchwork/models.py +++ b/patchwork/models.py @@ -225,8 +225,8 @@ def get_default_initial_patch_state(): class PatchQuerySet(models.query.QuerySet): - def with_tag_counts(self, project): - if not project.use_tags: + def with_tag_counts(self, project=None): + if project and not project.use_tags: return self # We need the project's use_tags field loaded for Project.tags(). @@ -236,7 +236,14 @@ class PatchQuerySet(models.query.QuerySet): qs = self.prefetch_related('project') select = OrderedDict() select_params = [] - for tag in project.tags: + + # All projects have the same tags, so we're good to go here + if project: + tags = project.tags + else: + tags = Tag.objects.all() + + for tag in tags: select[tag.attr_name] = ( "coalesce(" "(SELECT count FROM patchwork_patchtag" diff --git a/patchwork/tests/test_rest_api.py b/patchwork/tests/test_rest_api.py index 3be8ecf..ddce4d7 100644 --- a/patchwork/tests/test_rest_api.py +++ b/patchwork/tests/test_rest_api.py @@ -304,9 +304,8 @@ class TestPatchAPI(APITestCase): content='Reviewed-by: Test User \n') resp = self.client.get(self.api_url(patch.id)) tags = resp.data['tags'] - self.assertEqual(1, len(tags)) - self.assertEqual(1, tags[0]['count']) - self.assertEqual('Reviewed-by', tags[0]['name']) + self.assertEqual(3, len(tags)) + self.assertEqual(1, tags['Reviewed-by']) def test_anonymous_create(self): """Ensure anonymous "POST" operations are rejected."""