Bug #55393
IntegrityError: duplicate key value violates unique constraint "agendas_event_agenda_id_slug_9ab8727a_uniq"
0%
Description
https://sentry.entrouvert.org/entrouvert/publik/issues/49312/
Event.DoesNotExist: Event matching query does not exist.
File "chrono/agendas/models.py", line 1494, in get_or_create_event_recurrence
return Event.objects.get(agenda=self.agenda, slug=event.slug)
File "django/db/models/manager.py", line 82, in manager_method
return getattr(self.get_queryset(), name)(*args, **kwargs)
File "django/db/models/query.py", line 408, in get
self.model._meta.object_name
IntegrityError: duplicate key value violates unique constraint "agendas_event_agenda_id_slug_9ab8727a_uniq"
DETAIL: Key (agenda_id, slug)=(235, vdm-participer-aux-vaccinations-a-saint-symphorien-infirmiers-event-5--2021-08-07-0800) already exists.
File "django/db/backends/utils.py", line 84, in _execute
return self.cursor.execute(sql, params)
IntegrityError: duplicate key value violates unique constraint "agendas_event_agenda_id_slug_9ab8727a_uniq"
DETAIL: Key (agenda_id, slug)=(235, vdm-participer-aux-vaccinations-a-saint-symphorien-infirmiers-event-5--2021-08-07-0800) already exists.
(21 additional frame(s) were not displayed)
...
File "django/db/backends/utils.py", line 67, in execute
return self._execute_with_wrappers(sql, params, many=False, executor=self._execute)
File "django/db/backends/utils.py", line 76, in _execute_with_wrappers
return executor(sql, params, many, context)
File "django/db/backends/utils.py", line 84, in _execute
return self.cursor.execute(sql, params)
File "django/db/utils.py", line 89, in __exit__
raise dj_exc_value.with_traceback(traceback) from exc_value
File "django/db/backends/utils.py", line 84, in _execute
return self.cursor.execute(sql, params)
Fichiers
Révisions associées
Historique
Mis à jour par Valentin Deniaud il y a presque 3 ans
J'ai regardé sans succès, le code
with transaction.atomic():
try:
return Event.objects.get(agenda=self.agenda, slug=event.slug)
except Event.DoesNotExist:
event.save()
return event
lève l'exception sur la clé (agenda, slug) et je vois pas comment c'est possible.
Mis à jour par Paul Marillonnet il y a presque 3 ans
- Fichier 0001-agendas-don-t-mix-db-operation-and-exception-handlin.patch 0001-agendas-don-t-mix-db-operation-and-exception-handlin.patch ajouté
- Statut changé de Nouveau à Solution proposée
- Patch proposed changé de Non à Oui
Oui c’est ce bout de code qui pose problème, c’est un comportement non déterministe de django, i.e. écrire en base lors de la capture d’une exception à l’intérieur d’un bloc atomic.
Je crois qu’il vaut mieux faire quelque chose comme ça.
Mis à jour par Lauréline Guérin il y a presque 3 ans
Ca marchera mieux avec ta proposition, mais alors on commencera toujours par faire un create, puis un get, à partir du moment où l'event existera en DB.
C'est dommage de devoir faire 2 requêtes alors qu'une suffirait dans tous les cas sauf la première fois :)
Mis à jour par Paul Marillonnet il y a presque 3 ans
Lauréline Guerin a écrit :
Ca marchera mieux avec ta proposition, mais alors on commencera toujours par faire un create, puis un get, à partir du moment où l'event existera en DB.
C'est dommage de devoir faire 2 requêtes alors qu'une suffirait dans tous les cas sauf la première fois :)
Ah oui, mauvaise connaissance de ce code de ma part, je n’avais pas réalisé que c’est la lecture en base d’une occurrence existante qui aura lieu plus souvent que l’écriture d’une nouvelle occurrence.
Je crois qu’avec une solution comprenant le get d’abord, il faut un checkpoint intermédiaire. Je vais voir si ça tient la route.
Mis à jour par Lauréline Guérin il y a presque 3 ans
Et sinon, en transformant le code existant pour avoir un get_or_create ? Django gère très bien ce cas :) (et il commence par tenter un get)
Mis à jour par Valentin Deniaud il y a presque 3 ans
Le code a cette tête parce que je pensais qu'en mettant le try/except dans un transaction.atomic, un objet ne pouvait pas être créé entre le get et le create, mais en fait ça n'est pas du tout le cas, donc le code est nul avant même de considérer que c'est mal de rattraper une exception à l'intérieur du bloc.
Ce que je voulais faire c'est bien un get_or_create à la Django, comme ça https://github.com/django/django/blob/main/django/db/models/query.py#L573.
Mis à jour par Paul Marillonnet il y a presque 3 ans
Valentin Deniaud a écrit :
Le code a cette tête parce que je pensais qu'en mettant le try/except dans un transaction.atomic, un objet ne pouvait pas être créé entre le get et le create, mais en fait ça n'est pas du tout le cas, donc le code est nul avant même de considérer que c'est mal de rattraper une exception à l'intérieur du bloc.
Ce que je voulais faire c'est bien un get_or_create à la Django, comme ça https://github.com/django/django/blob/main/django/db/models/query.py#L573.
J’étais en train d’écrire une réponse en référençant précisément cette ligne (mais dans la version 2.2 de django, qui fait apparaître une méthode _create_object_from_params¹) :)
En effet on peut s’inspirer de ça, i.e. (a) commencer par essayer un get, et sur inexistance de l’objet, (b) basculer sur une transaction (telle que proposée dans le précédent patch) dont l’échec par une erreur d’intégrité provoque (c) un dernier get.
1. https://github.com/django/django/blob/stable/2.2.x/django/db/models/query.py#L567
Mis à jour par Paul Marillonnet il y a presque 3 ans
- Statut changé de Solution proposée à En cours
Mis à jour par Paul Marillonnet il y a presque 3 ans
Petite question au passage : quitte à reproduire le comportement django-esque, est-ce qu’on en profite pour s’aligner sur ce que django retourne sur get_or_create (non pas l’objet seul mais le tuple (object, created)) ? Est-ce que ça aurait un intérêt ici ?
Mis à jour par Valentin Deniaud il y a presque 3 ans
Paul Marillonnet a écrit :
Est-ce que ça aurait un intérêt ici ?
Nop, faire au plus simple.
Mis à jour par Paul Marillonnet il y a presque 3 ans
- Fichier 0001-mitigate-race-condition-while-get-or-creating-reccur.patch 0001-mitigate-race-condition-while-get-or-creating-reccur.patch ajouté
- Statut changé de En cours à Solution proposée
Ok. Une version simple qui reprend l’esprit du get_or_create de django. Sans test, je suis à court d’idée sur comment simuler l’accès concurrent en échec :)
Mis à jour par Valentin Deniaud il y a presque 3 ans
- Statut changé de Solution proposée à Solution validée
Mis à jour par Paul Marillonnet il y a presque 3 ans
- Statut changé de Solution validée à Résolu (à déployer)
commit bd51c5138a6da218013999d191479d8adb131cda
Author: Paul Marillonnet <pmarillonnet@entrouvert.com>
Date: Wed Jul 7 17:49:30 2021 +0200
mitigate race condition while get-or-creating reccurent event (#55393)
inspired from django's behavior on get_or_create, e.g.
https://github.com/django/django/blob/stable/-
2.2.x/django/db/models/query.py#L567
Mis à jour par Frédéric Péters il y a presque 3 ans
- Statut changé de Résolu (à déployer) à Solution déployée
mitigate race condition while get-or-creating reccurent event (#55393)