Project

General

Profile

Development #68324

SQL: requête sur custom_views exécutée énormément

Added by Pierre Ducroquet 5 months ago. Updated 5 months ago.

Status:
Fermé
Priority:
Normal
Assignee:
-
Target version:
-
Start date:
22 August 2022
Due date:
% Done:

0%

Estimated time:
Patch proposed:
Yes
Planning:
No

Description

Repéré sur toulouse

En environ 10 minutes, la requête suivante a été exécutée 4093 fois, prenant un total de 2 minutes 7 secondes:

SELECT id, title, slug, user_id, visibility, formdef_type, formdef_id, is_default, order_by, columns, filters FROM custom_views


Files

0001-CustomView-reduce-number-of-select-with-no-criteria-.patch (1.17 KB) 0001-CustomView-reduce-number-of-select-with-no-criteria-.patch Pierre Ducroquet, 22 August 2022 03:56 PM
0002-CustomView-index-the-formdef_id-column-we-are-going-.patch (1.11 KB) 0002-CustomView-index-the-formdef_id-column-we-are-going-.patch Pierre Ducroquet, 22 August 2022 03:56 PM
0001-CustomView-reduce-number-of-select-with-no-criteria-.patch (1.21 KB) 0001-CustomView-reduce-number-of-select-with-no-criteria-.patch Pierre Ducroquet, 22 August 2022 04:16 PM
0002-CustomView-index-the-formdef_id-column-we-are-going-.patch (1.94 KB) 0002-CustomView-index-the-formdef_id-column-we-are-going-.patch Pierre Ducroquet, 22 August 2022 04:16 PM
0002-CustomView-index-the-formdef_id-column-we-are-going-.patch (1.92 KB) 0002-CustomView-index-the-formdef_id-column-we-are-going-.patch Pierre Ducroquet, 22 August 2022 05:08 PM
0001-CustomView-reduce-number-of-select-with-no-criteria-.patch (1.72 KB) 0001-CustomView-reduce-number-of-select-with-no-criteria-.patch Pierre Ducroquet, 22 August 2022 05:08 PM
0001-CustomView-reduce-number-of-select-with-no-criteria-.patch (1.72 KB) 0001-CustomView-reduce-number-of-select-with-no-criteria-.patch Pierre Ducroquet, 23 August 2022 12:13 AM
0002-CustomView-index-the-formdef_id-column-we-are-going-.patch (1.92 KB) 0002-CustomView-index-the-formdef_id-column-we-are-going-.patch Pierre Ducroquet, 23 August 2022 12:13 AM
0003-CustomView-remove-other-calls-to-criteria-less-selec.patch (3.25 KB) 0003-CustomView-remove-other-calls-to-criteria-less-selec.patch Pierre Ducroquet, 23 August 2022 12:13 AM
0002-custom-views-remove-frequent-calls-to-criteria-less-.patch (2.87 KB) 0002-custom-views-remove-frequent-calls-to-criteria-less-.patch Frédéric Péters, 24 August 2022 09:36 PM
0001-custom-views-index-the-formdef_id-column-we-are-goin.patch (1.92 KB) 0001-custom-views-index-the-formdef_id-column-we-are-goin.patch Frédéric Péters, 24 August 2022 09:36 PM

Related issues

Related to w.c.s. - Bug #69699: Vue de traitement, vues personnalisées 'user' et 'all' avec le même slugFermé29 September 2022

Actions

Associated revisions

Revision 5f3deb17 (diff)
Added by Pierre Ducroquet 5 months ago

