Development #60469
Requête lente récurrente sur WCS
0%
Description
En regardant les logs nouvellement mis en place sur lille-prod, je me rends compte qu'une requête est souvent réalisée sous une forme qui ne peut être indexée (et qui de toute façon utilise des données non indexées).
Requête sur lille-prod:
SELECT id FROM formdata_113_demander_mon_pass_lillemoi WHERE '906c88eff7e047c5af891cf2e6b19c64' = ANY (concerned_roles_array);
=> 200 à 300ms.
Or, on peut faire mieux:
create index concurrently test_pierre on formdata_113_demander_mon_pass_lillemoi using gin(concerned_roles_array);
SELECT id FROM formdata_113_demander_mon_pass_lillemoi WHERE concerned_roles_array @> ARRAY['906c88eff7e047c5af891cf2e6b19c64'];
Le select prend alors une milliseconde environ. L'index pèse 136kB pour une table de 364MB, ce qui est tout à fait acceptable je pense.
Je ne maîtrise pas du tout wcs, mais j'ai pu identifier comment corriger la requête, il me manque juste la partie migrations:
diff --git a/wcs/sql.py b/wcs/sql.py
index bb8cbe52..78f2213d 100644
--- a/wcs/sql.py
+++ b/wcs/sql.py
@@ -2329,7 +2329,7 @@ class SqlDataMixin(SqlMixin):
value = str(value)
if '%s_array' % index in [x[0] for x in cls._table_static_fields]:
- sql_statement = '''SELECT id FROM %s WHERE %%(value)s = ANY (%s_array)''' % (
+ sql_statement = '''SELECT id FROM %s WHERE %s_array @> ARRAY[%%(value)s]''' % (
cls._table_name,
index,
)
Fichiers
Révisions associées
sql: add and use indexes on concerned_roles_array and actions_roles_array (#60469)
After reading some slow queries log in lille-prod, I noticed these two columns
had no usable index (and their queries were not able to use them anyway).
This patch fixes it.
Historique
Mis à jour par Frédéric Péters il y a environ 2 ans
Je ne maîtrise pas du tout wcs, mais j'ai pu identifier comment corriger la requête, il me manque juste la partie migrations:
Il y a une méthode do_formdef_indexes
pour la création des index.
Dans wcs/sql.py, def migrate()
reprend l'ensemble des migrations, il y a un niveau global SQL_LEVEL, à monter, et ça détermine ensuite ce qu'il y aura à exécuter, ici ça donnerait a priori
@@ -752,6 +752,10 @@ def do_formdef_indexes(formdef, created, conn, cur, concurrently=False): % {'create_index': create_index, 'table_name': table_name, 'attr': attr} ) + cur.execute( + # quelque chose + ) + @guard_postgres def do_user_table(): @@ -3499,7 +3503,7 @@ def get_period_total( # latest migration, number + description (description is not used # programmaticaly but will make sure git conflicts if two migrations are # separately added with the same number) -SQL_LEVEL = (55, 'update full text normalisation (switch to unidecode)') +SQL_LEVEL = (56, 'add gin indexes to concerned_roles_array') def migrate_global_views(conn, cur): @@ -3638,7 +3642,7 @@ def migrate(): # 51: add index on formdata blockdef fields # 55: update full text normalisation (switch to unidecode) set_reindex('formdata', 'needed', conn=conn, cur=cur) - if sql_level < 46: + if sql_level < 56: from wcs.carddef import CardDef from wcs.formdef import FormDef @@ -3646,6 +3650,7 @@ def migrate(): # 35: add indexes on formdata(receipt_time) and formdata(anonymised) # 36: add index on formdata(user_id) # 45 & 46: add index on formdata(status) + # 56: add gin indexes to concerned_roles_array for formdef in FormDef.select() + CardDef.select(): do_formdef_indexes(formdef, created=False, conn=conn, cur=cur) if sql_level < 32:
(aussi, peut-être ça vaut le coup de mettre le même index sur actions_roles_array)
Mis à jour par Pierre Ducroquet il y a environ 2 ans
- Fichier 0001-Add-and-use-indexes-on-concerned_roles_array-and-act.patch 0001-Add-and-use-indexes-on-concerned_roles_array-and-act.patch ajouté
- Statut changé de Nouveau à Solution proposée
- Assigné à mis à Pierre Ducroquet
- Patch proposed changé de Non à Oui
Mis à jour par Thomas Noël il y a environ 2 ans
Sur le contenu du patch, ça me semble bel et bon.
Pour le message de commit, indique que ça concerne le module sql. Et on met pas de majuscule, c'est comme ça, donc plutôt :
sql: add and use indexes on concerned_roles_array and actions_roles_array (#60469)
Ensuite, créer une branche wip/60469-add-indexes (ce qui est important c'est que son nom commence par wip/NNNNN où NNNNN est le numéro du ticket) et y pousser ce patch.
Jenkins va alors faire tourner les tests sur cette branche (ça sera visible dans https://jenkins.entrouvert.org/job/wcs-wip/). En cadeau il y aura en bas de ce ticket un petit javascript qui en montrera le résultat (vert/rouge).
Mis à jour par Thomas Noël il y a environ 2 ans
- Statut changé de Solution proposée à En cours
Crash parce que do_formdef_indexes est appelé sur les nouvelles tables alors que les colonnes n'existent pas encore. Désolé de ne pas avoir vu ça.
Sur le salon, proposition de Fred (à intégrer au patch) :
@@ -540,6 +540,7 @@ def do_formdef_tables(formdef, conn=None, cur=None, rebuild_views=False, rebuild AND table_name = %s''', (table_name,), ) + new_table = False if cur.fetchone()[0] == 0: cur.execute( '''CREATE TABLE %s (id serial PRIMARY KEY, @@ -564,7 +565,7 @@ def do_formdef_tables(formdef, conn=None, cur=None, rebuild_views=False, rebuild formdata_id integer REFERENCES %s (id) ON DELETE CASCADE)''' % (table_name, table_name) ) - do_formdef_indexes(formdef, created=True, conn=conn, cur=cur) + new_table = True cur.execute( '''SELECT column_name FROM information_schema.columns @@ -705,6 +706,9 @@ def do_formdef_tables(formdef, conn=None, cur=None, rebuild_views=False, rebuild # them even if not asked to. redo_views(conn, cur, formdef, rebuild_global_views=rebuild_global_views) + if new_table: + do_formdef_indexes(formdef, created=True, conn=conn, cur=cur) + if own_conn: conn.commit() cur.close()
Mis à jour par Pierre Ducroquet il y a environ 2 ans
- Fichier 0001-sql-add-and-use-indexes-on-concerned_roles_array-and.patch 0001-sql-add-and-use-indexes-on-concerned_roles_array-and.patch ajouté
- Statut changé de En cours à Solution proposée
Mis à jour par Thomas Noël il y a environ 2 ans
- Statut changé de Solution proposée à Solution validée
Cette fois ci je valide en étant totalement sûr de moi (tousse tousse).
Attention cependant, ne pas pousser dans main, on est en "semaine de release".
Je suis du genre conservateur et je préfère éviter, et attendre plutôt vendredi ou lundi (ie après la release de ce 2ème jeudi du mois). Sauf si Frédéric est plus confiant que moi et a envie de ce patch right now (mais il est largement aussi conservateur que moi).
Mis à jour par Frédéric Péters il y a environ 2 ans
Yep une fois que c'est bon dans jenkins on attend vendredi.
Mis à jour par Pierre Ducroquet il y a environ 2 ans
- Fichier 0001-sql-add-and-use-indexes-on-concerned_roles_array-and.patch 0001-sql-add-and-use-indexes-on-concerned_roles_array-and.patch ajouté
- Statut changé de Solution validée à Solution proposée
J'ai changé le nommage de l'index, c'est trop long actuellement. Puis j'en ai profité pour utiliser "IF NOT EXISTS", c'est présent depuis PostgreSQL 9.5 tout de même...
Comme le nom de la table commence par un discriminant, j'ai mis le nom de la table en dernier élément pour que comme ça ce soit ce nom qui soit tronqué.
Mis à jour par Thomas Noël il y a environ 2 ans
Ok pour la longueur, bien vu
Pour pinailler, sur le "IF NOT EXISTS", comme on ne l'utilise pas encore ailleurs, par mimétisme j'aurais bien vu le code rester avec un bêbête « if ... not in existing_indexes: » dans un premier temps.
... et donc un autre ticket-patch "utiliser IF NOT EXISTS dans dans la création des index" (do_formdef_indexes, do_loggederrors_table).
Mis à jour par Pierre Ducroquet il y a environ 2 ans
Thomas Noël a écrit :
Ok pour la longueur, bien vu
Pour pinailler, sur le "IF NOT EXISTS", comme on ne l'utilise pas encore ailleurs, par mimétisme j'aurais bien vu le code rester avec un bêbête « if ... not in existing_indexes: » dans un premier temps.
... et donc un autre ticket-patch "utiliser IF NOT EXISTS dans dans la création des index" (do_formdef_indexes, do_loggederrors_table).
J'ai choisi de le faire comme ça pour ne pas réimplémenter en Python la limitation en longueur des noms de relations dans PG. Je prévoyais de faire un patch ultérieur pour simplifier la création des index et utiliser IF NOT EXISTS à la place. Après si vous préférez qu'on fasse par mimétisme d'abord puis qu'on modernise tout d'un coup... As you wish (lorraine).
Mis à jour par Thomas Noël il y a environ 2 ans
- Statut changé de Solution proposée à Solution validée
Pierre Ducroquet a écrit :
J'ai choisi de le faire comme ça pour ne pas réimplémenter en Python la limitation en longueur des noms de relations dans PG. Je prévoyais de faire un patch ultérieur pour simplifier la création des index et utiliser IF NOT EXISTS à la place. Après si vous préférez qu'on fasse par mimétisme d'abord puis qu'on modernise tout d'un coup... As you wish (lorraine).
C'est malin d'écrire ça alors que j'ai faim.
Ok, allons-y ainsi, ta méthode est la bonne. Je vais un autre ticket pour moderniser les autres. Je pense qu'on est bien en 9.6 partout donc ça va passer.
(encore une fois, juste pour rappel, attendre vendredi pour pousser cela)
Mis à jour par Thomas Noël il y a environ 2 ans
Thomas Noël a écrit :
Je vais un autre ticket pour moderniser les autres.
Et ça, c'est fait → #60507
Mis à jour par Pierre Ducroquet il y a environ 2 ans
Thomas Noël a écrit :
(encore une fois, juste pour rappel, attendre vendredi pour pousser cela)
Du coup, on est vendredi, je peux pousser dans main ?
Mis à jour par Pierre Ducroquet il y a environ 2 ans
- Statut changé de Solution validée à Résolu (à déployer)
Et c'est pushèd
Mis à jour par Frédéric Péters il y a environ 2 ans
- Statut changé de Résolu (à déployer) à Solution déployée
Mis à jour par Pierre Ducroquet il y a environ 2 ans
- Statut changé de Solution déployée à Fermé
On a diminué considérablement le nombre de requêtes de plus de 200ms sur l'instance lille-prod (où j'avais observé le problème initialement).
Il y a d'autres cibles faciles qui apparaissent désormais, pour de futurs tickets.
Mis à jour par Emmanuel Cazenave il y a environ 2 ans
Pierre Ducroquet a écrit :
Et c'est pushèd
Une fois que tu as poussé, notre usage est d'inclure dans le ticket une référence au commit, façon #60675#note-6.
Mis à jour par Pierre Ducroquet il y a environ 2 ans
commit 0e8868dd5dac75a9823a2977dc9bf3a9c76aba71 Author: Pierre Ducroquet <pducroquet@entrouvert.com> Date: Mon Jan 10 18:01:08 2022 +0100 sql: add and use indexes on concerned_roles_array and actions_roles_array (#60469)
Add and use indexes on concerned_roles_array and actions_roles_array (#60469)
After reading some slow queries log in lille-prod, I noticed these two columns
had no usable index (and their queries were not able to use them anyway).
This patch fixes it.