DISIC / observatoire

🕵️‍♀️ Observatoire de la qualité des démarches en ligne
https://observatoire.numerique.gouv.fr/
GNU Lesser General Public License v3.0
10 stars 4 forks source link

Limiter le nombre d'avis exportés pour les démarches à forte volumétrie #1188

Closed lucaa closed 1 year ago

lucaa commented 1 year ago

Cette tâche concerne la fonctionnalité d'export des avis d'une démarche, disponible en bas de l'ecran avec la liste des avis d'une démarche.

Initialement cet export était limité à 500 avis, pour des raisons de performance. Ensuite, la fonctionnalité a été refaite et cette limite a été enlevée , voir #882 .

Par contre, aujourd'hui pour certaines démarches on arrive à la limite de cette nouvelle implémentation aussi. Ainsi, il faudrait, au moins jusqu'à l'implémentation de #1110 , réinstaurer une limite pour éviter les perturbations de performance du serveur lié à un export fait sur une de ces démarches.

Cette limite sera, évidemment, beaucoup plus élevée que 500 avis, et pourra être différente pour l'export csv vs. export excel. D'ailleurs, pour l'export excel on pourrait tout simplement ne pas proposer l'option si le nombre d'avis est trop élevé.

AnthonyBrunelli commented 1 year ago

Pour des raisons de performances, il faut limiter les exports excel à 50.000 avis et les exports CSV à 150.000 avis. Donc :

lucaa commented 1 year ago

note: à faire #1097 avec aussi.

lucaa commented 1 year ago

Lors de la mise en place du nouveau stockage d'avis et correctif de #1110 , une limite a été ajoutée pour l'export excel, à 65 000 avis, car le format excel utilisé par l'observatoire a une limite du nombre de lignes à 65535.

Il faudrait vérifier si, avec le nouveau stockage, l'export CSV a encore besoin d'être limité ou pas - des améliorations importantes ont été apportées par #1194 (#1110).

lucaa commented 1 year ago

@raphj @ldubost quels sont les résultats de vos tests pour l'export CSV en terme de temps et mémoire consommée lors d'un export massif (400 000 lignes)?

raphj commented 1 year ago

@raphj @ldubost quels sont les résultats de vos tests pour l'export CSV en terme de temps et mémoire consommée lors d'un export massif (400 000 lignes)?

J'ai testé avec 500 000 avis sur perf-staging. La requête SQL a pris autour de 5 minutes à s'exécuter mais l'export échoue au bout de 20 minutes. Le serveur semble galérer et beaucoup ralentir au dessus de 200 000 lines et l'export finit par échouer.

Avec 211182 entrées, l'export s'est bien passé. La requête SQL a pris autour de 4 minutes et l'export complet a pris autour de 6 et 7 minutes.

raphj commented 1 year ago

J'ai limité l'export CSV à 150 000 lignes comme initialement demandé, ça a l'air d'être une valeur raisonnable. C'est facilement paramétrable.

Voici le rendu (avec des valeurs plus petites pour les tests, je n'ai pas autant d'avis sur ma copie locale de travail) :

https://user-images.githubusercontent.com/3817365/210392337-9bec4531-2702-4cb8-801c-2d36177f50db.mp4

lucaa commented 1 year ago

Testé pendant les vacances, l'export des 235 000 avis échoue, avec le même constat que @raphj sur le ralentissement.

lucaa commented 1 year ago

Aussi, aujourd'hui il est possible de lancer un export excel pendant qu'un export CSV est déjà en cours (et viceversa) à partir de la même page.

Contrairement à ma première impression cela ne lance pas vraiment un deuxième export en parallèle (il y a un message affiché qui annonce qu'un export est en cours et que l'export va être lancé dès que l'autre est fini). Ceci s'applique également à des exports à des avis pour des démarches différentes, sur des pages différentes: image

Ce comportement est correct, il faut le garder.

Par contre, il faut améliorer le cas où ces 2 exports sont lancés depuis la même page:

lucaa commented 1 year ago

Suite à la mise en place des limites dynamiques par @raphj , il y a une discordance entre les messages affichés: dans la confirmation d'export (lors du click), le comptage réel est utilisé alors que l'export se fait dans les limites posées:

image

(capture obtenue avec des fausses limites mises en place pour des tests)

lucaa commented 1 year ago

J'ai simplifié l'implementation de @raphj pour utiliser le paramètre "limit" de la macro livetables exporter. Ainsi, le problème de message de confirmation ci-dessus devient un bug de cette macro : https://jira.xwiki.org/browse/XALTXLS-29 .

lucaa commented 1 year ago

le bug a été corrigé dans la version 4.0.1 de l'extension livetable exporter et les instructions de ce projet ont été mises à jour pour noter cette nouvelle version requise.

lucaa commented 1 year ago

Aussi, aujourd'hui il est possible de lancer un export excel pendant qu'un export CSV est déjà en cours (et viceversa) à partir de la même page. il faut améliorer le cas où ces 2 exports sont lancés depuis la même page [...] (voir la demande complète dans https://github.com/DISIC/observatoire/issues/1188#issuecomment-1397128470 )

Techniquement il est plutôt complexe de détecter cette situation, ainsi, pour l'instant, je ne mets pas en place cette amélioration supplémentaire. On peut étudier le comportement des utilisateurs à l'export suite à la mise en place des limites et revenir sur cette amélioration avec un nouveau ticket, si nécessaire .

lucaa commented 1 year ago

La limite d'export a été installée sur le serveur de production.