It is easy to overlook select for update when using Django. Currently, I am working on a project that uses Django 1.11.x and Django REST Framework 3.8.2 (upgrade to Django 2.1 is planned, but we need to move to Python 3.7.1 first) and ran into an issue (again) of not having
.select_for_update() where it is required. The issue appears as if your changes to the database do not happen, but what actually happens is that two parallel requests (or other parallel tasks) overwrite each others data. This is because both requests retrieved a record from database and reconstructed a corresponding model instance via ORM each its own copy. It a some moment one of the requests have modified its instance attributes and saved changes to the database. But it did not modify them in the second copy that sits in the other request. At a later moment second request modifies its copy attributes and saves changes to the database. It may not be an issue (in some cases) if the same attributes are being modified - the database then would contain the most up to date values (probably what actually need), but if it is not the case we have an issue with older values being stored to the database.
With Django default behavior (at least for version 2.11) all model attributes are being saved to the database even if they were not modified. Imagine you have a model
A with attributes
b and
c. Two parallel requests get its instance from the database
instance = A(b='b1', c='c1'). Then request 1 changes
b = 'b2' and saves instance to the database:
instance.save().
A(b='b2', c='c1') will be stored to the database. Note, that although request 1 did not change attribute
c it will be stored to the database anyway. At this moment request 2 still contains its own copy as
A(b='b1', c='c1'). Then it changes
c = 'c2' and saves changes to the database:
instance.save().
A(b='b1', c='c2') will be stored to database. Again attribute
b was not changed by the request, but its value is being saved to the database by default. Therefore overwrites the value saved by request 1 (
b = 'b2') with an older value
b = 'b1' that was retrieved before. It is a typical
lost update problem (also see
write-write conflict).
In development and test environments this issue rarely reproduces. This is because of very low level of concurrency in these environments. But in production environment it may very well happen. And when it does it is really hard to debug (because it is production and because it is hard to figure out what are conditions for reproduction). Therefore this kind of issues should be prevented during development. One should make habit to query objects with
.select_for_update() if they are to be modified.
But developers still forget to do it (I do forget sometimes at least). There are two things that could be done here. First, if Django REST Framework is used it could use
.select_for_update() for all modification operations by default or at least for
PATCH and
PUT (
my question to the core developer). Second, Django should not save all attributes blindly, but only those that were modified (this will cover cases where different attributes are modified by parallel requests and also improve performance).
UPDATE: While we are waiting a reply from core developer here is a snippet for Django REST Framework:
from rest_framework import mixins
from rest_framework.generics import GenericAPIView
from rest_framework.viewsets import ViewSetMixin
class CustomGenericAPIView(GenericAPIView):
def get_queryset(self):
qs = super(CustomGenericAPIView, self).get_queryset()
if self.request.method in ('PATCH', 'PUT'):
qs = qs.select_for_update()
return qs
class CustomGenericViewSet(ViewSetMixin, CustomGenericAPIView):
pass
class CustomModelViewSet(mixins.CreateModelMixin,
mixins.RetrieveModelMixin,
mixins.UpdateModelMixin,
mixins.DestroyModelMixin,
mixins.ListModelMixin,
CustomGenericViewSet):
pass
