Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TECH] Migrer la colonne knowledge_elements.id de INTEGER en BIG INTEGER (partie 1). #3357

Merged
merged 3 commits into from
Aug 20, 2021

Conversation

jbuget
Copy link
Contributor

@jbuget jbuget commented Aug 19, 2021

🦄 Problème

Historiquement, les identifiants des tables de données utilisateur (PostgreSQL) sont de type Integer.

Si on ne fait rien, d’ici quelques mois, nous risquons de taper la limite des enregistrements possibles sur la table knowledge-elements qui est de loin la table la plus volumineuse de Pix (700M de lignes), avec un fonctionnement qui se rapproche d’un comportement évènementiel (énormément d’insertions par secondes).

Pour tenir les échéances à venir, nous devons changer le type de de l’ID (clé primaire) des tables les plus volumineuses risquées : knowledge-elements et answers en tête.

Au vu de la quantité de données, les risques sont :

  • nécessiter une (très importante) indispo de la plateforme
  • corrompre ou perdre des données
  • fracasser toutes les données / la plateforme

🤖 Solution

Après étude / POC, nous en sommes venus à la conclusion que la meilleure façon de procéder, avec le minimum de downtime + gestion de projet / com / partenaires et le maximum de contrôle, consiste à développer et exécuter un script / une procédure de changement de type.

Le principe général de ce script est le suivant :

  1. une première phase de "préparation des données" au cours de laquelle on recopie l'identifiant (Integer) vers une nouvelle colonne de type BigInteger (nommée bigintId). Cette phase prend plusieurs heures sur plusieurs centaines de millions de lignes.
  2. une seconde phase dite de "maintenance" dans laquelle on bloque l'accès (via un lock) à la table knowledge-elements (en lecture et écriture) afin de réaliser le renommage des meta-data (faire de la colonne bigintId la nouvelle colonne id)

Lors de notre étude (menées sur des volumes et types de données se rapprochant de la prod, sur zone SecNum Cloud), nous avons obtenu un passage de plusieurs heures de maintenance avec une approche naïve (un bête ALTER TABLE xxx) à une fenêtre de maintenance de moins d'un dixième (< 50ms) d'une seconde (!). En tenant compte d'éventuel passage à l'échelle de la réalité, nous estimons qu'avec cette solution, nous aurons moins de 5 secondes de downtime.

🌈 Remarques

Cette PR est la première d'un lot de 3 PR:

Son but est de :

  • fournir un premier script de migration qui ajoute la colonne temporaire bigint
  • faire en sorte qu'il s'exécute automatiquement sur les bases / environnement non-prod (nombre de lignes sur la table KE < 10 millions)
  • fournir le script Node à exécuter manuellement sur la base de production qui réalise la première phase de préparation (cf. solution ci-dessus)

Notre méthode pour la rédaction des scripts (migration et traitement des données) a été de copier petit à petit – et en y ajoutant des tests autant que possible – les instructions du POC vers le repository.

Il est conseillé de lire les commits unitairement.

💯 Pour tester

Migration des données si faible volumétrie

Localhost ou RA

1/ Exécuter les migrations (depuis /api/) :

npm run db:reset

2/ Rollback les 2 dernières migrations (car c'est tout l'intérêt de cette PR)

npx knex migrate:down --knexfile db/knexfile.js 20210811153908_alter_table_account-recovery-demands_add_foreign_key_not_null_userId.js

3/ Vérifier qu'il y a bien 2 migrations à jouer

npx knex migrate:list --knexfile db/knexfile.js

4/ Jouer la première migration, qui ajoute la colonne knowledge-elements.bigintId ainsi que le trigger

npx knex migrate:up --knexfile db/knexfile.js 20210818155256_copy_id_to_bigintid_on_knowledge-elements.js

5/ Vérifier que la colonne et le trigger sont bien déployés en se connectant à la base en local

$ psql postgresql://postgres@localhost/pix_test
> \d "knowledge-elements"

image

6/ Jouer la seconde migration

npx knex migrate:up --knexfile db/knexfile.js 

7/ Vérifier qu'il n'existe plus de KE avec un champ bigintId ayant pour valeur -1 (valeur par défaut)

$ psql postgresql://postgres@localhost/pix_test
> select * from "knowledge-elements" where "bigintId"=-1;

image

Migration des données si forte volumétrie

RA

