etalab / transport-site

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

Amélioration des jobs périodiques : constat et quelques idées #3641

Open vdegove opened 10 months ago

vdegove commented 10 months ago

Le cycle de vie des datasets et resources, via les jobs associés, est difficile à comprendre, voir commentaire ici https://github.com/etalab/transport-site/pull/3632#issuecomment-1835834268 :

Une partie des jobs est lancée depuis le crontab du fichier apps/transport/lib/transport/scheduler.ex (d’ailleurs ce ne sont pas des jobs Oban juste des modules classiques), l’autre depuis le crontab de config/runtime.exs

On s’est retrouvés sur un bug difficile à comprendre, parce que un job de vérification de la disponibilité enregistrait un truc en BDD, puis un autre job envoyait une notification, puis enfin le job d’import écrasait la valeur écrite par le job de vérification de la disponibilité.

Quelques idées d’amélioration :

  1. Documenter tout ça :)
  2. Rassembler tous les jobs dans un crontab unique
  3. Peut-être fusionner des jobs dans un job unique qui fait plusieurs choses.
  4. Utiliser davantage le schedule de jobs

1. Documenter tout ça :)

Je peux le faire :grin: faut juste me dire où.

2. Crontab unique

Le fait qu’il y ait deux crontabs n’aide pas à la compréhension, on pourrait supprimer le scheduler et transformer les fonctions qui y sont listées en jobs, et au passage aussi les renommer, par exemple Transport.DataChecker,.outdated_data/0 ne fait que envoyer des emails, donc ça gagnerait à être réécrit au standard xxxNotificationJob.

3 Fusion des jobs

Le fond du problème de l’issue #3630, c’est peut-être qu’on a un peu trop découpé les choses : on a un 1er job de synchro périodique de datagouv qui va écrire des URL en BDD ; un 2ème job qui vérifie plus tard que ces URL sont bien dispos, si ce n’est pas le cas et que l’URL a changé, écrit la nouvelle en BDD, et sinon écrit une indispo en base de données ; et un 3ème job qui envoie des emails à partir des indispos écrites en BDD.

Puisque de toutes façons, on va aller taper sur les serveurs de datagouv pour vérifier périodiquement que les URL sont bonnes, pourquoi ne pas simplement faire la synchro du dataset plus régulièrement, et au même moment vérifier la disponibilité de l’URL finale ?

Autre question : pourquoi envoyer les notifications dans des jobs séparés de ceux qui vérifient l’indisponibilité / la validité / etc ? Réponse de ma part : peut-être parce que les mails sont groupés par producteur, et aussi qu’on a envie de vérifier certaines choses fréquemment et n’envoyer que des emails de récapitulation.

4. Utilisation du schedule de jobs

Puisque le crontab est assez dur à suivre pour s’assurer que les choses sont bien chaînées dans le bon ordre, comprendre à quelle heure et quand sont faites les choses, est-ce qu’il n’y a pas des jobs qui pourraient être regroupés dans un meta-job qui lui-même va scheduled des jobs dans le futur ? Avec MyJob.new(scheduled_at: blah) |> Oban.insert()

vdegove commented 6 months ago

Le scheduler historique dans apps/transport/lib/transport/scheduler.ex, s’appuie sur Quantum https://hex.pm/packages/qui lui-même s’appuie sur Crontab https://hex.pm/packages/crontab dont la dernière release date de 2022, et qui déclenche des warnings à la compilation dans Elixir 1.16, voir https://github.com/etalab/transport-site/pull/3815, si le problème n’est pas résolu en amont, c’est une raison additionnelle pour déprécier ce scheduler.