Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cor_area_taxon et performances #997

Closed
hypsug0 opened this issue Aug 15, 2020 · 20 comments
Closed

cor_area_taxon et performances #997

hypsug0 opened this issue Aug 15, 2020 · 20 comments

Comments

@hypsug0
Copy link
Contributor

hypsug0 commented Aug 15, 2020

Les triggers qui peuplent cor_area_taxon sont extrèmement lourds sur de gros jeux de données et plombent littéralement les performances.

A titre d'exemple, j'ai été contraint d'interrompre une requête de suppression 2000 données dans ma synthèse (19 millions de lignes pour plus de 4000 taxons) après plus de 8h. Une fois les triggers supprimés, suppression effective en moins d'une minute.

Il en est de même sur les update, il y a quelques temps, je ne parvenais pas à peupler ma table cor_area_synthese (requête qui a tourné plus d'une semaine...). Supprimer les triggers " upsert" pour cor_area_taxon a réduit ce travail à quelques heures.

Une recherche sur les dépots montre que cor_area_taxon n'est utilisée que pour la vue d'API /color_taxon (table cor_area_taxon > vue v_color_area_taxon > model VColorAreaTaxon > route /color_taxon). Pour quel usage? l'appli mobile mais apparement plus utilisée dans la nouvelle appli occtax (utilisé dans l'ancienne appli mobile)?

D'où ma question "qui tue", Est-il pertinent de conserver cette table, en tout cas dans leur fonctionnement actuel avec triggers très lourds pour les performances ?

@hypsug0
Copy link
Contributor Author

hypsug0 commented Aug 15, 2020

En lien avec #617

@camillemonchicourt
Copy link
Member

Oui c'est peut-être améliorable et on se pose toujours la question d'alternative aux triggers mais pas encore trouvé car on travaille parfois directement dans la BDD.

Cette table est utile pour Occtax-mobile en effet et important à son fonctionnement.

En effet quand on localise dans l'application, cela indique pour chaque taxon si ils n'ont jamais été vu dans la zone. Et si ils ont déjà vu, alors combien de fois et depuis quand.
Donc cela permet d'orienter les observations sur les choses les plus intéressantes.

@TheoLechemia
Copy link
Member

Oui, il faut se poser la question de l'importance de instantanéité de l'info dans ce cas. Est-ce qu'une VM raffrachit à un certain pas de temps serait plus performante ?

@jpm-cbna
Copy link
Contributor

Qu'en est il aussi de la possibilité d'utiliser des triggers DEFERRABLE proposé dans le ticket #617 par @jbrieuclp ?

Pour importer plusieurs millions de données, je suis aussi obligé de désactiver ce trigger car il s'applique aussi aux ajouts. J’insère les données puis je le rejoue globalement. Or, le rejouer globalement le rend bien plus rapide.

Si en passant ce trigger en DEFERRABLE cela le déclenche globalement qu'en fin de transaction cela pourrait peut être résoudre notre problème ? Quelqu'un a t il déjà testé ce type de mécanisme sur un trigger ?

@camillemonchicourt
Copy link
Member

Oui si une fonction permet de les jouer en une fois à la fin et non pas ligne par ligne, ça nous aiderait fortement !

En attendant, il faut voir comment combiner des triggers ligne par ligne et des actions globales, sans dupliquer du code et garder une maintenance viable des actions et de la BDD.

@camillemonchicourt
Copy link
Member

Pour les triggers et le fait de faciliter leur désactivation et d'exécuter leur équivalent globalement, l'idéal serait de les baser au maximum sur des fonctions SQL, pour faciliter la maintenance de leur action et éviter de les dupliquer. Mais je ne sais pas dans quelle mesure cela est possible car le trigger travaille ligne par ligne ?

Et pour certains cas où on n'a pas besoin d'instantanéité, on peut imaginer en pas utiliser de trigger mais exécuter une action en cron à intervalle régulier.
Justement pour les couleurs des taxons, c'est suffisant de rafraîchir l'info toutes les heures ou tous les jours. Cette méthode est un peu plus fragile que les triggers et nécessite de mettre en place un CRON, mais elle évite les soucis de performances à chaque gros import de données.
Elle n'est pas viable pour les autres triggers déjà en place où on a besoin d'instantanéité. Et idem pour le calcul de la sensibilité à venir, où on ne peut pas attendre une heure ou un jour, on doit avoir l'info dès l'intégration dans la BDD.

@camillemonchicourt
Copy link
Member

