Bug #69606
openPerformance: mauvaise requête SQL sur les statistiques du journal
0%
Description
On voit beaucoup de requêtes de ce genre passer sur la prod, malgré notre machine de course :
SELECT MIN("month"), MAX("month") FROM (
SELECT
DATE_TRUNC('month', "journal_event"."timestamp") AS "month",
("journal_event"."data" ->> 'service_name') AS "service_name",
COUNT("journal_event"."id") AS "count"
FROM "journal_event"
WHERE ("journal_event"."type_id" = 3 AND "journal_event"."timestamp" >= '2019-01-01T00:00:00+01:00'::timestamptz)
GROUP BY DATE_TRUNC('month', "journal_event"."timestamp"), ("journal_event"."data" ->> 'service_name')
) subquery;
Cette requête est complètement inefficace (800ms sur glc), surtout en l'absence d'index sur (type_id, timestamp).
Si on ajoute un tel index, la requête ne s'en sort pas mieux à cause de son écriture bien trop complexe.
Première simplification:
SELECT MIN("month"), MAX("month") FROM (
SELECT
DATE_TRUNC('month', "journal_event"."timestamp") AS "month"
FROM "journal_event"
WHERE ("journal_event"."type_id" = 3 AND "journal_event"."timestamp" >= '2019-01-01T00:00:00+01:00'::timestamptz)
) subquery;
On tombe à 350/400ms, ~250ms avec l'index supplémentaire, ce qui est bien mais pas encore bon.
Le cœur du problème est l'inversion de l'ordre logique : on ne veut pas le minimum/maximum du mois des événements, on veut le mois du minimum/maximum des événements.
Si on écrivait la requête comme suit, on obtiendrait un bien meilleur résultat:
SELECT DATE_TRUNC('month', MIN(timestamp)), DATE_TRUNC('month', MAX(timestamp)) FROM "journal_event"
WHERE ("journal_event"."type_id" = 3 AND "journal_event"."timestamp" >= '2019-01-01T00:00:00+01:00'::timestamptz);
Là, on tombe à moins d'une milliseconde avec l'index supplémentaire (350 sans l'index).
Côté Django, j'ai identifié le code responsable :
# src/authentic2/apps/journal/utils.py
class Statistics:
...
def __init__(self, qs, time_interval):
self.time_interval = time_interval
self.x_labels = self.build_x_labels(qs)
self._x_labels_indexes = {label: i for i, label in enumerate(self.x_labels)}
self.series = {}
self.y_labels = []
...
def build_x_labels(self, qs):
if self.time_interval == 'timestamp':
return list(qs.distinct().values_list(self.time_interval, flat=True))
aggregate = qs.aggregate(min=Min(self.time_interval), max=Max(self.time_interval))
Je vois deux façons de régler le problème :
1) dans build_x_labels, massacrer altérer l'objet qs pour en extraire les critères WHERE, les paramètres du date_trunc, et construire une nouvelle requête à partir de tout ça,
2) modifier les signatures pour pouvoir ajouter un interval_qs qui retournera le min/max de l'axe X.
Ma requête simplifiée se construit a priori bien en Django:
Event.objects.filter(type=3, timestamp__gte=datetime.datetime(2020, 1, 1)).aggregate(min=Trunc(Min("timestamp"), kind='month'), max=Trunc(Max("timestamp"), kind='month'))
Files
Updated by Pierre Ducroquet over 3 years ago
- File 0001-performance-use-a-distinct-query-for-statistics-X-ax.patch 0001-performance-use-a-distinct-query-for-statistics-X-ax.patch added
- Tracker changed from Support to Bug
- Status changed from Nouveau to Solution proposée
- Patch proposed changed from No to Yes
Premier patch, il manque au moins la migration, mais je pense que ça devrait faire le taf.
Updated by Benjamin Dauvergne over 3 years ago
- Status changed from Solution proposée to En cours
- Assignee set to Pierre Ducroquet
Il manque un import pour Min/Max (Trunc aussi peut-être).
Updated by Robot Gitea about 3 years ago
Pierre Ducroquet (pducroquet) a ouvert une pull request sur Gitea concernant cette demande :
- URL : https://git.entrouvert.org/entrouvert/authentic/pulls/15
- Titre : WIP: improve statistics performance (#69606)
- Modifications : https://git.entrouvert.org/entrouvert/authentic/pulls/15/files
Updated by Benjamin Dauvergne 2 months ago
- Assignee changed from Pierre Ducroquet to Benjamin Dauvergne
Updated by Benjamin Dauvergne 2 months ago
🤖 Une pull request concernant ce ticket a été ouverte :
- URL : https://git.entrouvert.org/entrouvert/authentic/pulls/658
- Titre : WIP: performance: use a distinct query for statistics X axis (#69606)
- Modifications : https://git.entrouvert.org/entrouvert/authentic/pulls/658/files
Updated by Benjamin Dauvergne 2 months ago
- Status changed from En cours to Solution proposée
Updated by Benjamin Dauvergne 2 months ago
J'étais optimiste sur ce patch, les mesures de temps de requêtes semblaient juste et l'ajour de l'index sur (type, timestamp) pertinente; mais au final je me demande si la recherche du Trunc(group_time, Min/Max('timestamp')) sert à quelque chose. Il me semble qu'on pourrait juste obtenir ces valeurs à la volée en parcourant le queryset.
Peut-être qu'en revoyant l'interaction entre get_global_statistics() et la classe Statistic tout deviendrait plus simple, i.e. laisser à Statistic le job de charger le qs (au lieu d'une boucle qui fait des stats.add(...)) et ne pas faire build_x_labels() en avance mais à la volée.
Updated by Benjamin Dauvergne 2 months ago
- Status changed from Solution proposée to Information nécessaire
- Assignee changed from Benjamin Dauvergne to Valentin Deniaud
Updated by Valentin Deniaud 2 months ago
Benjamin Dauvergne a écrit (#note-8):
Il me semble qu'on pourrait juste obtenir ces valeurs à la volée en parcourant le queryset.
Oui j'ai la même lecture
On pourrait même simplement retirer tout le code d’agrégation par mois/année et de bouchage des trous pour les jours, car combo sait désormais gérer une réaggrégation (#53180)
Updated by Valentin Deniaud 2 months ago
- Status changed from Information nécessaire to Nouveau
- Assignee deleted (
Valentin Deniaud)