zestedesavoir / zds-site

Cœur du projet technique de Zeste de Savoir
https://zestedesavoir.com
Other
269 stars 161 forks source link

Risque de problème de perfs sur les longs topics #406

Closed SpaceFox closed 10 years ago

SpaceFox commented 10 years ago

Dans le code, je lis, dans /ZesteDeSavoir/zds/forum/views.py à la ligne 107

    # Retrieves all posts of the topic and use paginator with them.
    posts = Post.objects\
                .filter(topic__pk=topic.pk)\
                .order_by('position')\
                .all()

J'ai comme l'impression que sur un gros topic (plusieurs dizaines de pages), ce code va récupérer tous les posts pour pouvoir les paginer, donc devenir super lent.

Une solution serait de dénormaliser le modèle et d'ajouter la page dans le topic comme un attribut du post. Les conséquences seraient :

  1. Qu'il faut calculer cette page à l'enregistrement
  2. Que l'utilisateur ne peut pas choisir sa longueur de page / son ordre de présentation
  3. Qu'en cas de suppression de post, il faut soit recalculer les pages de tous les posts du topic, soit admettre que certaines pages auront moins de posts.
Alex-D commented 10 years ago

Ou sinon, faire un count pour le nombre de message et un count sur les messages ayant un ID plus petit et puis voilà...

SpaceFox commented 10 years ago

Je me cite, sur l'autre bug :

si j'ai bien compris ce que dit la doc et les requêtes que je vois passer, on ne récupère que les données des posts de la page (le bloc dans /ZesteDeSavoir/zds/forum/views.py ligne 105 ne correspond pas à une vraie requête). La requête réellement exécutée a une LIMIT qui correspond à la page courante.

Par contre j'ai l'impression qu'à cause des lignes 137 et 138 du même fichier on fait une requête par post présent dans la page pour en récupérer les données...

firm1 commented 10 years ago

Par contre j'ai l'impression qu'à cause des lignes 137 et 138 du même fichier on fait une requête par post présent dans la page pour en récupérer les données...

Non, non. là on manipule uniquement le queryset. On charge des queryset dans la liste. La requête ne sera lancée en vrai que lors de l'appel d'une méthode du modèle ou de la demande d'attributs.

Alex-D commented 10 years ago

C'est là qu'est tout le problème. C'est exactement comme avec Doctrine... si tu ne fais pas tes jointures en amont dans la vue (controlleur pour Sf2) c'est l'ORM qui s'en charge au moment voulu, et il te claque une requête par entrée/attribut/... et ça explose les compteurs.

firm1 commented 10 years ago

C'est là qu'est tout le problème. C'est exactement comme avec Doctrine

Django est quand même plus intelligent que Doctrine hein.

Quand tu passes des variables dans le contexte, il ne fais rien. Le calcul des requetes survient au moments ou il compte rendre les vues, et à ce moment là, normalement ta requete est déjà ULTRA sectorisée (forum précis, topic précis, page précise, etc.). Le seul truc, qui fait danser la bd c'est au moment ou il appelle des tags/filter dans les vues. C'est principalement là qu'il faut optimiser pour moi.

SpaceFox commented 10 years ago

Je crée un topic de 200 pages et regarde ce qu'il en est.

SpaceFox commented 10 years ago

Bon même avec un topic de 1000 pages (20000 posts) ça passe, parce que le Paginator est plus intelligent que ce qu'on pourrait croire.

Les 2 requêtes qui passent sont (pour le topic 1003) :

SELECT COUNT(*) FROM `forum_post` WHERE `forum_post`.`topic_id` = 1003
SELECT [...]
    FROM `forum_post`
    INNER JOIN `utils_comment` ON ( `forum_post`.`comment_ptr_id` = `utils_comment`.`id` )
    WHERE `forum_post`.`topic_id` = 1003
    ORDER BY `utils_comment`.`position` ASC
    LIMIT 21 OFFSET 19908;

ON pourrait faire mieux (order by + limit) mais ça reste très raisonnable.