Une alternative entre le trigger et le cron : un cron au niveau de la BDD ?
Voir https://www.citusdata.com/blog/2016/09/09/pgcron-run-periodic-jobs-in-postgres/
Mais là aussi, non viable pour les traitements pour lesquels on a besoin d'instantanéité.

@jpm-cbna
Copy link
Contributor

jpm-cbna commented Aug 20, 2020

Une autre piste à envisager serait de modifier les triggers pour qu'ils agissent non plus ligne par ligne mais pour l'ensemble des lignes modifiées lors d'une transaction.

Pour cela les triggers doivent être créés avec FOR EACH STATEMENT à la place de FOR EACH ROW.
Cela nous oblige a repenser ces triggers mais cela résoudrait peut être les problèmes de performance.

Depuis Postgresql v10, il est possible d'utiliser des tables de transition avec un trigger agissant au niveau STATEMENT.
Voir : https://www.postgresql.org/docs/current/plpgsql-trigger.html#PLPGSQL-TRIGGER-AUDIT-TRANSITION-EXAMPLE

Ressource : https://stackoverflow.com/questions/24193567/for-each-statement-trigger-example

@TheoLechemia
Copy link
Member

J'ai passé le nouveau trigger de sensibilité et le trigger de calcul des aires en "on each statement".
Voilà le retour des analyses de performances d'insertion dans la synthese:

-- sans trigger
-- 1000: 541ms
-- 10 000: 4s
-- 100 000: 47s

-- Avec trigger sensi on each statement
-- 1000 : 1.76s
-- 10 000: 16s
-- 100 000 2m 49

-- trigger area_synthese on each row initial (st_touches sur toutes les geom)
-- 1000: 2s
-- 10 000: 22s
-- 100 000: 3m34s

--trigger area_synthese on each statement (st_touches sur toutes les geom)
--- 1000: 2s
--- 10 000: 20s
-- 100 000: 3m30

-- triger area on each statement optimisé (st_touches seulement sur les geom non ponctuelles)
-- 1000: 826ms
-- 10 000: 9s
-- 100 000: 1m11s

-- tout trigger activé sauf cor_area_taxon
-- 1000: 1.43
-- 10 000: 12s
-- 100 000: 2m11s

-- tout trigger activé
-- 1000: 31s
-- 10 000 : ?
-- et exponentiel plus on a de données dans la synthese

Le "on each statement" ne semble pas faire gagner énormément de performances (ce qui est étrange). Mais c'est clairement le trigger de calcul de cor_area_taxon qui pose problème. Une solution serait de remplacer ce trigger par une vue materialisée (ce qui aurait aussi pour conséquence de pouvoir diminuer la taille de la synchronisation mobile en ne prenant que les aires et les taxons (?) utilisés)

@lpofredc
Copy link
Contributor

Merci @TheoLechemia d'avoir pris le temps de faire ces tests! très inscructif. En effet, personnellement, je pencherai plutôt pour une vue matérialisée.

@camillemonchicourt
Copy link
Member

Oui pour cor_area_taxon, on va passer sur une vue matérialisée car ça alourdit trop les intégrations inutilement.
Même si on se méfie de l'hétérogénéité et qu'avec une vue matérialisée, on maitrise moins que sa mise à jour est bien en place sur tous les serveurs.
Dans le module EXPORTS, @amandine-sahl a fait en sorte que le CRON soit mis en place automatiquement à l'installation, donc on va voir de ce côté.

@lpofredc
Copy link
Contributor

