Laisser le serveur mysql/mariadb choisir le moteur à utiliser #29

Open
cam.lafit wants to merge 1 commits from issue_4277 into master
cam.lafit commented 1 year ago
Collaborator
  • SPIP fonctionne aussi bien avec innodb, si ce moteur est actif autant l'utiliser
  • Prémuni de certains problèmes d'erreurs silencieuses comme les index trop long sur mysisam

cf #4277

* SPIP fonctionne aussi bien avec innodb, si ce moteur est actif autant l'utiliser * Prémuni de certains problèmes d'erreurs silencieuses comme les index trop long sur mysisam cf #4277
Owner

J'y connais vraiment vraiment rien en stockage de bdd etc car c'est vraiment de l'admin sys, mais si SPIP n'a rien de particulier qui demande absolument tel moteur, alors oui : faut totalement enlever ça, et c'est à la config serveur de se débrouiller suivant son défaut.

J'y connais vraiment vraiment rien en stockage de bdd etc car c'est vraiment de l'admin sys, mais si SPIP n'a rien de particulier qui demande absolument tel moteur, alors oui : faut totalement enlever ça, et c'est à la config serveur de se débrouiller suivant son défaut.
Owner

Un git blame permet de voir que la ligne n'a pas été ajoutée gratuitement :
https://core.spip.net/issues/2727

Donc certes il serait bien de supporter innoDB, mais encore faudrait-il traiter les problèmes que cela génère...

(Peut-être que plus aucun de ces soucis n'est d'actualité, mais il faut a minima vérifier pour savoir)

Un git blame permet de voir que la ligne n'a pas été ajoutée gratuitement : https://core.spip.net/issues/2727 Donc certes il serait bien de supporter innoDB, mais encore faudrait-il traiter les problèmes que cela génère... (Peut-être que plus aucun de ces soucis n'est d'actualité, mais il faut a minima vérifier pour savoir)
Poster
Collaborator

A mon sens le ticket d'origine n'est plus valide on remonte à 8 ans en arrière et les remarques ne sont plus valides (3 versions majeures de retard sur mysql).

  • Innodb accepte fulltext
  • lent ou pas lent, c'est le moteur par défaut depuis des années et c'est le moteur maintenu à l'heure actuelle
  • que spip utilise ou non les transactions n'a pas d'impact sur le moteur
  • de plus on peut mélanger (même si non recommandé) du mysam et innodb dans la même base.

Si /ecrire/?exec=base_repair pose problème c'est à lui d'être corrigé. Pas le moteur de base de données.
Sur un dump SQL il est conseillé d'indiquer le moteur utilisé (ce qui se fait par défaut actuellement) ainsi à l'import c'est celui ci qui sera pris en compte et non le défaut.

A mon sens le ticket d'origine n'est plus valide on remonte à 8 ans en arrière et les remarques ne sont plus valides (3 versions majeures de retard sur mysql). * Innodb accepte fulltext * lent ou pas lent, c'est le moteur par défaut depuis des années et c'est le moteur maintenu à l'heure actuelle * que spip utilise ou non les transactions n'a pas d'impact sur le moteur * de plus on peut mélanger (même si non recommandé) du mysam et innodb dans la même base. Si /ecrire/?exec=base_repair pose problème c'est à lui d'être corrigé. Pas le moteur de base de données. Sur un dump SQL il est conseillé d'indiquer le moteur utilisé (ce qui se fait par défaut actuellement) ainsi à l'import c'est celui ci qui sera pris en compte et non le défaut.
b_b commented 1 year ago
Owner

@cam.lafit +1 et comme je le disais il y a 8 ans, les champs de type geometry ton bien pris en charge par mysql à partir de la version 5.1, et mariadb gère aussi ça et les index de type spatial depuis la 10.2.2 :

On MyISAM and Aria tables, as well as on InnoDB tables from MariaDB 10.2.2, MariaDB can create spatial indexes (an R-tree index) using syntax similar to that for creating regular indexes, but extended with the SPATIAL keyword. Currently, columns in spatial indexes must be declared NOT NULL.

https://mariadb.com/kb/en/spatial-index/

