search

betagouv / rdv-service-public

Prise de RDV pour les services publics
https://rdv.anct.gouv.fr
GNU Affero General Public License v3.0
12 stars 2 forks source link

Ajout d'index uniques manquants sur des tables de jointure #4373

Closed francois-ferrandis closed 1 week ago

francois-ferrandis commented 1 week ago

Contexte

Nous avons deux tables de jointure qui n'ont aucune garantie d'unicité. Cela peut mener à des incohérence dans nos données.

Solution

Pour garantir cette unicité, nous pouvons :

  1. ajouter un index composite UNIQUE
  2. déclarer une primary key : cela ajoute un index ainsi qu'un NOT NULL sur les colonnes impliquées

D'un point de vue logique, j'aurais aimé ajouter une primary key, mais actuellement nous n'en n'avons aucune sur nos tables de jointures, et nous utilisons plutôt la solution 1.

Côté perfs, un index sur [col1, col2] sera utilisé et efficace pour un WHERE col1 = x mais moins efficace pour WHERE col2 = x (voir doc ^1). Je pense donc qu'on pourra supprimer les index sur nos col1, dans une prochaine PR.

Déploiement

J'ai testé la migration sur un dump de prod en local (en activant transaction et safety_assured) et ça passe en une poignée de millisecondes, sûrement parce qu'il construit l'index composite à partir des deux index existants. :zap:

Checklist

adipasquale commented 1 week ago

cf https://thoughtbot.com/blog/how-to-create-postgres-indexes-concurrently-in pour comprendre disable_ddl_transaction! et algorithm: :concurrently

adipasquale commented 1 week ago

J'ai testé la migration sur un dump de prod en local (en activant transaction et safety_assured)

tu peux me guider pour comprendre ces options transaction et safety assured ?

francois-ferrandis commented 1 week ago

J'ai testé la migration sur un dump de prod en local (en activant transaction et safety_assured)

tu peux me guider pour comprendre ces options transaction et safety assured ?

Comme l'indique l'article que tu as partagé, il y a deux façons de créer un index :

Je voulais vérifier qu'il n'y avait pas de doublon en prod, et pour ce faire j'ai voulu lancer notre migration, mais en synchrone, pour avoir une erreur qui pop, car je ne sais pas où l'erreur apparaîtrait pour une création asynchrone.

Et donc concrètement j'ai commenté disable_ddl_transaction! et algorithm: :concurrently. Une fois que j'ai fait ça, si je lance db:migrate, la gem strong_migration me dit que l'ajout synchrone d'index est dangereux, et que si je suis sûr de vouloir le faire, je dois envelopper mon code de migration avec safety_assured (voir doc), ce que j'ai fait avant de lancer la migration.

adipasquale commented 1 week ago

très clair @francois-ferrandis merci beaucoup 🙇

On peut donc en déduire qu’il n’y a aucun cas problématique en prod jusqu’à aujourd’hui c’est bien ça ? cas problématiques = des doublons dans ces tables de jointures

francois-ferrandis commented 1 week ago

très clair @francois-ferrandis merci beaucoup 🙇

On peut donc en déduire qu’il n’y a aucun cas problématique en prod jusqu’à aujourd’hui c’est bien ça ? cas problématiques = des doublons dans ces tables de jointures

Oui, c'est ça, et en extrapolant on pourrait en déduire que cette PR relève de la paranoïa, ce dont je suis prêt à débattre. :thinking: