[Beta 15.5.1] Amélioration des performances des pages liste des article, liste des tutos, home

[SpaceFox]

Q	R
Correction de bugs ?	~Oui
Nouvelle Fonctionnalité ?	~Oui aussi (mais c'est pour corriger le bug)
Tickets (issues) concernés	#2669, #2671

Les nouvelles pages "Liste des tutos", "Liste des articles" et la home sont beaucoup plus lourdes que les actuelles, ce qui est problématique parce que ce sont des pages très vues.

Or, ces pages sont constituées d'éléments qui bougent très peu :

• Les blocs d'affichage des tutos : ne changent qu'à la mise à jour du tuto
• Les blocs d'affichage des articles : changent à la mise à jour de l'article ou au post d'un nouveau commentaire... ce qui met à jour l'article (pointeur sur le dernier commentaire)
• Le bloc des unes ne change que lors de la sauvegarde d'une une
• Le menu "Tutoriels / Articles / Forum" est toujours le même pour un utilisateur donné (ou presque, ça doit bouger 2 fois dans les semaines très riches en nouvelles catégories)

Les blocs des forums (en pied de home), par contre, sont très souvent mis à jour.

Donc, j'ai mis du cache sur les 4 premiers types de blocs listés, avec les règles suivantes :

1. Les blocs des tutos sont cachés séparément.
   • Clé de cache : (ID du tuto + est-ce qu'on doit afficher la description).
   • Temps de cache : 1 heure
   • Cache du bloc invalidé quand on sauvegarde le tuto correspondant
2. Les blocs des articles sont cachés séparément.
   • Clé de cache : (ID de l'article + URL (bizarrement passé à la procédure d'affichage) + est-ce qu'on doit afficher la description).
   • Temps de cache : 1 heure
   • Cache du bloc invalidé quand on sauvegarde l'article correspondant
3. La série d'éléments "à la une" est cachée en entier
   • Clé de cache : une seule entrée de cache
   • Temps de cache : 1 heure
   • Cache du bloc invalidé quand on sauvegarde une "une" quelconque
4. Le menu "Tuto / articles / forums" est caché par utilisateur
   • Clé de cache : (ID de l'utilisateur).
   • Temps de cache : 1 heure
   • Cache du bloc invalidé uniquement sur timeout (on peut avoir donc un élément faux dans le menu pendant 1h max)

Tout ceci réduit très massivement le poids de ces pages pour un risque d'erreur de cache minime. Caches remplis :

• La page d'accueil en mode déconnecté descend à 22 requêtes, au lieu de 107 au début de la release et 57 suite à la PR Portable autom script #270, et j'espère la servir en moins de 150 ms en prod.
• La page des forums en mode admin descend à 79000 function calls

Comment QA tout ceci ?

1. Activez un système de cache quelconque autre que le DummyCache
2. Chargez des tutos, articles et forums sinon vous n'allez rien voir.
3. Vérifiez que, sur les pages suivantes, en mode connecté ou non, la page est plus rapide et fait moins de requêtes au rechargement qu'au premier chargement :
   • Accueil
   • Liste des tutos
   • Liste des articles
4. Vérifiez que si vous mettez à jour un tuto (titre, auteur) la home et la liste des tutos sont bien MAJ sans avoir à attendre 1h
5. Vérifiez que si vous mettez à jour un article (titre, auteur) la home et la liste des articles sont bien MAJ sans avoir à attendre 1h
6. Vérifiez que commenter un article met bien à jour le compteur sur la home et la liste des articles sans avoir à attendre 1h
7. Vérifiez que le bloc des "Unes" est bien mis à jour si vous en modifiez ou ajoutez une
8. Connectez vous avec un utilisateur qui a le droit de voir des forums masqués. Vérifiez :
   • Qu'il voit bien ces forums dans son menu
   • Que les membres normaux et déconnecté ne voient pas ces forums

PS : Ce message est bien plus long que la PR...

[landscape-bot]

Repository health decreased by 0.14% when pulling c7b39f3 on SpaceFox:issue_2669_perf_liste_tutos into 253a6e3 on zestedesavoir:release-v15.5.1.

• 5 new problems were found (including 0 errors and 5 code smells).
• No problems were fixed.

[Eskimon]

Travis gueule sur un truc nomme force_insert dans les fonctions save mais pourtant tu ne semblent pas avoir rajoute cet élément. C'est un side-effect du cache et de ton héritage de la fonction save ?

Sinon un petit bout de doc (equivalent de ta PR en fait) serait le bienvenu

[SpaceFox]

Non, je pense que c'est lié à ce que dit Landscape : j'aurais plutôt enlevé des éléments à save()...

[Eskimon]

Sinon pour QA tu aurais un bout de doc qque part sur "comment activer du cache" ?

[SpaceFox]

Rien de mieux que la doc Django.

[firm1]

Quelques remarques pour moi :

• Il faudrait je pense éviter de mettre les paramètres de cache en dur dans les templates. Non seulement ce n'est pas pratique pour faire de la QA (attendre 1h pour vérifier que le cache passe n'est pas top), mais le jour ou l'on veut modifier la durée on sera obligé de faire une PR juste pour ça. La bonne façon serait de l'avoir dans le settings.py
• Il faut mettre à jour la doc du projet, en expliquant un minimum comment est géré le cache sur le site. Et je dirais même de mettre à jour les doc d'installations pour rajouter l'installation de memcached (pour faciliter la QA des prochaines PRs).
• Un test unitaire serait le bienvenue, à la fois pour vérifier que le contenu est bien mis en cache et vérifier que la clé de cache est bien invalidé.
• Activer le service memcached dans travis pour le faire tester ça pour éviter des regressions futures.

[SpaceFox]

1. OK (mais il faut vraiment fouiller la doc pour voir que c'est possible)
2. OK, et on a absolument pas besoin d'avoir Memcached pour QA, n'importe quel système de cache fait le boulot (autre que DummyCache évidemment). En fait, aujourd'hui la conf utilise memcached avec le réglage cached_db, donc ça devrait marcher aujourd'hui sans toucher quoi que ce soit (utilisation du cache en base je pense)
3. Hmmm, je ne suis pas sûr du tout qu'on puisse faire un test unitaire là-dessus. En tous cas je ne vois pas comment, si tu as une idée elle est bienvenue.
4. Pourquoi pas mais c'est pas indispensable.

[firm1]

Hmmm, je ne suis pas sûr du tout qu'on puisse faire un test unitaire là-dessus. En tous cas je ne vois pas comment, si tu as une idée elle est bienvenue.

Il faudrait décrire plusieurs type de scénarios. Voici quelques pistes :

Scénario 1 (montrer que la cache fait son boulot):

• Tu fais un get sur l'url de la page des tutoriels une fois (tu compte le requets lancées)
• Tu refais par get sur la même url et tu recompte le nombre de requêtes sql.
• Tu vérifie que le le deuxième count et moins élevé que le premier count

Scénario 2 (montrer que la cache se reset quand il faut):

Idem que le scénario 1, puis tu publies un tutoriel et tu relance ton comptage de requêtes. Tu devrais avoir un peu plus que le tout premier compte.

NB : Les deux sécnarios précédents supposent que tu sais combien de requêtes tu lance en temps normal.

Scénario 3 (inspecter les clés de cache):

• Tu compte les clés de cache au départ
• Tu fais un get sur l'url des tutos
• Tu refais un second get et tu calcule le nombre de clés dans ton cache (voir l'api de cache de django)
• Tu peux encore refaire un get pour être sur que le cache fonctionne toujours (que les clés ne sont pas détruites)
• Tu valide un tutoriel et tu vérifie l'état de tes clés de cache.

On peut imaginer de nombreux autres cas.

Voilà, en gros comment je vois les tests sur le cache. Basées essentiellement sur l'api de django cache ou encore sur le nombre de requêtes SQL lancés.

[SpaceFox]

D'accord, mais en pratique, est-ce que ça marche ? Parce que tous tes tests
supposent que pendant les tests, le cache est local au test. Si le cache
n'est pas réinitialisé à chaque test (ce dont je doute très fortement),
aucun ne fonctionne correctement.

Le 19 mai 2015 14:26, firm1 notifications@github.com a écrit :

Hmmm, je ne suis pas sûr du tout qu'on puisse faire un test unitaire
là-dessus. En tous cas je ne vois pas comment, si tu as une idée elle est
bienvenue.

Il faudrait décrire plusieurs type de scénarios. Voici quelques pistes :

Scénario 1 (montrer que la cache fait son boulot):

• Tu fais un get sur l'url de la page des tutoriels une fois (tu
  compte le requets lancées
  https://docs.djangoproject.com/fr/1.8/faq/models/#how-can-i-see-the-raw-sql-queries-django-is-running
  )
• Tu refais par get sur la même url et tu recompte le nombre de
  requêtes sql.
• Tu vérifie que le le deuxième count et moins élevé que le premier
  count

Scénario 2 (montrer que la cache se reset quand il faut):

Idem que le scénario 1, puis tu publies un tutoriel et tu relance ton
comptage de requêtes. Tu devrais avoir un peu plus que le tout premier
compte.

NB : Les deux sécnarios précédents supposent que tu sais combien de
requêtes tu lance en temps normal.

Scénario 3 (inspecter les clés de cache):

• Tu compte les clés de cache au départ
• Tu fais un get sur l'url des tutos
• Tu refais un second get et tu calcule le nombre de clés dans ton
  cache (voir l'api de cache de django
  https://docs.djangoproject.com/en/1.8/topics/cache/#django.core.cache.caches
  )
• Tu peux encore refaire un get pour être sur que le cache fonctionne
  toujours (que les clés ne sont pas détruites)
• Tu valide un tutoriel et tu vérifie l'état de tes clés de cache.

On peut imaginer de nombreux autres cas.

Voilà, en gros comment je vois les tests sur le cache. Basées
essentiellement sur l'api de django cache ou encore sur le nombre de
requêtes SQL lancés.

—
Reply to this email directly or view it on GitHub
#2724 (comment)
.

[firm1]

Parce que tous tes tests supposent que pendant les tests, le cache est local au test. Si le cache n'est pas réinitialisé à chaque test (ce dont je doute très fortement), aucun ne fonctionne correctement.

L'API de django cache te permet de cleaner ton cache. Tu peux le faire avant chaque test pour t'assurer que ton contexte est le bon.

[SpaceFox]

La contrainte que tu mets sur les scénarios 1 et 2 n'est pas possible, puisque ce nombre de requête change très souvent. A la limite pour le 2 on peut vérifier qu'on a plus de requêtes qu'avant.

Le scénario 1 n'a pas d'intérêt, "vérifier que l'outil fait bien ce qu'on veut" c'est un scénario qui doit se trouver dans l'outil. Si on commence à vérifier tous nos outils, on en a jamais fini.

Le scénario 3, je ne comprends pas ce que tu essaies de vérifier avec.

PS : Dans tous les cas, ta méthode fait des tests ultra-lourds à mettre en place.

@Situphen les labels sur les PRs ne servent à rien, je pense.

[Situphen]

@SpaceFox : ils servent à trier les PRs

[SpaceFox]

Est-ce un besoin ?

2015-05-19 16:59 GMT+02:00 Situphen notifications@github.com:

@SpaceFox https://github.com/SpaceFox : ils servent à trier les PRs

—
Reply to this email directly or view it on GitHub
#2724 (comment)
.

[Situphen]

Pour plus facilement repérer ce qui est urgent (Bloquant, Régression, Bug...) oui. Après, ce n'est pas non plus vital, hein !

[firm1]

La contrainte que tu mets sur les scénarios 1 et 2 n'est pas possible, puisque ce nombre de requête change très souvent. A la limite pour le 2 on peut vérifier qu'on a plus de requêtes qu'avant.

Vérifier qu'on a moins de requête qu'avant dans la mesure du possible serait déjà très bien.

Le scénario 1 n'a pas d'intérêt, "vérifier que l'outil fait bien ce qu'on veut" c'est un scénario qui doit se trouver dans l'outil. Si on commence à vérifier tous nos outils, on en a jamais fini.

Il sert juste a vérifier l'intégration entre l'outil et l'application. On peut avoir memcached installé mais qui n'est jamais vraiment utilisé.

PS : Dans tous les cas, ta méthode fait des tests ultra-lourds à mettre en place.

ça doit faire 4 méthodes de tests à tout péter avec 10 lignes max par test sur cette PR. Je ne pense pas que ça prenne plus d'1min à s'éxecuter.

[landscape-bot]

Code quality remained the same when pulling 5c2f473 on SpaceFox:issue_2669_perf_liste_tutos into 253a6e3 on zestedesavoir:release-v15.5.1.

[SpaceFox]

Si quelqu'un comprends pourquoi mon commit 5c2f473 passe Travis mais pas le 5fd0889 ... Je me demande si activer Memcached était une vraie bonne idée. Je relance le 260a2e2 que j'avais coupé en le pensant inutile.

[SpaceFox]

D'autre part, je ne pense pas avoir le temps de comprendre les erreurs Travis et de faire des tests unitaires pour ce truc.

[landscape-bot]

Repository health decreased by 0.01% when pulling 7679441 on SpaceFox:issue_2669_perf_liste_tutos into 253a6e3 on zestedesavoir:release-v15.5.1.

• 1 new problem was found (including 0 errors and 1 code smell).
• No problems were fixed.

[SpaceFox]

Bon, Cf le bug #2727, on ne peut pas activer Memcached en l'état sans se priver des tests Travis. Donc je le supprime, et tant pis pour toute forme de test unitaire sur le cache pour l'instant.

[Situphen]

Il y a un moyen simple de connaître le nombre de requêtes faites en BDD ?

[pierre-24]

Il y a un moyen simple de connaître le nombre de requêtes faites en BDD ?

La django toolbar te les donne :)

[pierre-24]

Bon. Tout d'abord, questions (pour savoir si j'ai fait la QA convenablement):

• Est ce qu'on est bien d'accord que par défaut, la config pointe sur memcached ?
• Est ce que pour activer memcached, il suffit de taper memcached dans la première console qui passe, après l'avoir installé ?

Si tel est le cas,

• Effectivement, le nombre de requête chute assez bien: je passe de 91 à 63 sur la home, de 61 à 29 sur la liste des articles et de 46 à 30 sur celle des tutos. Ces nombres ne veulent absolument rien dire sans que je vous dise combien de chaque, mais c'est appréciable
• Le cache capte bien qu'un nouveau truc est publié (que ce soit article ou tuto)
• Par contre, quand je commente un article, pas moyen d'avoir la home qui met à jour le compteur. Pareil pour l'ajout d'un auteur ou la modif du titre. Et même en le republiant, rien à faire, memcached fait la sourde oreille. Par contre, ça marche très bien sur la page qui liste les articles
• Ok sur la home et la liste des tutos

Par contre, détail un peu bizarre: logiquement, on a "écrit par vous" dans le cas ou le tuto/l'article l'est. Mais quand on se déconnecte, on garde le "écrit par vous". Pire encore, si on emplois un autre navigateur, la page reste pareil. Et forcément, quand on se connecte avec quelqu'un de différent, même chose, des "écrit par vous et user" alors qu'on est connecté avec user.

Du coup, va falloir retirer le "vous", si on veux que cette PR voie le jour.

[SpaceFox]

Pour tes questions :

1. Oui
2. Je ne sais pas, tu es sous quel OS ? Sous Debian c'est un service.

Par contre le problème de l'invalidation m'étonne. Je soupçonne un problème avec l'URL forcée dans le cas des commentaires d'articles (un cas qui ne devrait jamais exister d'ailleurs : forcer une URL).
Le coup des auteurs / titres est plus mystérieux, à moins qu'il ne soit relié au premier.

Du coup, va falloir retirer le "vous", si on veux que cette PR voie le jour.

Si ça permet de simplifier la ligne qui crée la liste des auteurs, c'est une bonne chose, parce qu'aujourd'hui elle est à peu près incompréhensible.

[Eskimon]

Du coup, va falloir retirer le "vous", si on veux que cette PR voie le jour.

Si ça permet de simplifier la ligne qui crée la liste des auteurs, c'est une bonne chose, parce qu'aujourd'hui elle est à peu près incompréhensible.

J'ai fait une PR sur la branche a @SpaceFox

[landscape-bot]

Repository health decreased by 0.01% when pulling a00f229 on SpaceFox:issue_2669_perf_liste_tutos into 253a6e3 on zestedesavoir:release-v15.5.1.

• 1 new problem was found (including 0 errors and 1 code smell).
• No problems were fixed.

[coveralls]

Coverage increased (+0.03%) to 81.92% when pulling a00f229 on SpaceFox:issue_2669_perf_liste_tutos into 253a6e3 on zestedesavoir:release-v15.5.1.

[pierre-24]

Je ne sais pas, tu es sous quel OS ? Sous Debian c'est un service.

Debian. Du coup, si c'est un service, c'pas comme ça que je devrait le lancer, en théorie. Mais soit.

Par contre le problème de l'invalidation m'étonne. Je soupçonne un problème avec l'URL forcée dans le cas des commentaires d'articles (un cas qui ne devrait jamais exister d'ailleurs : forcer une URL).
Le coup des auteurs / titres est plus mystérieux, à moins qu'il ne soit relié au premier.

?!?

[Eskimon]

On en est ou la ?

[SpaceFox]

Il faut que je regarde ça, mais je n'ai pas eu le temps. Peut-être ce soir.

[artragis]

J'ai installé memcached sur mon instance. zds.francoisdambrine.me et j'y ai mis cette PR en exécution.

Je n'ai pas vu de doc spéciale quant à l'installation du cache je me suis donc contenté de faire un sudo apt-get install memcached puis sudo service memcached start.

[SpaceFox]

Tous les problèmes signalés provenaient de la même source et devraient être corrigés.

[landscape-bot]

Repository health decreased by 0.01% when pulling 06288d5 on SpaceFox:issue_2669_perf_liste_tutos into 253a6e3 on zestedesavoir:release-v15.5.1.

• 1 new problem was found (including 0 errors and 1 code smell).
• No problems were fixed.

[pierre-24]

Ok pour moi. Bon, évidement, ça va pas sans #2732, sans quoi on aurait un problème :)

[pierre-24]

(j'attend quand même Travis pour le principe)