Et avec l'extension pg_cron, apparement native sur Buster (postgresql-11-cron) :
`https://github.com/citusdata/pg_cron

Cela éviterait un niveau supplémentaire d'administration (cron système).

@camillemonchicourt
Copy link
Member

Éventuellement mais il faut voir les conséquences en terme de dépendances et de maintenance.

On prévoit aussi d'imposer une version 10 minimum de PG à partir de la prochaine release pour supporter les triggers FOR EACH STATEMENT mais là ça imposerait la version 11 si je comprends bien. A creuser pour un peu plus tard je pense.

@camillemonchicourt
Copy link
Member

Après avoir fait des tests, @amandine-sahl propose même de n'en fait qu'une vue, OK en performances à ses premiers tests.
Ca éviterait de devoir faire un cron et les données seraient plus à jour.
Contre-partie, ça enverrait une requête potentiellement un peu lourde à chaque synchronisation de l'application Sync-mobile.
Mais je partirai sur ça dans un premier temps.

Vue de test :

CREATE OR REPLACE VIEW gn_synthese.cor_area_taxon
AS SELECT s.cd_nom,
    cas.id_area,
    count(DISTINCT s.id_synthese) AS nb_obs,
    max(s.date_min) AS last_date
   FROM gn_synthese.synthese s
     JOIN gn_synthese.cor_area_synthese cas ON cas.id_synthese = s.id_synthese
     JOIN ref_geo.li_grids lg ON lg.id_area = cas.id_area AND lg.zc = true
  GROUP BY s.cd_nom, cas.id_area;
  • A renommer en gn_synthese.v_area_taxon et adapter gn_synthese.v_color_taxon_area pour qu'elle tape dessus.
  • Ou alors directement faire qu'une seule vue en revoyant gn_synthese.v_color_taxon_area pour éviter une vue qui tape sur une vue
  • Revoir la route (sans la renommer) pour qu'elle tape dans la vue et non plus la table, en renvoyant le même contenu
  • Ajouter un paramètre dans la table gn_commons.t_parameters avec le code_area_type à utiliser comme filtre pour les unités géographiques utilisé par Occtax-mobile (Mailles 5 km par défaut). Par exemple nommé occtaxmobile_area_type

@camillemonchicourt
Copy link
Member

Les triggers ON EACH STATEMENT ne sont pas disponibles dans PostgreSQL 9. Or c'est la version 9 qui est fournie avec Debian 9. Depuis la version 2.5 de GeoNature, on ne supporte plus Debian 9 officiellement, mais on avait fourni une méthode pour continuer à utiliser Debia 9 (en forçant l'installation d'une version plus récente de Python).

Avec GeoNature 2.6.0 à venir on a besoin de PostgreSQL 10 minimum pour les triggers ON EACH STATEMENT.

J'ai tenté une mise à jour automatique de PostgreSQL en version 11 sur le serveur de DEMO qui est en Debian 9, ça passe bien, mais pour PostGIS c'est plus compliqué. A priori la mise à niveau automatique ne se fait pas toute seule. Il faut dumper les BDD spatiales et les restaurer dans la version mise à jour de PostgreSQL.

Pour la mise à jour de PostgreSQL 9 à 11 sur Debian 9, j'ai suivi https://praderas.org/en/upgrading-postgresql-11-debian-stretch, mais en utilisant aussi https://computingforgeeks.com/how-to-install-postgresql-11-on-debian-9-debian-8/ pour ajouter les sources lists avant de lancer l'installation de PostgreSQL 11.
Mais au moment de la migration j'ai eu des tonnes d'erreurs de restauration au moment de "sudo pg_upgradecluster -v 11 9.6 main" car il fallait installer PostGIS avant sur le serveur V11 et car il semble ne pas pouvoir migrer les BDD spatiales automatiquement.
Voir : https://www.rigacci.org/wiki/doku.php/doc/appunti/linux/sa/postgresql_upgrade

Du fait qu'on utilise PostGIS, c'est plus complexe qu'une migration de version de PostgreSQL.

De mon côté je n'ai pas réussi, donc je ne suis pas sur qu'on puisse indiquer une méthode assez simple et fiable.
Il faudrait donc imposer le passage à Debian 10 pour pouvoir passer sur GeoNature version 2.6.0 ?

@TheoLechemia
Copy link
Member

Oui c'est dans la logique des choses. Plutôt que de forcer des versions non officiellement supportées par la distrib, on impose le passage à Debian 10.

@camillemonchicourt
Copy link
Member

OK la 2.5.0 préparait déjà le terrain dans ce sens. 😀

@bouttier
Copy link
Contributor

Remplacement par une vue initié dans #1201.

À terme la vue gn_synthese.v_color_taxon_area est amené à disparaître, le calcul de la couleur relevant du front. Ainsi, il ne m’a pas semblé très utile d’essayer d’éviter d’avoir une vue sur une vue.

@bouttier
Copy link
Contributor

bouttier commented Dec 23, 2020

J’ai rajouté le filtrage pour un type de maille donnée avec le paramètre occtaxmobile_area_type qui a M5 pour valeur initiale.

Note : il n’y a pas de route permettant d’accéder à la vue v_area_taxon, mais seulement une route permettant d’accéder à la vue v_color_area_taxon (cette dernière ayant été mise-à-jour pour taper sur v_area_taxon au lieu de cor_area_taxon).

@camillemonchicourt
Copy link
Member

La table et son trigger ont été remplacés par une vue dans la table.
Pour limiter les couleurs de taxon à calculer par unité géographiques, on filtre sur un seul type de zonage, grâce au nouveau paramètre occtaxmobile_area_type dans la table gn_commons.t_parameters.

Si vous utilisez Occtax-mobile, à remplir avec le code de type de zonage que vous souhaitez utiliser pour les unités géographiques.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants