Optimisation de la recherche par modes

thbar commented 1 week ago

Intro

DRAFT

Suite à :

Et en lien avec :

https://github.com/etalab/transport-site/issues/3640

La pagination / recherche actuelle prend jusqu'à 20 secondes par requête, en monopolisant le pool Ecto, ce qui crée actuellement un risque opérationnel sur tout le site (downtime l'autre jour, 5k+ erreurs).

Cette PR optimise fortement la recherche par modes, et s'appuie sur le fait que:

la recherche actuelle va chercher (via des joins additionnels), à chaque requête utilisateur, des informations qui n'évoluent qu'à chaque nouvelle version de [ressource / metadata de validation associée]
on peut donc pré-calculer ces informations (a minima de façon quotidienne, ou plus finement lors du changement de "metadata associée" si on voulait affiner)

Le principe général est d'avoir un bout de code qui va aller faire cette pré-calc, et d'ajouter un champ spécifique counter_cache sur chaque ressource pour noter le résultat du calcul (et éventuellement dedans des informations d'invalidation type "metadata id" si on le souhaite).

Cette payload est structurée comme suit, de façon à permettre un peu de flexibilité et d'ajout si besoin:

{
  "gtfs_json": ["bus", "ferry"]
}

Une fois le cache calculé, le temps passe sur un exemple (région + modes, voir plus bas) de 11 secondes à 50 millisecondes environ.

Requêtes testées en local

J'ai réalisé une implémentation "side by side" temporaire avec un paramètre modes_v2 (qu'on fera sauter avant merge) pour faciliter le travail de vérification / recettage (non régression fonctionnelle, comparaison de la perf).

Version actuelle

http://localhost:5000/datasets/region/14?modes%5B%5D=bus (une des plus coûteuses)
http://localhost:5000/datasets?modes%5B%5D=ferry

Version optimisée

Liste des tâches

Script initial

[x] Extraire le pré-calcul dans un module dédié et vérifier son bon fonctionnement (script expérimental)
[x] Créer une structure en base pour accueillir le résultat
[x] Mettre à jour les ressources via du code

Début d'usage

[x] Voir comment cabler le filtrage via le web
[x] Faciliter le test comparatif (v1 vs v2)

Mise au propre

[x] Créer la PR et une première version de documentation pour faciliter le travail collaboratif
[x] Déplacer le code de calcul dans la codebase
[x] Ajouter des tests avec un peu de data
[x] Créer un job pour le pré-calcul
[x] Scheduler le job (le matin ? 6h ? après les historisations ?)
~~Voir si on veut ou pas quelque chose de plus fin (ex: une nouvelle resource history -> pub sub pour rafraichir la pré-calc et éviter un délai confusant pour les utilisateurs ?)~~ On assume le lag de 5 minutes
[x] Déployer sur prochainement
[x] Correction CI
[x] Créer un test-case pour reproduire les erreurs pertinentes (ne pas prendre modes vs modes_v2, mais le reste oui)
[x] Corriger ces erreurs après
[x] Documenter le SQL généré avec values (pour faciliter l'apprentissage de ce pattern)
[x] Comparer avant-après: vérifier la non-régression fonctionnelle, et mesurer les temps
[x] Remplacer modes par modes_v2 et nettoyer le code
[x] Revue de code (hors @thbar et @vdegove)
[x] Déployer en production
[x] Suivre Sentry pour détecter des régressions
[x] Aller voir dans AppSignal le changement

thbar commented 1 week ago

@vdegove has entered the chat and is teaming with @thbar on PR #4006

ptitfred commented 1 week ago

Il s'agit davantage d'une indexation que d'un cache non ?

thbar commented 1 week ago

Il s'agit davantage d'une indexation que d'un cache non ?

Cela dépend de comment tu définis chaque terme je dirais.

Pour moi une indexation est plutôt un système qui va "pointer du doigt" vers ailleurs, là où est la donnée, tandis qu'un cache va "copier la donnée" (ou une partie de la donnée), et la rendre directement disponible à la source (ce qu'on fait ici).