Mais attention, par exemple debian stretch (supportée jusqu'à 2020) ne fourni que la version 10.1.44, qui pose aussi des soucis sur la longeurs des index et certainement d'uatres choses.

@cam.lafit +1 et comme je le disais il y a 8 ans, les champs de type geometry ton bien pris en charge par mysql à partir de la version 5.1, et mariadb gère aussi ça et les index de type spatial depuis la 10.2.2 : > On MyISAM and Aria tables, as well as on InnoDB tables from MariaDB 10.2.2, MariaDB can create spatial indexes (an R-tree index) using syntax similar to that for creating regular indexes, but extended with the SPATIAL keyword. Currently, columns in spatial indexes must be declared NOT NULL. https://mariadb.com/kb/en/spatial-index/ Mais attention, par exemple debian stretch (supportée jusqu'à 2020) ne fourni que la version 10.1.44, qui pose aussi des soucis sur la longeurs des index et certainement d'uatres choses.
Poster
Collaborator

Le problème des index n'est pas propre à innodb, en myisam il se présente aussi. Cela dépend entre autre si on utilise au non utf8mb4 qui consomme plus d'octets.
Le conflit qu'on rencontre sur stretch est plus lié au fait que les 2 évolutions taille des indexes et largeur d'encodage n'ont pas suivi le même mouvement.

https://florent.poinsaut.fr/2018/08/17/mysql-mariadb-index-column-size-too-large-the-maximum-column-size-is-767-bytes/

Ce cas de figure se présente déjà (sans le présent correctif) car il suffit d'importer un dump sql pour avoir des problèmes d'index (c'est comme ça que je suis tombé sur le ticket associé à cette PR : import plus montée de version)

Le problème des index n'est pas propre à innodb, en myisam il se présente aussi. Cela dépend entre autre si on utilise au non utf8mb4 qui consomme plus d'octets. Le conflit qu'on rencontre sur stretch est plus lié au fait que les 2 évolutions taille des indexes et largeur d'encodage n'ont pas suivi le même mouvement. https://florent.poinsaut.fr/2018/08/17/mysql-mariadb-index-column-size-too-large-the-maximum-column-size-is-767-bytes/ Ce cas de figure se présente déjà (sans le présent correctif) car il suffit d'importer un dump sql pour avoir des problèmes d'index (c'est comme ça que je suis tombé sur le ticket associé à cette PR : import plus montée de version)
Owner

Bon alors pour être constructif :

  • on a un code qui marche avec forçage du engine=myisam
  • on a une proposition de revert de ce forçage

Pour décider d'intégrer ça, il faut donc vérifier si les 2 problèmes bloquants sont résolus et/ou les résoudre si ce n'est pas le cas

  • restaurer un dump avec des index fulltext venant d'une base en myisam dans une base en innoDB
  • avoir un exec=base_repair fonctionnel en innoDB

Si c'est deux conditions sont remplies on peut enlever le forçage de l'engine.

La solution intermédiaire c'est celle proposée dans https://core.spip.net/issues/2727, à savoir une constante _MYSQL_STORAGE_ENGINE définie par défaut dans SPIP à MYSISAM mais qui peut être personalisée (pour forcer un autre engine) ou mise à vide (pour ne rien forcer du tout et conserver le réglage par défaut de mysql)

Bon alors pour être constructif : - on a un code qui marche *avec* forçage du `engine=myisam` - on a une proposition de revert de ce forçage Pour décider d'intégrer ça, il faut donc vérifier si les 2 problèmes bloquants sont résolus et/ou les résoudre si ce n'est pas le cas - [ ] restaurer un dump avec des index fulltext venant d'une base en myisam dans une base en innoDB - [ ] avoir un `exec=base_repair` fonctionnel en innoDB Si c'est deux conditions sont remplies on peut enlever le forçage de l'engine. La solution intermédiaire c'est celle proposée dans https://core.spip.net/issues/2727, à savoir une constante `_MYSQL_STORAGE_ENGINE` définie par défaut dans SPIP à `MYSISAM` mais qui peut être personalisée (pour forcer un autre engine) ou mise à vide (pour ne rien forcer du tout et conserver le réglage par défaut de mysql)
Owner

Je pense aussi comme @cam.lafit et @b_b qu’il faut au moins changer la valeur par défaut de SPIP. Donc, va falloir véririer les dumps et le repair :)

Je pense aussi comme @cam.lafit et @b_b qu’il faut au moins changer la valeur par défaut de SPIP. Donc, va falloir véririer les dumps et le repair :)
Owner

