fab-geocommuns / RNB-coeur

Le coeur du Référentiel National des Bâtiments : imports, APIs, logique métier
https://rnb.beta.gouv.fr
Apache License 2.0
3 stars 0 forks source link

Parent buildings : ajout d'un index #467

Open fchabouis opened 3 weeks ago

fchabouis commented 3 weeks ago

On va avoir besoin de faire des requêtes rapides sur le champ parent_buildingsdans le cadre de l'affichage de l'arbre généalogique d'un bâtiment et donc que le champ soit indexé.

fchabouis commented 3 weeks ago

Est-ce qu'on ne devrait pas pas utiliser AddIndexConcurrently pour éviter de rendre la backend indisponible pendant X heures ?

La discussion qu'on avait eu sur le risque des index invalid est là : #399 (comment)

J'aurais tendance à le lancer tel quel et à au moins être sûr que l'index est bien créé, quitte à le lancer la nuit. Qu'en penses tu ?

pauletienney commented 3 weeks ago

Est-ce qu'on ne devrait pas pas utiliser AddIndexConcurrently pour éviter de rendre la backend indisponible pendant X heures ? La discussion qu'on avait eu sur le risque des index invalid est là : #399 (comment)

J'aurais tendance à le lancer tel quel et à au moins être sûr que l'index est bien créé, quitte à le lancer la nuit. Qu'en penses tu ?

Depuis le 1er juin, nous avons eu 30536 appels à un de nos endpoints entre 23h et 3h du matin. C'est loin d'être la majorité des appels mais ce n'est pas rien non plus. Je trouve dommage de rendre indispo les services alors que nous avons une solution pour indexer sans être bloquants.

En plus de la situation actuelle, ça me semble sain d'apprendre à réaliser ce type d'opérations sans bloquer les services en général.

fchabouis commented 3 weeks ago

J'ai recreusé le sujet, parce que j'avais gardé en souvenir que le AddIndexConcurrently c'était pas ça qu'il nous fallait mais je ne me souvenais plus pourquoi.

Version courte

la migration est quand même bloquante.

Explications plus détaillées

Le concurrently signifie que Postgres ne lock pas la table en écriture pendant la création de l'index. Ce qui n'est pas un soucis pour nous car personne n'écrit pour le moment dans la table building sans nous le dire. Voir la doc.

Lorsque l'on merge une PR qui contient une migration, cela déclenche un rebuild du docker compose, qui entraine le lancement des migrations via entrypoint.sh. Donc avec notre fonctionnement le serveur s'arrête et doit relancer ses migrations avant de redémarrer. Si les migrations sont lentes, le site reste down un certain temps.

Je viens de faire le test sur staging : j'ai lancé la création d'un index avec concurrently, j'ai relancé la commande pour builder le container et le "up", j'ai une 502 pendant que la migration tourne.

Et même comme d'après la doc la création "concurrente" est plus lente que la normale, c'est à priori contre productif de le faire.

Capture d’écran 2024-09-26 à 17 11 58

Capture d’écran 2024-09-26 à 17 12 28

Capture d’écran 2024-09-26 à 17 12 59

pauletienney commented 3 weeks ago

Ok. Je ne me souvenais plus qu'on avait été bloqués lorsqu'on l'avait utilisé la dernière fois. Je me souvenais seulement de l'index invalid. Merci pour les détails.