etalab / transport-site

Rendre disponible, valoriser et améliorer les données transports
https://transport.data.gouv.fr
192 stars 30 forks source link

Evaluer la possibilité d'augmenter la limite de taille sur traitement GTFS-diff #3738

Closed thbar closed 7 months ago

thbar commented 8 months ago

Voir:

Le chargement de fichiers GTFS est limité à 10 Mo dans le GTFS Diff - comparaison de fichiers GTFS. Est-ce qu'il serait possible de repousser cette limite à 20 Mo ? Je souhaite utiliser cet outil de comparaison pour les besoins de la Semitan (TC Nantes Métropole) et les zip des gtfs actuels font entre 15 et 20 Mo).

Il faut voir si augmenter la limite ne pose pas de problème opérationnel en l'état (car l'implémentation actuelle est gourmande, c'est une limitation connue).

Reproduction

Comparer le fichier suivant avec lui même:

CleanShot 2024-01-24 at 16 56 36@2x
thbar commented 8 months ago

Je documente ici ma recherche.

La limite est actuellement fixée ici:

https://github.com/etalab/transport-site/blob/c04ccef4dce2c4579380f72f23a4e1d5639f8340/apps/transport/lib/transport_web/live/gtfs_diff_select_live.ex#L11

Le traitement est enqueué ici suite à un upload vers le bucket S3, et cela se fait aujourd'hui via un job Oban:

https://github.com/etalab/transport-site/blob/c04ccef4dce2c4579380f72f23a4e1d5639f8340/apps/transport/lib/transport_web/live/gtfs_diff_select_live.ex#L58-L65

Job qui tourne normalement sur le worker.

A priori du coup le gros de l'accroissement de consommation mémoire se fera plutôt sur le worker, noeud séparé. Il peut y avoir un impact sur le frontal (prod-site) si la taille de la payload augmente, mais le coût majoritaire ne sera a première vue pas là.

thbar commented 8 months ago

J'ai fait un test en local avec:

Je vois que ça semble consommer jusqu'à pas loin de 4GB, et l'instance worker est à 4GB (pour ce que la comparaison vaut).

Je n'ai pas pu aller jusqu'au bout du test pour un petit souci de paramétrage de la fin avec MinIO, je corrigerai ça.

Je pense qu'on pourra tenter de déployer ça en production, mais d'observer ce qui se passe au niveau RAM (crash ou pas), à discuter, et ça va être limite au minimum dans certaines situations (autres choses qui tournent etc).

thbar commented 8 months ago

La question sous-jacente c'est celle de l'optimisation de cette implémentation qui se voulait temporaire, mais qui n'est pas scalable (et qui pourrait être modifiée pour l'être je pense).

thbar commented 8 months ago

On va déployer un hot-fix et je testerai (et commenterai ici) sur le résultat.

Ca peut tout à fait faire planter le worker de façon systématique, mais on verra bien, l'impact est acceptable et ce test permettra d'être sûr du comportement.

thbar commented 8 months ago

Après avoir déployé:

J'ai procédé à un test en comparant le fichier https://data.nantesmetropole.fr/explore/dataset/244400404_tan-arrets-horaires-circuits/table/ à lui-même en production.

J'ai surveillé la mémoire libre via l'url de health-check, et vu que c'était assez stable une bonne partie du temps, puis c'est passé à 300MB de disponible, et ça a crashé.

On retrouve cette info sur le dashboard CleverCloud:

CleanShot 2024-01-25 at 15 03 43@2x

C'est géré assez "joliment" on va dire par le site, qui affiche "diff is taking too long":

CleanShot 2024-01-25 at 15 04 09@2x

Au final ça ne fonctionne pas.

Les solutions à ce stade:

  1. Augmenter la RAM sur prod-worker : passer de M (4GB, 76€ par mois) à L (8GB, 152€ par mois, soit un surcoût de 76€ par mois) -> solution la plus immédiate, ça me paraît une bonne idée pour temporiser sur la solution 2. plus pérenne (il faut vérifier que ça fonctionne bien :smile: dans la réalité)
  2. Revoir la copie sur l'algorithme - sur le trimestre ça me semble coûteux à faire, il faudra déjà faire un petit diagnostic pour examiner ce qui est le plus coûteux, j'ai quelques techniques pour faire ça si on veut aller dans cette direction

J'en parle avec l'équipe.

thbar commented 8 months ago

