Cache menu par fragments #3736

vhf · 2016-07-23T16:13:02Z

Q	R
Type de modification	correction de bug
Ticket(s) (issue(s)) concerné(s)	#3695 #3607

QA

Vérifier que l'onglet de menu .current est bien celui sur lequel on se trouve après avoir navigué entre différentes sections du site.
Vérifier qu'on se prend la requête de groupe qu'une seule fois au premier chargement, et idem pour le menu. Normalement ça fait une grosse différence. Sur une page comme /pages/apropos, on passe de 10 requêtes au premier chargement à 0 (oui, 0) requête aux chargements suivants par tranche de 30 minutes. (0 pour visiteur pas connecté, hein, sinon y'a quand même les notifs, toussa)

Notes

Je pense que c'est bien si cette régression est réparée dans la v20. C'est moche comme bug, et le fix est bénin.
Ajout d'un filtre groups à appliquer à la variable user dans les templates. Son contenu est toujours caché, donc on peut l'utilisé comme clé partout où on a besoin de cacher un fragment d'après les groupes. Vu qu'on change rarement de groupes et qu'il s'agit que de visuel, pas de fonctionnalité, j'ai mis un cache de 4h.

coveralls · 2016-07-23T16:19:30Z

Coverage remained the same at 87.786% when pulling 810ad94 on vhf:fix-3695 into bba9bd7 on zestedesavoir:dev.

SpaceFox · 2016-07-23T16:20:16Z

templates/base.html

+                                <a href="{% url "tutorial:list" %}" class="mobile-menu-link {% block menu_tutorial %}{% endblock %}">
+                                    {% trans "Tutoriels" %}
+                                </a>
+                                {% cache 1800 menu_tutorial user.pk|default_if_none:"0" %}


Est-ce que les menus des tutos et des articles peuvent être différents selon l'utilisateur ?

Je ne crois pas. J'ai pas lu beaucoup de doc à ce sujet, mais le fragment caché semble uniquement identifié par la clé de cache. Du coup on pourrait pas avoir plusieurs fragments cachés avec la même clé. Je pense pas que ça ferait vraiment une différence de toute façon.

Ici on aurait la situation inverse : le même fragment caché plein de fois avec des clés différentes (puisque tu définis user.pk comme clé de cache alors qu'en fait ce n'est pas un critère différenciant). C'est pas très important mais dommage.

Ah j'avais compris que tu proposais autre chose. Très bien vu en effet.

Pourquoi la clé originale était l'user.pk @gustavi ? Dans quels cas le menu diffère entre deux utilisateurs ? Est-ce une affaire de groupe utilisateur ? Autre ?

J'ai l'impression que connecté et déconnecté le contenu de ces 3 menus est le même. Du coup on peut avoir qu'un seul cache par menu pour tous les utilisateurs, au lieu d'avoir un cache par menu par utilisateur, non ? Le cache hit sera nettement plus élevé, mais l'empreinte mémoire très très nettement plus basse, du coup on peut sans problème réduire la durée du cache à genre 10min.

Ok, donc j'avais pas fait gaffe mais genre j'ai accès au menu ||CA de l'association||. Du coup il est dans mon menu. Donc on peut pas avoir le même cache pour tout le monde.

Peut-on le baser sur les groupes ? Genre un join des groupes ?

Le menu forums est personnalisé (au moins par groupe). Les autres je ne pense pas, d'où mon commentaire.

Ok.

Du coup un fix intéressant et plus puissant que la solution en place ou la présente PR serait de se baser sur les groupes et le flag is_staff.

Il suffit de créer une clé, pour ça y'a plein d'options, en voici 2 :

groups = User.objects.filter(username='victor').prefetch_related('groups').values_list('groups', 'is_staff') key = '-'.join([str(int(group)) + str(int(staff)) for (group, staff) in groups]) # ou alors groups_pks = User.objects.filter(username='victor').prefetch_related('groups').values_list('groups', flat=True) key = '{}-{}'.format( str(int(self.is_staff)), '-'.join(groups_pks) )

Ça c'est la partie facile. Maintenant, la question que je me pose, c'est où faire ça ? Si on fait un filter pour ça auquel on passe notre utilisateur et qu'on l'appelle à chaque cache_fragment, on ajoute une requête par fragment, ce qui est con. Du coup où ?

J'aime bien baser le clé sur les groupes. Par contre, le flag is_staff n'est pas utilisé au sein de ZdS (de mémoire).

L'option de @vhf avec les groupes me semble la meilleure option !

@GerardPaligot Il me semble que si. Tous les gens qui ont le badge "staff" semblent être is_staff=True.

Je suis ravi d'implémenter cette histoire de groupe, mais j'aimerais bien quand même vos avis sur mes questions de mon précédent commentaire.

vhf · 2016-07-28T22:47:13Z

à QA

GerardPaligot · 2016-07-29T07:53:51Z

Par hasard, tu connais la procédure pour avoir un cache actif en local ?

PS : C'est moche de fail sur des erreurs lint. :)

vhf · 2016-07-29T08:14:03Z

Yep @GerardPaligot https://github.com/vhf/zds-site/blob/10cac916752eee0952823f08dde88c1421290dc9/zds/settings_test_local.py

Le caching casse un test pas très réaliste. J'enlève la partie du test qui passe pas ?

GerardPaligot · 2016-07-29T09:20:34Z

Le caching casse un test pas très réaliste. J'enlève la partie du test qui passe pas ?

Pourquoi le forum ne serait-il plus afficher suite au cache ? De mémoire, il est disponible dans le fil d'Ariane et ne devrait donc pas rentrer dans le cache.

vhf · 2016-07-29T11:29:25Z

il est disponible dans le fil d'Ariane et ne devrait donc pas rentrer dans le cache.

Il le serait si le test était correct. En l'occurrence le test passait par accident, grâce au menu.

GerardPaligot · 2016-07-29T11:33:24Z

rofl ... :)