Bon, fulltext est pas prévu pour actuellement :

Bon, fulltext est pas prévu pour actuellement :
Owner

J’ai envoyé des modifications sur le plugin Fulltext pour gérer InnoDB ou MyISAM.
Une version 2.0 pour SPIP 3.2+ (mais je n’ai pas fait le tag encore).

Concernant :

avoir un exec=base_repair fonctionnel en innoDB

Il n’y a pas de fonction sql "repair" sous InnoDB.
Mysql propose éventuellement de recréer la table avec ALTER TABLE t1 ENGINE = InnoDB; (en remettant l’engine en cours). Mais pas certain que ça soit la même chose.

J’ai envoyé des modifications sur le plugin Fulltext pour gérer InnoDB ou MyISAM. Une version 2.0 pour SPIP 3.2+ (mais je n’ai pas fait le tag encore). Concernant : > avoir un exec=base_repair fonctionnel en innoDB Il n’y a pas de fonction sql "repair" sous InnoDB. [Mysql](https://dev.mysql.com/doc/refman/8.0/en/rebuilding-tables.html) propose éventuellement de recréer la table avec `ALTER TABLE t1 ENGINE = InnoDB;` (en remettant l’engine en cours). Mais pas certain que ça soit la même chose.
cam.lafit requested review from rastapopoulos 10 months ago
cam.lafit removed review request for rastapopoulos 10 months ago
cam.lafit requested review from rastapopoulos 10 months ago
cam.lafit requested review from marcimat 10 months ago
cam.lafit removed review request for marcimat 10 months ago
cam.lafit requested review from marcimat 10 months ago
cam.lafit requested review from cerdic 10 months ago
Owner

Concernant :

avoir un exec=base_repair fonctionnel en innoDB

Il n’y a pas de fonction sql "repair" sous InnoDB.
Mysql propose éventuellement de recréer la table avec ALTER TABLE t1 ENGINE = InnoDB; (en remettant l’engine en cours). Mais pas certain que ça soit la même chose.

Claro, je pense qu'on peut désactiver le bloc Réparation de la base et son exec en innodb, cf :

In case the corrupted table uses the InnoDB storage engine, then the process for repairing it will be different. InnoDB features automated corruption checking and repair operations. It checks for corrupted pages by performing checksums on the pages it reads, and if there is a discrepancy it will stop the MySQL server automatically.

It’s not often a need to repair InnoDB tables, as it features a crash recovery mechanism that can resolve most issues when the server is restarted. However, if you find a situation where you need to rebuild a corrupted InnoDB table, the MySQL documentation recommends the use of Dump and Reload method.

https://www.letscloud.io/community/how-to-fix-corrupted-tables-in-mysql

Et du côté de mariadb aussi :
https://mariadb.com/kb/en/innodb-recovery-modes/#fixing-things
https://mariadb.com/kb/en/repairing-mariadb-tables-for-sql-server-users/#innodb

> Concernant : > > avoir un exec=base_repair fonctionnel en innoDB > > Il n’y a pas de fonction sql "repair" sous InnoDB. > [Mysql](https://dev.mysql.com/doc/refman/8.0/en/rebuilding-tables.html) propose éventuellement de recréer la table avec `ALTER TABLE t1 ENGINE = InnoDB;` (en remettant l’engine en cours). Mais pas certain que ça soit la même chose. > Claro, je pense qu'on peut désactiver le bloc Réparation de la base et son exec en innodb, cf : > In case the corrupted table uses the InnoDB storage engine, then the process for repairing it will be different. InnoDB features automated corruption checking and repair operations. It checks for corrupted pages by performing checksums on the pages it reads, and if there is a discrepancy it will stop the MySQL server automatically. > > It’s not often a need to repair InnoDB tables, as it features a crash recovery mechanism that can resolve most issues when the server is restarted. However, if you find a situation where you need to rebuild a corrupted InnoDB table, the MySQL documentation recommends the use of Dump and Reload method. https://www.letscloud.io/community/how-to-fix-corrupted-tables-in-mysql Et du côté de mariadb aussi : https://mariadb.com/kb/en/innodb-recovery-modes/#fixing-things https://mariadb.com/kb/en/repairing-mariadb-tables-for-sql-server-users/#innodb
Owner

Non il faut pas le desactiver, mais l'adapter, comme en SQLite où il est utilisé simplement pour vérifier que toutes les tables sont présentes et les créer si besoin (et je sais plus si il créé les champs manquants aussi)

Non il faut pas le desactiver, mais l'adapter, comme en SQLite où il est utilisé simplement pour vérifier que toutes les tables sont présentes et les créer si besoin (et je sais plus si il créé les champs manquants aussi)
Owner

Non il faut pas le desactiver, mais l'adapter, comme en SQLite où il est utilisé simplement pour vérifier que toutes les tables sont présentes et les créer si besoin (et je sais plus si il créé les champs manquants aussi)

Oui je disais de la merde sur ce point, il faut le garder ne serait-ce que pour relancer la création des tables des plugins, puisqu'on a presque tout le temps des bugs de création de tables à l'install chez OVH avec leur opcache.

> Non il faut pas le desactiver, mais l'adapter, comme en SQLite où il est utilisé simplement pour vérifier que toutes les tables sont présentes et les créer si besoin (et je sais plus si il créé les champs manquants aussi) Oui je disais de la merde sur ce point, il faut le garder ne serait-ce que pour relancer la création des tables des plugins, puisqu'on a presque tout le temps des bugs de création de tables à l'install chez OVH avec leur opcache.
Owner

Pour info c’est déjà le cas :

https://git.spip.net/spip/spip/src/branch/master/ecrire/base/repair.php#L128

Le repair retourne un message d’erreur sous innoDb ; la fonction continue et fait la création des tables ou champs manquants, et affiche ensuite le message d’erreur, donnant :

spip_articles (10 éléments)
spip-git.spip_articles repair note The storage engine for the table doesn't support repair

spip_auteurs (33 éléments)
spip-git.spip_auteurs repair note The storage engine for the table doesn't support repair 

...
Pour info c’est déjà le cas : https://git.spip.net/spip/spip/src/branch/master/ecrire/base/repair.php#L128 Le repair retourne un message d’erreur sous innoDb ; la fonction continue et fait la création des tables ou champs manquants, et affiche ensuite le message d’erreur, donnant : ``` spip_articles (10 éléments) spip-git.spip_articles repair note The storage engine for the table doesn't support repair spip_auteurs (33 éléments) spip-git.spip_auteurs repair note The storage engine for the table doesn't support repair ... ```
Owner

@marcimat super, mais le message en question n'est pas très rassurant pour les users. En bref, il annonce que les tables ne sont pas réparables, et je ne suis pas certain qu'il annonce qu'il a créé des tables de plugins qui pourraient manquer par exemple.

@marcimat super, mais le message en question n'est **pas très rassurant pour les users**. En bref, il annonce que les tables ne sont pas réparables, et je ne suis pas certain qu'il annonce qu'il a créé des tables de plugins qui pourraient manquer par exemple.
Owner

@b_b j’ai pas dit que c’était bien en l’état. Juste que ça fait déjà ce qui est attendu finalement. On n’annoncait déjà pas si on créait des tables ou des champs.

@b_b j’ai pas dit que c’était bien en l’état. Juste que ça fait déjà ce qui est attendu finalement. On n’annoncait déjà pas si on créait des tables ou des champs.
b_b commented 1 month ago
Owner

Il n’y a pas de fonction sql "repair" sous InnoDB.
Mysql propose éventuellement de recréer la table avec ALTER TABLE t1 ENGINE = InnoDB; (en remettant l’engine en cours). Mais pas certain que ça soit la même chose.

Pour info, j'ai pu le tester sur des tables crashées (nextcloud de mémoire) chez Infini, et ça a très bien fonctionné :)

> Il n’y a pas de fonction sql "repair" sous InnoDB. > [Mysql](https://dev.mysql.com/doc/refman/8.0/en/rebuilding-tables.html) propose éventuellement de recréer la table avec `ALTER TABLE t1 ENGINE = InnoDB;` (en remettant l’engine en cours). Mais pas certain que ça soit la même chose. Pour info, j'ai pu le tester sur des tables crashées (nextcloud de mémoire) chez Infini, et ça a très bien fonctionné :)

Reviewers

rastapopoulos was requested for review 10 months ago
marcimat was requested for review 10 months ago
cerdic was requested for review 10 months ago
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
Sign in to join this conversation.
No Label
No Milestone
No Assignees
5 Participants
Notifications
Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

Loading…
There is no content yet.