J'ai été voir l'algorithme un peu en détail (voir https://github.com/etalab/transport-site/blob/master/apps/transport/lib/transport/gtfs_diff.ex).

Il n'est pas optimisable simplement, il faudra optimiser plusieurs endroits, et streamer également. Je vois comment on peut aller vers ça, mais ce n'est pas un travail d'un après-midi (sauf vrai coup de bol, à regarder de plus près et détecter un raccourci sur une sous-partie éventuellement, en profilant chaque partie de façon indépendante).

Donc je préconise plutôt une augmentation de la taille dans l'immédiat, pour pouvoir quand même répondre à la demande sur des fichiers un petit peu plus gros.

thbar commented 8 months ago

Je regarde quels sont les fichiers impactés potentiellement pour me rendre compte de ce qui est mis de côté actuellement:

WITH sizes AS (
    SELECT DISTINCT ON (resource_id)
        (payload ->> 'total_compressed_size')::integer AS file_size,
        resource_id
    FROM
        resource_history
    WHERE
        payload ->> 'format' = 'GTFS'
        AND resource_id IS NOT NULL
    ORDER BY
        resource_id,
        file_size DESC
)
SELECT
    pg_size_pretty(file_size::numeric),
    ('* https://transport.data.gouv.fr/resources/' || resource_id)
FROM
    sizes
ORDER BY
    file_size DESC
thbar commented 8 months ago

Le résultat sur la réplica de production à l'instant, en ne gardant que les ressources de + de 10MB:

pg_size_pretty ?column?
101 MB * https://transport.data.gouv.fr/resources/80921
71 MB * https://transport.data.gouv.fr/resources/78904
56 MB * https://transport.data.gouv.fr/resources/11681
56 MB * https://transport.data.gouv.fr/resources/81330
52 MB * https://transport.data.gouv.fr/resources/79818
51 MB * https://transport.data.gouv.fr/resources/81514
50 MB * https://transport.data.gouv.fr/resources/80937
48 MB * https://transport.data.gouv.fr/resources/81516
47 MB * https://transport.data.gouv.fr/resources/40287
46 MB * https://transport.data.gouv.fr/resources/81559
40 MB * https://transport.data.gouv.fr/resources/81091
38 MB * https://transport.data.gouv.fr/resources/79420
38 MB * https://transport.data.gouv.fr/resources/79127
37 MB * https://transport.data.gouv.fr/resources/81081
31 MB * https://transport.data.gouv.fr/resources/79005
27 MB * https://transport.data.gouv.fr/resources/8055
26 MB * https://transport.data.gouv.fr/resources/81026
24 MB * https://transport.data.gouv.fr/resources/80721
24 MB * https://transport.data.gouv.fr/resources/81090
20 MB * https://transport.data.gouv.fr/resources/80694
17 MB * https://transport.data.gouv.fr/resources/81391
16 MB * https://transport.data.gouv.fr/resources/79642
14 MB * https://transport.data.gouv.fr/resources/65812
13 MB * https://transport.data.gouv.fr/resources/81218
13 MB * https://transport.data.gouv.fr/resources/78961
13 MB * https://transport.data.gouv.fr/resources/65813
10180 kB * https://transport.data.gouv.fr/resources/80960
thbar commented 8 months ago

(tableau à prendre avec des pincettes, j'ai vu des différences avec la réalité sur certains cas).

thbar commented 8 months ago

J'ai proposé à @vdegove et @AntoineAugusti de tenter un passage de 4GB à 8GB et on est tous OK.

Malheureusement ça n'a pas suffi et ça a crashé.

Je vais tenter de remettre en place du swap (en demandant à CleverCloud), et refaire un test.

Si ce n'est pas bon, je retournerai sur 4GB et il faudra voir ce qu'on fait.

thbar commented 8 months ago

En augmentant le swap via le support CleverCloud, ce qu'on peut voir à présent ici:

https://workers.transport.data.gouv.fr/health-check/metrics

CleanShot 2024-01-27 at 09 31 46@2x

le worker ne plante plus sur un premier test.

Toutefois le message "Job aborted, the diff is taking too long (> 5.0 min)." apparaît.

Je suis en train de préparer un correctif que je déploierai.

thbar commented 8 months ago

Ça a été au bout...

CleanShot 2024-01-27 at 10 35 20@2x

À prendre avec des pincettes tout ça ne sera pas super solide, mais je préviendrai l'utilisateur.

thbar commented 8 months ago

Par contre le swap semble être à zéro, je vais voir si ça évolue.

CleanShot 2024-01-27 at 10 36 49@2x

thbar commented 7 months ago

Pour le moment on met de côté. Il faudrait une ré-écriture de l'algorithme.