Du coup, le forum11 n'est pas affiché parce qu'on a fait une requête avant sans le forum11 ?

coveralls · 2016-07-29T11:35:47Z

Coverage increased (+0.01%) to 87.712% when pulling 152f13e on vhf:fix-3695 into 7178e1e on zestedesavoir:dev.

vhf · 2016-07-29T12:05:53Z

Pas exactement @GerardPaligot . Ces forums et catégories sont créés dans le setUp, avant chaque test.

Du coup la première fois qu'on y passe, on créé des catégories et des forums etc. Ensuite à la première requête HTTP, le menu se retrouve en cache. Puis au prochain test le setUp en créera d'autres avec un nom contenant un ID incrémenté.

Le cache n'est pas donc presque jamais synchro.

Pour te donner un exemple, si tu lances seulement les tests de zds.forum, au premier passage t'auras

Mon Forum No73
Mon Forum No74
Mon Forum No75
Mon Forum No76
Mon Forum No77

ensuite y'aura quelques tests sur d'autres forums, puis le test va casser là :

Mon Forum No88
Mon Forum No89
Mon Forum No90
Mon Forum No91
Mon Forum No92

puis y'a plein d'autres tests qui passent, et le dernier sera genre :

Mon Forum No173
Mon Forum No174
Mon Forum No175
Mon Forum No176
Mon Forum No177

Mais à ce moment là, dans le cache du menu, on a toujours

Mon Forum No73
Mon Forum No74
Mon Forum No75
Mon Forum No76
Mon Forum No77

Ce qui est normal et attendu. C'est pour ça que je disais que le test n'était pas réaliste. Il simule une situation où chaque seconde on créé 5 forums et on les supprime après quelques fractions de secondes.

GerardPaligot · 2016-07-31T20:31:17Z

Je confirme le 0 requête (GG) et j'ai bien un cache correct.

QA ok donc.

vhf added C-Front Concerne l'interface du site S-Régression Corrige un problème sur un composant qui fonctionnait auparavant labels Jul 23, 2016

SpaceFox reviewed Jul 23, 2016
View reviewed changes

GerardPaligot changed the title ~~(fix) Cache menu par fragments~~ Cache menu par fragments Jul 27, 2016

(fix) Cache menu par fragments

152f13e

GerardPaligot merged commit 25fbddf into zestedesavoir:dev Jul 31, 2016

GerardPaligot added this to the Version de développement milestone Jul 31, 2016

vhf deleted the fix-3695 branch August 15, 2016 19:37

vhf mentioned this pull request Aug 23, 2016

Beta test v20 #3787

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cache menu par fragments #3736

Cache menu par fragments #3736

vhf commented Jul 23, 2016 •

edited

Loading

coveralls commented Jul 23, 2016 •

edited

Loading

SpaceFox Jul 23, 2016

vhf Jul 23, 2016 •

edited

Loading

SpaceFox Jul 23, 2016

vhf Jul 23, 2016 •

edited

Loading

vhf Jul 23, 2016 •

edited

Loading

SpaceFox Jul 23, 2016

vhf Jul 23, 2016 •

edited

Loading

GerardPaligot Jul 27, 2016

gustavi Jul 28, 2016

vhf Jul 28, 2016

vhf commented Jul 28, 2016

GerardPaligot commented Jul 29, 2016

vhf commented Jul 29, 2016 •

edited

Loading

GerardPaligot commented Jul 29, 2016

vhf commented Jul 29, 2016

GerardPaligot commented Jul 29, 2016

coveralls commented Jul 29, 2016 •

edited

Loading

vhf commented Jul 29, 2016

GerardPaligot commented Jul 31, 2016

Cache menu par fragments #3736

Cache menu par fragments #3736

Conversation

vhf commented Jul 23, 2016 • edited Loading

QA

Notes

coveralls commented Jul 23, 2016 • edited Loading

SpaceFox Jul 23, 2016

Choose a reason for hiding this comment

vhf Jul 23, 2016 • edited Loading

Choose a reason for hiding this comment

SpaceFox Jul 23, 2016

Choose a reason for hiding this comment

vhf Jul 23, 2016 • edited Loading

Choose a reason for hiding this comment

vhf Jul 23, 2016 • edited Loading

Choose a reason for hiding this comment

SpaceFox Jul 23, 2016

Choose a reason for hiding this comment

vhf Jul 23, 2016 • edited Loading

Choose a reason for hiding this comment

GerardPaligot Jul 27, 2016

Choose a reason for hiding this comment

gustavi Jul 28, 2016

Choose a reason for hiding this comment

vhf Jul 28, 2016

Choose a reason for hiding this comment

vhf commented Jul 28, 2016

GerardPaligot commented Jul 29, 2016

vhf commented Jul 29, 2016 • edited Loading

GerardPaligot commented Jul 29, 2016

vhf commented Jul 29, 2016

GerardPaligot commented Jul 29, 2016

coveralls commented Jul 29, 2016 • edited Loading

vhf commented Jul 29, 2016

GerardPaligot commented Jul 31, 2016

vhf commented Jul 23, 2016 •

edited

Loading

coveralls commented Jul 23, 2016 •

edited

Loading

vhf Jul 23, 2016 •

edited

Loading

vhf Jul 23, 2016 •

edited

Loading

vhf Jul 23, 2016 •

edited

Loading

vhf Jul 23, 2016 •

edited

Loading

vhf commented Jul 29, 2016 •

edited

Loading

coveralls commented Jul 29, 2016 •

edited

Loading