Simuler que les données n'ont pas été migrées

UPDATE "knowledge-elements" SET "bigintId" = -1;
DROP INDEX IF EXISTS "knowledge-elements_bigintId_index";

Paramétrer la variable d'environnement

KNOWLEDGE_ELEMENTS_BIGINT_MIGRATION_CHUNK_SIZE=10000

Exécuter la migration

scalingo --region osc-fr1 --app pix-api-review-pr3357 run --detached "node scripts/prepare-ke-bigint-id-to-be-used-as-primary-key.js"

Vérifier dans les logs que la migration a eu lieu et s'est arrêtée

2021-08-19 12:32:40.084123841 +0200 CEST [one-off-2954] {"name":"pix-api","hostname":"pix-api-review-pr3357-one-off-2954","pid":24,"level":30,"msg":"Updated rows : 50","time":"2021-08-19T10:32:40.083Z","v":0}`
2021-08-19 12:32:40.485565540 +0200 CEST [manager] container [one-off-2954] (611e33466ffbd90d43fcc7ea) has stopped

Note: dû à la structure des seeeds (discontinuité des identifiants), les logs mentionnent

2021-08-19 12:32:40.084123841 +0200 CEST [one-off-2954] {"name":"pix-api","hostname":"pix-api-review-pr3357-one-off-2954","pid":24,"level":30,"msg":"Updated rows : 0","time":"2021-08-19T10:32:40.083Z","v":0}`

Vérifier en BDD que les données ont été migrées

SELECT * FROM "knowledge-elements" WHERE "bigintId" = -1; 
(0 rows)

SELECT * FROM "knowledge-elements" WHERE "bigintId" <> "id";
(0 rows)

Vérifier que l'index est créé et valide

SELECT ndx.indisvalid 
FROM pg_index ndx INNER JOIN pg_class cls ON ndx.indexrelid = cls.oid
WHERE cls.relname  = 'knowledge-elements_bigintId_index'

Copie de la production

TODO (dump en cours de restauration)

scalingo --regionosc-secnum-fr1 --app pix-int-to-bigint-test run --detached "node scripts/prepare-ke-bigint-id-to-be-used-as-primary-key.js"

@pix-service
Copy link
Contributor

@VincentHardouin VincentHardouin marked this pull request as draft August 19, 2021 08:16
@jbuget jbuget force-pushed the tech-ke-int-to-bigint branch 3 times, most recently from 6d5bd03 to 2b2fb09 Compare August 19, 2021 10:21
@octo-topi octo-topi changed the title Tech ke int to bigint [TECH] Migrer la colonne knowledge_elements.id de INTEGER en BIG INTEGER. Aug 19, 2021
@octo-topi octo-topi added Development in progress cross-team Toutes les équipes de dev labels Aug 19, 2021
@octo-topi octo-topi changed the title [TECH] Migrer la colonne knowledge_elements.id de INTEGER en BIG INTEGER. [TECH] Migrer la colonne knowledge_elements.id de INTEGER en BIG INTEGER (partie 1). Aug 19, 2021
@jbuget jbuget force-pushed the tech-ke-int-to-bigint branch 2 times, most recently from 4be03b8 to 8ac9a42 Compare August 19, 2021 13:54
Copy link
Contributor

@octo-topi octo-topi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pair prog

@octo-topi octo-topi marked this pull request as ready for review August 19, 2021 15:18
@octo-topi octo-topi force-pushed the tech-ke-int-to-bigint branch 2 times, most recently from 7e1e3af to 4962cc1 Compare August 19, 2021 15:31
Copy link
Member

@VincentHardouin VincentHardouin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

En local ça m'a l'air tout bon

Copy link
Member

@laura-bergoens laura-bergoens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

J'ai testé fonctionnellement le trigger, la colonne ID est bien copiée dans bigintId

…-elements ID column

This commit also contains the delete of a useless test.

This commit also contains the first (knex) raw SQL statements that create PLSQL function and a trigger.
…mall databases

This migration is only executed for small databases (local, dev, integration, recette). It is why we make a test on MAX_ROW_COUNT_FOR_SYNCHRONOUS_MIGRATION.

We select 10 millions rows value because in production there are 700M rows.

Unfortunately, we did not find a pragmatic way to make tests on this migration.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cross-team Toutes les équipes de dev 🚀 Ready to Merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants