Le bilan de la release 15.5.1

Ami développeurs et autres, bonjour !

Je vais tenter de faire un petit résumé et une petite analyse de ce qui s'est passé avec notre dernière bêta en date. Avant toute chose, je tient à préciser que cette analyse n'est pas exhaustive, et qu'elle n'engage que moi. Notre DTC préféré est la personne la plus à même à confirmer/infirmer ce que je raconte le cas échéant.

Et on y va.

Les performances et le "front"

(j'ai mis front entre guillemet parce que ça revient souvent à une histoire de back)

Et en fait, toute cette histoire n'as pas commencé avec cette release, mais avec la précédente. Il y a une vingtaine de jours, notre DTC adoré nous a fait remarqué que depuis le passage à la version précédente, les performances étaient vachement tirées vers le bas. Ce qui a déclanché ce problème est pas clair et est un mélange du passage à Django 1.7 et/ou autre chose.

Les causes annoncées sont à peu près listée dans le ticket donné en lien plus haut. En vrac:

• Le morceau de template badge.part.html. Avec, en toile de fond, un problème encore plus profond qui est celui de la séparation entre la classe Profile et User, la première étant à nous, la seconde à Django, donc le besoin quasi systématique d'une requête pour récupérer l'un ou l'autre (en l'occurence, ici c'est Profile). Pour pas arranger les choses, on a réussi à désyncroniser les deux classes, ce qui cause, entre autre, des problèmes avec l'API (et, si je suis bien, la décision de retarder l'arrivée de l'API des MPs)
• Le morceau de template message.part.html qui, non content de contenir la précédente, rajoute des requêtes en masse pour la récupération des +1/-1 et qui fait un nombre proprement incroyable de requêtes liées à de la vérification de droits (ce qui est particulièrement ridicule et facile à régler: au lieu de vérifier à chaque message que l'admin est bien un admin, on le fait une fois au début)
• Le code front est parsemé de perms.machin.bidule pour vérifier que l'utilisateur a bien les droits qu'il prétend avoir. C'est une requête à chaque fois. Et c'est un machin qu'on retrouve dans plein de {% if %} ! side-effect, naviguer sur le site en temps qu'admin vous fait automatiquement choper plein de requêtes supplémentaires du genre, ZdS est pas staff-friendly
• La template /forum/base.html, qui souffre en fait d'une absence de données renvoyées par le back et qui compense par énormément de requêtes. La partie qui pose plus particulièrement problème est celle qui gère les messages suivis: pour chacun d'eux, la template doit récupérer le dernier message lu, ainsi que vérifier que ledit message est lu ou pas (avec le templatetag is_read, qui finit par refaire la même requête, basiquement). La solution à ce problème est d'aller rajouter des morceaux de requêtes dans le bon templatetag
• Même problème dans un autre templatetags, celui de la récupération des MPs, le bien nommé interventions_privatetopics: une fois encore, pour un MPs non lu, c'est une requête pour récupérer le dernier message et son auteur. Par contre, sur celle là, il va falloir que quelqu'un trouve le courage de faire de la magie noire, parce qu'il y a une grosse requête SQL inscrite en dur.
• Et d'autres, j'imagine, mais c'est les plus gros que j'aie pu débusquer. J'imagine que si on règle ça, on y verra déjà un peu plus clair dans ce que la django toolbar nous sort. Instinctivement, vu les deux derniers points, j'aurais tendance à dire que le templatetags qui récupère les notifs se comporte de manière semblable, mais j'accuse sans preuve.

Et là, Spacefox me répondra probablement "La consommation du site, ce n'est pas que le nombre de requêtes, loin de là.", et il aura probablement raison. D'ailleurs, cette réponse est tellement intéressante que je la cite de suite:

Je me permet d'appuyer fortement le second point: le code en est carrément infesté, et tout ce que j'ai noté plus haut est relié à ce problème de près ou de loin. Par contre, s'en passer va être difficile: pour avoir l'info, il faut quand même faire la requête à un moment. Donc la question, c'est où ?

La ZEP-04

Je ne ferais jamais assez les louanges de la ZEP-04, c'est un excellent travail, et de loin. D'ailleurs, les développeurs de la ZEP-04 n'y sont globalement pour rien. Avant de recevoir des tomates trop mûres dans la figure, laissez moi expliquer le pourquoi de ce titre, donc.

Contrairement à ce qu'on pourrait croire, la ZEP-04 ne fait pas tant de requêtes supplémentaires que ça. Je m'explique: basiquement, la liste des articles et des tutos, on l'avait déjà, la récupération du nombre de commentaire et du dernier commentaire pour les articles, on l'avais déjà, et grosso modo, les seuls trucs nouveaux sont les "unes" (featured items) et la liste des derniers topics en date. Rien de vraiment neuf sous le soleil, donc, sauf que tout ces points sont des nids à problèmes énoncés précédemment : get_*() générant des requêtes supplémentaires et ainsi de suite.

D'où …

La vraie fausse solution, le cache, et pourquoi c'est parti en cacahuètes

… D'où l'idée un peu folle de Spacefox d'intégrer un cache aux éléments couteux de la page d’accueil, pour calmer les statistiques de Munin qui s'affolaient complètement:

Sauf que le front est, en plus d'être infesté de get_*() vicieux, ultra-factorisé (Spacefox vous dira "trop dynamique"). Il semblerait donc que la factorisation, c'est pas très cache-friendly.

Deux choses à séparer, là:

1. Et d'un, certaines templates (souvent rangées à cet effet dans un dossier /includes/) sont appelées avec des paramètres. Pour certaines, (par exemple message.part.html), ce nombre de paramètre est proprement exorbitant (mais ce n'est pas le seul exemple). Le problème est que ces paramètres ne sont écrits nuls part, il faut les deviner en lisant le code (un jeu qui peu vite devenir lassant, et il semblerait que notre bon DTC a un peu moins de patience que moi, et je le comprend). Ce qui pose, d'une manière ou d'une autre, le problème de la documentation front, qui a déjà fait débat par le passé (et ce n'est qu'un exemple), pour un résultat probablement pas très satisfaisant si on en crois ce que je viens de dire. Notons les efforts de Situphen en ce sens, initiative plus que salutaire puisque … Tout est à faire. Sauf que cette doc n'as pas de standard (comme autodoc), ce qui rend la chose très difficile. Bref, tout ça pour dire que choisir des clés de caches devient difficile.
2. Les templates s'appelent les unes les autres. Une fois encore, c'est un comportement souhaité quand on fait un minimum de factorisation. Il semblerait donc que ça ne soit pas souhaité dans ce cas-ci. Ça, c'est à traiter au cas par cas. Quand on sais que le moteur de template est un peu lent, il semblerait que ce soit un comportement à éviter un maximum (même si on sais souvent pas faire autrement).

On en est donc arrivé à des PRs un peu rustines comme celle là, ou on règle un problème tout bête: évidement, le lien qui envois au dernier commentaire lu n'est pas le même d'un utilisateur à un autre. Et pourtant, dans cette PR là, tout le problème du cache est résumé (et j'imagine que c'est ce que notre DTC appelle "trop dynamique"). Il est évident que ce lien ne peut pas être mis en cache, puisqu'il change d'un utilisateur à un autre. Et pourtant, le retirer enlèverait une fonctionnalité au site. Et ainsi de suite.

Dans tout les cas, l'inclusion de ce cache a entraîné une montagne de problème du même genre, à cause d'un front qui semble mal pensé pour y intégrer un cache tel quel (et comment lui en vouloir, puisque ça n'as pas été pensé à la base pour ça).

Du coup, on tend à l'exclure jusqu'à nouvel ordre.

À noter ceci dit qu'on a déjà du cache sur d'autres éléments, donc TOUT le cache ne disparais pas, simplement celui qui a tenté d'être mis en place cette fois-ci.

Bonus, ou parce que quand même, tout n'est pas la faute du front (loin de là)

Pire que les requêtes en base de donnée, il faut quand même que je mentionne que l'I/O est un sujet sensible. À l'heure d'aujourd'hui, le back des articles et tutoriels est proprement catastrophique à ce sujet: le recourt systématique à Git (parfois plusieurs fois pour la même chose) y est probablement aussi pour quelque chose dans les problème de performance. Ceci étant dit, cette partie là n'as pas beaucoup évolué ces derniers temps pour des raisons évidentes.

Comme dirait artragis, #ilovezep12, on est dessus.

En conclusion

Si je devais résumer tout les points qu'il est important de traiter, les voici:

• Il faut qu'on repasse dans notre code pour optimiser nos requêtes. C'est peut être que l'arbre qui cache¹ la foret, mais c'est déjà un pas en avant.
• Il faut qu'on repense une partie du code pour éviter les get_*() qui font des requêtes supplémentaire gratuitement. Avec cette question: à quel moment récupérer l'info: dans le back en temps que paramètre passé à la template ? Dans le front avec {% set machin as truc %} au bon endroit ?
• quid d'une documentation des templates ? Est ce que c'est possible ou complètement délirant ?
• Si on veut mettre du cache, il faut qu'on repense des morceaux de front en fonction. Est ce que c'est vraiment nécessaire, ceci dit, vu la montagne de problèmes que ça a apporté cette fois-ci ?

C'est à vous !

Je vais faire une passe dans la journée pour mettre ssur dev les différents travaux de précalcul réalisés dans la zep12 et qui marchent vraiment bien.

Ensuite j'essayerai de simplifier certains getters que j'ai remarqué durant le dev de la Zep 12.

Enfin, je regarderai pour trouver un moyen de faire baisser la complexité de nos templates pour que le cache soit plus facilement intégrable.

Concernant les sujets suivis et les messages privés, il est fort possible que ça soit amélioré par la ZEP-24. Alors bon, c'est pas pour demain, mais on avance :-).

Merci pour le bilan !

C'est bien résumé. Quelques remarques en vrac :

Pour pas arranger les choses, on a réussi à désyncroniser les deux classes, ce qui cause, entre autre, des problèmes avec l'API

Alors là je vais être très chiant, mais je ne lâcherai jamais le morceau. Ce n'est pas la désynchronisation le problème, mais le fait qu'on a considéré que les IDs techniques de User et de Profile étaient identiques. Alors que ce sont des IDs techniques et que le code doit pouvoir fonctionner même s'ils sont attribués complètement au hasard.

Tu la vois, la bêta ? D'ailleurs, c'est moi ou ce graphe énonce clairement "c'est pas la page d’accueil le problème, les mecs" ?

C'est le graphe de prod.

ultra-factorisé (Spacefox vous dira "trop dynamique").

En fait, suite à une "factorisation" mal maîtrisée, les morceaux de "code commun" sont pourris de tests conditionnels qui font que ces morceaux de code sont surtout spécifiques en réalité. Ce qui rends ces morceaux de code communs, théoriquement super simples à cacher, impossibles à cacher.

Je pense que nous sommes tous d'accord qu'il faut faire quelque chose, moi même je pense contribuer prochainement pour améliorer les performances des forums. Par contre, nous devons commencer à prendre en considération une chose :

Le projet Zeste de Savoir devient un gros projet et, surtout, complexe. A partir de maintenant, nous ne pouvons plus nous permettre de mettre en oeuvre une fonctionnalité sans mesurer ses performances. Je pense donc que la mesure des performances doit être documenté au mieux dans notre documentation ("au mieux" parcece que c'est sans doute pas simple), que nous devons inclure cette vérification dans nos QAs et donc qu'il ne faut plus seulement que ça fonctionne mais que ça conserve de bonnes performances aussi.

C'était sympa de faire joujou en ajouter des fonctionnalités et à refactoriser à tout va mais nous ne pouvons plus nous le permettre aujourd'hui.

Merci bien pour ce résumé clair et détaillé.

Désolé par contre, ce n'est pas du tout ce cache là que j'avais en tête. Je pensais plus à du cache de fichiers type "assets" qui ne s'invalidait pas lors d'une MeP (et du coup donne des résultats très curieux tant qu'on n'a pas fait CTRL+F5).

Du coup pour ce type de cache je n'ai pas d'idée, désolé

Je suis en train de lire la documentation Django pour me renseigner plus sur l’optimisation. D'après ce bout de doc, il parait que l'on peut mettre en cache pour quand on appelle un gabarit (template), Django cache l'endroit où est le fichier et ne le cherche plus dans les répertoires lors d'un prochain appel. Je ne sais pas si ça vaut le coup de mettre en place (quel sera le gain ?) mais on peut toujours essayer (peut-être qu'on gagnera quelques % ?) !

@artragis: Vas-y, seulement !

Alors là je vais être très chiant, mais je ne lâcherai jamais le morceau. Ce n'est pas la désynchronisation le problème, mais le fait qu'on a considéré que les IDs techniques de User et de Profile étaient identiques. Alors que ce sont des IDs techniques et que le code doit pouvoir fonctionner même s'ils sont attribués complètement au hasard.

Autrement dit, un models.ForeignKey('User') et tenter d'appeller un maximum la classe Profile quand c'est possible et ça va aller mieux ?

C'est le graphe de prod.

Oups, en effet (j'ai corrigé). Dommage que le même graphe existe pas pour la bêta (j'ai été vérifier dans Munin, mais j'imagine que la raison à ça, c'est l'authentification).

C'est "moins critique": pour le remarquer, il a fallu que je génère plein de MP (m'enfin tester l'API des MPs et le processus de validation d'un tutoriel, ça aide bien à en générer). Donc je dirais que ça peut attendre la ZEP-24, au pire

À par la partie "refactoriser à tout vas" avec laquelle j'ai un peu de mal, j’appuie fortement ce message: si on continue comme on fait pour le moment, on va générer encore plus de ce genre de problème. Ceci étant dit, pour prendre un exemple, Spacefox veille bien à ça: le sujet de la ZEP-11 (statistique des tutoriels) est rempli de question du genre "combien ça va nous couter de perf'" et "qu'est ce que ça implique au niveau maintenance", donc je pense qu'on commence à se rendre compte que notre pile logicielle est lourde, que notre code pèse son poids aussi et qu'il faut pas faire n'importe quoi n'importe comment.

Malheureusement, ZdS n'est plus un jouet

J'ai peur qu'on ne retombe dans les mêmes travers pour les mêmes raisons. M'enfin ça coute rien d'essayer

@artragis: Vas-y, seulement !

je ne comprends pas le "seulement". Mais j'ai quand même fait une PR qui commence à nettoyer les choses et à faire du precalcul en m'inspirant de la ZEP12.

… C'est du "belge", il semblerait … Fait comme si il n'existait pas.

Sinon, j'ai pas le temps de lire la PR, mais j'ai vu que Kje était déjà passé dessus, je lirais donc ça ce soir (pour moi)

Autrement dit, un models.ForeignKey('User') et tenter d'appeller un maximum la classe Profile quand c'est possible et ça va aller mieux ?

En dehors d'une migration un poil relou, qu'est-ce qui empêche de joindre les deux modèles (User et Profile) ? Je l'ai déjà mis en place dans un coin, et il y a juste une classe à dériver et un réglage à changer pour utiliser une autre classe User avec tout le module auth de Django.

Ceci étant dit, pour prendre un exemple, Spacefox veille bien à ça: le sujet de la ZEP-11 (statistique des tutoriels) est rempli de question du genre "combien ça va nous couter de perf'" et "qu'est ce que ça implique au niveau maintenance"

Je pense que c'est parce que c'est firm1 qu'il est aussi chiant.

Sinon, j'ai pas le temps de lire la PR, mais j'ai vu que Kje était déjà passé dessus, je lirais donc ça ce soir (pour moi)

Rien à faire, je crois que tu confondras Kje et moi tout le temps.

En dehors d'une migration un poil relou, qu'est-ce qui empêche de joindre les deux modèles (User et Profile) ? Je l'ai déjà mis en place dans un coin, et il y a juste une classe à dériver et un réglage à changer pour utiliser une autre classe User avec tout le module auth de Django.

Le problème n'est pas spécialement technique. C'est juste que ça va faire d'énorme changement dans le projet et faire chier les grosses ZEP en cours, la ZEP 12 surtout.

Je pense que c'est parce que c'est firm1 qu'il est aussi chiant.

Mmmmh … Dire que tu as tort serai me mentir à moi même

Rien à faire, je crois que tu confondras Kje et moi tout le temps.

P*** de b*** de m***, encore une fois désolé, j'arrive vraiment pas à m'y faire :s

Le problème n'est pas spécialement technique. C'est juste que ça va faire d'énorme changement dans le projet et faire chier les grosses ZEP en cours, la ZEP 12 surtout.

Objectivement, concernant la ZEP-12, j'ai envie de dire qu'on en est pas à ça prêt, si c'est ce qu'il faut. On peut retarder des fonctionnalités pour la ZEP-12, mais là on parle d'optimisation urgente, donc si c'est ce que ça prend, tant pis. Par contre, à un moment, faudra écrire un script de migration si on part sur cette voie là (et ça, c'est clairement long et réverbatif à écrire, dixit ZEP-12).

c'est clairement long et réverbatif à écrire

j'ai tendance à être moins policé que toi à ce propos, mais l'idée y est, je pense.

Rha je tombe malade 48h et c'est à ce moment que tout les résumé de l'action arrivent !

Je prend le temps de faire le tour et de dépiler mes notifs github et j'essai de penser revenir ici…

En tout cas merci beaucoup Pierre pour le résumé et l'analyse !

• Le morceau de template badge.part.html.

Le badge est corrigé et ça marche.

• Le morceau de template message.part.html qui, non content de contenir la précédente, rajoute des requêtes en masse pour la récupération des +1/-1 et qui fait un nombre proprement incroyable de requêtes liées à de la vérification de droits (ce qui est particulièrement ridicule et facile à régler: au lieu de vérifier à chaque message que l'admin est bien un admin, on le fait une fois au début)

Ce template est le plus chiant. Pour la ZEP 12 on a même été jusqu'à le séparer en deux : un pour les fofo et un pour les tuto car y'a trop de if! C'est du coup aussi là que j'ai eu mes effets de bord dans ma PR, j'ai dû oublier de passer un droit (ou faire une typo) du coup les alertes ne sont plus lisibles, je corrige ça demain.

Le code front est parsemé de perms.machin.bidule pour vérifier que l'utilisateur a bien les droits qu'il prétend avoir. C'est une requête à chaque fois. Et c'est un machin qu'on retrouve dans plein de {% if %} ! side-effect, naviguer sur le site en temps qu'admin vous fait automatiquement choper plein de requêtes supplémentaires du genre, ZdS est pas staff-friendly

les perms.machin.bidule sont calculés dans les contrôleurs désormais, sur le modèle de la zep12

• La template /forum/base.html, qui souffre en fait d'une absence de données renvoyées par le back et qui compense par énormément de requêtes. La partie qui pose plus particulièrement problème est celle qui gère les messages suivis: pour chacun d'eux, la template doit récupérer le dernier message lu, ainsi que vérifier que ledit message est lu ou pas (avec le templatetag is_read, qui finit par refaire la même requête, basiquement). La solution à ce problème est d'aller rajouter des morceaux de requêtes dans le bon templatetag

précalcul des like/dislike, plusieurs prefetch sur les alertes et les notifs ont été réalisés. Malheureusement les pages purement staff (validation par exemple) ne sont toujours pas changées. J'ai fait aussi deux/trois migrations vers des managers, ce qui a eu pour effet de diminuer les appels foireux à des getters qui faisaient des requêtes à chaque fois.

Et d'autres, j'imagine, mais c'est les plus gros que j'aie pu débusquer. J'imagine que si on règle ça, on y verra déjà un peu plus clair dans ce que la django toolbar nous sort. Instinctivement, vu les deux derniers points, j'aurais tendance à dire que le templatetags qui récupère les notifs se comporte de manière semblable, mais j'accuse sans preuve.

le templatetag des notifs est horrible. Il faudrait passer faire deux/trois prefetch.

C'est quand je vois un calme plat de plusieurs jours sur le développement et un caribou qui arrive pour secouer toute la fourmilière pour résoudre 3/5 des issues bloquantes de cette release que je me dis que nous avons vraiment bien fait d'avoir un Chef de Projet pour Zeste de Savoir.

Ca me semblait important de dire que notre caribou fait vraiment du bon taff.

<3

Je l'aurais bien fait plus tôt mais j'avais de la famille en visite la semaine dernière et du coup pas du tout de temps . (et sinon j'ai beaucoup beaucoup envie de voir la nouvelle home page sortir et enchaîner avec la ZEP-12 après (qui d'ailleurs nous virera un tiers des bugs back a piori )

Alors maintenant que tout le monde s'est reposé pendant que j'avais le dos tourné, au boulot !

enchaîner avec la ZEP-12 après (qui d'ailleurs nous virera un tiers des bugs back a piori )

Pour en rajouter le double …

Ca peut arriver (mais on leur en voudra même pas tellement c'est de toute beauté ce qu'ils nous préparent !) mais comme le truc est bien tenu chez eux (sur le dépôt d'artragis) y aura pas de bug ! pis c'est tout

Parle pas de malheur! Et puis la ZEP12 est stable depuis un bout de temps, pas de notre faute s'il n'y a pas assez de testeurs de dispo.

En attendant, rien que la ZEP04 qui arrive ça va faire du bien au site.