custom views: index the formdef_id column we are going to use more (#68324)

Revision 33f6acea (diff)
Added by Pierre Ducroquet 5 months ago

custom views: remove frequent calls to criteria-less select() (#68324)

History

#1

Updated by Pierre Ducroquet 5 months ago

Exemples de code repéré pouvant correspondre à cette requête:

# wcs/admin/forms.py
        # remove existing shared views
        for view in get_publisher().custom_view_class.select():
            if view.match(user=None, formdef=self.formdef):
                view.remove_self()

# wcs/backoffice/management.py
    def get_custom_views(self, criterias=None):
        for view in get_publisher().custom_view_class.select(clause=criterias):
            if view.match(get_request().user, self.formdef):
                yield view
    def get_formdata_sidebar(self, qs=''):
        r = TemplateIO(html=True)
        r += htmltext('<ul id="sidebar-actions">')
        r += self.get_formdata_sidebar_actions(qs=qs)
        r += htmltext('</ul>')
        views = list(self.get_custom_views())
        # ...

    def save_view(self):
        # ...
            for view in self.get_custom_views():
                if view.id == custom_view.id:
                    continue
                if custom_view.visibility == view.visibility and view.is_default:
                    view.is_default = False
                    view.store()
    # ....
    def _q_lookup(self, component):
        if component == 'ics':
            return self.ics()

        if not self.view:
            for view in self.get_custom_views():
                if view.get_url_slug() == component:
                    return self.__class__(formdef=self.formdef, view=view)

Je vais tenter d'identifier si il y a des requêtes web qui sont plus fréquentes que les autres.

#2

Updated by Pierre Ducroquet 5 months ago

Ce patch corrige le chemin le plus violent que j'ai repéré.

#3

Updated by Agate Berriot 5 months ago

C'est possiblement un détail mais je vois une condition if self.formdef_type != formdef.xml_root_node: dans la fonction def match( que tu as remplacée par ton .select. Est-ce que ça ne risque pas de poser problème ?

#5

Updated by Agate Berriot 5 months ago

  • Status changed from Solution proposée to Solution validée
#7

Updated by Pierre Ducroquet 5 months ago

Note : cette requête représente 25% du temps d'exécution PostgreSQL total cet après-midi sur la prod de toulouse. Donc il y a un vrai intérêt à l'identifier et à l'éliminer.

#8

Updated by Pierre Ducroquet 5 months ago

Suite à une discussion avec Fred, je suis allé chercher plus loin.
J'ai optimisé tous les appels aveugles à custom_views que j'ai trouvé. Je ne vois pas de raison pour qu'en optimisant tous les appels on ne tombe pas sur le bon. Celui sur le slug m'intrigue le plus, ça a l'air d'être un chemin pas mal utilisé.

#9

Updated by Frédéric Péters 5 months ago

Je ne suis pas totalement à l'aise avec ça à deux jours de la mise en prod, ça demande un effort de relecture difficile.

En fait 1/ je ne prendrais pas 0001, qui concerne du code d'import/export, pas sensible, risque inutile.

0002 oui d'accord

0003 en fait ce que je voulais dire dans la discussion c'était qu'on pouvait sans risque déplacer le filtre sur les colonnes formdef_type/formdef_id vers l'SQL, mais garder l'appel à la méthode match() pour vérifier l'appartenance.

0003 en lisant j'ai l'impression qu'il fait l'inverse, et perd en chemin la vérification formdef_type/formdef_id.

Par ailleurs je regarde maintenant dans la db et je compte pour une seule démarche 579 vues personnalisées, donc je me dis que j'étais trop naïf à espérer nous en sortir avec juste formdef_type/formdef_id, que le reste est à prendre aussi.

Mais que peut-être ça ne sera pas assez et qu'en fait il y a quelque part de base un problème chez un utilisateur, qui à lui seul a 512 de ces 579 vues, toutes avec le même nom, et les slugs tous-les-dossiers, tous-les-dossiers-2, tous-les-dossiers-3... tous-les-dossiers-512.

Cet utilisateur aura encore des problèmes, il y a sans doute quelque chose dans l'UI qui pose problème.

Dans l'immédiat donc, j'éliminerais 0001 de la branche, 0002 ok (mais retirer la majuscule au préfixe du message de commit), 0003 ajouter le Equal('formdef_type', ...), Equal('formdef_id', ...) qui me semble manquer (dans le get_custom_views(); je voyais vaguement quelque chose comme ça, pas testé :

    def get_custom_views(self, criterias=None):
        for view in get_publisher().custom_view_class.select(
            clause=[
                Equal('formdef_type', self.formdef_class.xml_root_node),
                Equal('formdef_id', str(self.formdef_class.id)),
            ]
            + (criterias or [])
        ):
            if view.match(get_request().user, self.formdef):
                yield view
#11

Updated by Frédéric Péters 5 months ago

Pas à l'aise avec le get_by_url_slug introduit dans 0003, il ne discrimine pas sur l'id de l'agent connecté, ce que faisait l'enchainement actuel (boucle sur le résultat de get_custom_views() qui vérifie ça); j'ai modifié pour plutôt faire

-            for view in self.get_custom_views():
-                if view.get_url_slug() == component:
-                    return self.__class__(formdef=self.formdef, view=view)
+            view_slug = component
+            if view_slug.startswith('user-'):
+                view_slug = view_slug[5:]
+            for view in self.get_custom_views([Or([Equal('slug', view_slug), Equal('slug', component)])]):
+                return self.__class__(formdef=self.formdef, view=view)

i.e. on filtre sur slug = le slug attendu (et/ou sans le préfixe user-, si jamais on se trouvait avec des vues commençant vraiment par user-).

J'ai mis ça dans une branche wip/68324-custom_view-2 (qui retire aussi le 0001 et adapte les messages de commit).

#12

Updated by Frédéric Péters 5 months ago

C'est plus que limite côté calendrier mais c'est une optimisation qui aurait été bienvenue pour la mise en production de ce soir.

#13

Updated by Pierre Ducroquet 5 months ago

  • Status changed from Solution proposée to Solution validée

Frédéric Péters a écrit :

Pas à l'aise avec le get_by_url_slug introduit dans 0003, il ne discrimine pas sur l'id de l'agent connecté, ce que faisait l'enchainement actuel (boucle sur le résultat de get_custom_views() qui vérifie ça); j'ai modifié pour plutôt faire

[...]

i.e. on filtre sur slug = le slug attendu (et/ou sans le préfixe user-, si jamais on se trouvait avec des vues commençant vraiment par user-).

Dac. La justification pour moi de cette fonction était surtout de garder côte à côte la logique du get_url_slug() et du filtrage sur le slug.

Et détail tout à fait mineur dans un tel cas :

Or([Equal('slug', view_slug), Equal('slug', component)])

s'écrit plus avantageusement:
Contains('slug', [view_slug, component])

(plus court, et sur les cas plus complexe bien plus efficace pour le PG)

Pour ma part, vu le gain que ça procurerait pour toulouse, je suis largement favorable à ce qu'on fasse une exception ici.

#14

Updated by Frédéric Péters 5 months ago

  • Status changed from Solution validée to Résolu (à déployer)

modifié pour utiliser Contains,

commit 33f6acea6a6b1476fb08dea12c08daee9e005210
Author: Pierre Ducroquet <pducroquet@entrouvert.com>
Date:   Mon Aug 22 18:03:28 2022 +0200

    custom views: remove frequent calls to criteria-less select() (#68324)

commit 5f3deb17f702362d2e56f36850ec5477d1f0db96
Author: Pierre Ducroquet <pducroquet@entrouvert.com>
Date:   Mon Aug 22 15:55:48 2022 +0200

    custom views: index the formdef_id column we are going to use more (#68324)
#15

Updated by Transition automatique 5 months ago

  • Status changed from Résolu (à déployer) to Solution déployée
#16

Updated by Lauréline Guérin 4 months ago

  • Related to Bug #69699: Vue de traitement, vues personnalisées 'user' et 'all' avec le même slug added
#17

Updated by Transition automatique 3 months ago

Automatic expiration

Also available in: Atom PDF