Les deux termes ayant aussi de l'overlap (dans une certaine mesure, les deux peuvent devenir "stale", toutefois en général l'index est conservé à jour de façon plus immédiate pour que ça fonctionne, et le cache ça peut être un peu moins le cas).

thbar commented 1 week ago

GG @vdegove. Je propose de déployer sur prochainement, et de noter les comparaisons après sur les deux requêtes types en terme de temps de réponse, ça te va ?

thbar commented 1 week ago

J'ajoute @vdegove qu'une fois que c'est déployé sur prochainement, on peut partager les urls avec les @etalab/transport-bizdev pour un peu + de tests plus ouvert, et que si c'est concluant, on pourra remplacer ici modes par la logique de modes_v2, redéployer sur prochainement, puis finaliser la PR pour que ça parte en prod semaine prochaine par exemple.

vdegove commented 1 week ago

Je propose de déployer sur prochainement, et de noter les comparaisons après sur les deux requêtes types en terme de temps de réponse, ça te va ?

On fait ça.

vdegove commented 1 week ago

@AntoineAugusti a trouvé deux cas de reproduction d’erreurs 500 sur prochainement :

thbar commented 1 week ago

merci @AntoineAugusti - et poke @vdegove j'ai ajouté deux todos liées à ça sur la PR, on s'en reparle !

thbar commented 1 week ago

Merci @ptitfred

Je n'ai pas assez d'expérience du support JSON de PG pour avoir un regard critique sur les requêtes SQL utilisant le cache.

Ça peut être l'occasion de faire un tour ensemble, même sans le côté regard critique est-ce que déjà les appels sont plutôt clairs ou pas trop, justement pour la maintenabilité future ? Est-ce que le principe général te semble suffisamment lisible ? (en dehors de l'aspect privé / public qu'on va raffiner) ?

Merci !

thbar commented 1 week ago

Merci @ptitfred pour la review.

Au final j'ai supprimé le filtrage par dataset ids, car c'est à la réflexion plutôt un "reste" des besoins de développer en incréments, que vraiment un besoin applicatif derrière à ce stade (on n'a même pas besoin de batcher tellement c'est rapide etc). Voir https://github.com/etalab/transport-site/pull/4006/commits/5dd3b1ddec91fe03c60bf9fff664449f69c0e895.

thbar commented 1 week ago

@vdegove j'ai refait une passe ! (merci @ptitfred pour les retours).

thbar commented 1 week ago

On a fait le point avec @vdegove, on déploie c'est parti, et je viendrai compléter les TODOs post deploy ici.

thbar commented 1 week ago

J'ai été voir: baisse nette des erreurs DB, temps de réponse nettement amélioré, on est OK.

thbar commented 4 days ago

j'ai été "recetter" après quelques jours de recul:

Point intéressant, le taux d'erreur (% de requêtes web non réussies, qui lèvent une erreur) pour nos utilisateurs a baissé fortement (on était entre 4% et 5% avant, on est entre 0,5 et 1% après). En terme de fiabilité / utilisabilité pour nos visiteurs / utilisateurs d'API, c'est un gros bon qualitatif au final.

CleanShot 2024-06-28 at 09 09 29@2x

Le gain en temps de réponse se confirme dans le temps, ce qui est une bonne nouvelle (pas de "surprise")

CleanShot 2024-06-28 at 09 09 57@2x

Ce round est donc nettement concluant.

Concernant les erreurs DB selon où on regarde la conclusion n'est pas la même (merci @vdegove pour tes liens), et je vais aller ajouter ces éléments sur #3997

AntoineAugusti commented 4 days ago

Stylé ! Est-ce qu'on connait la part de crawlers sur la recherche ? Il me semble que les crawlers perdent pas mal de temps là-bas vu le nombre de liens à suivre (pagination, filtres etc).

etalab / transport-site