Django @transaction.atomic() to prevent creating objects in concurrency

Question

I have a ticket model, and its ticket serialiazer. The ticket model has a bought and a booked_at field. And also a unique_together attribute for show and seat.

class Ticket(models.Model):
    show = models.ForeignKey(Show, on_delete=models.CASCADE)
    seat = models.ForeignKey(Seat, on_delete=models.CASCADE)
    user = models.ForeignKey(User, on_delete=models.CASCADE)
    booked_at = models.DateTimeField(default=timezone.now)
    bought = models.BooleanField(default=False)

    class Meta:
        unique_together = ('show', 'seat')

• On the ticket serializer, the serializer on validation checks whether there is any ticket with the required seat and show
  • If there is a ticket then it checks whether the ticket was bought or not.
    • If it is bought, then it will raise an error.
    • If its not bought then Check if the ticket was booked within 5 minutes.
      • If its booked within 5 minutes, then raise an error.
      • Else if the booked time is more than 5 minutes, then delete the old ticket and return valid.
• If there is not ticket then return valid

TicketSerializer:

class TicketSerializer(serializers.Serializer):
    seat = serializers.PrimaryKeyRelatedField(queryset=Seat.objects.all())
    show = serializers.PrimaryKeyRelatedField(queryset=Show.objects.all())
    user = serializers.PrimaryKeyRelatedField(queryset=User.objects.all())
    bought = serializers.BooleanField(default=False)

    def validate(self, attrs):
        if attrs['seat']:
            try:
                ticket = Ticket.objects.get(show=attrs['show'], seat=seat)
                if not ticket.bought:
                    if ticket.booked_at < timezone.now() - datetime.timedelta(minutes=5):
                        # ticket booked crossed the deadline
                        ticket.delete()
                        return attrs
                    else:
                        # ticket in 5 mins range
                        raise serializers.ValidationError("Ticket with same show and seat exists.")
                else:
                    raise serializers.ValidationError("Ticket with same show and seat exists.")
            except Ticket.DoesNotExist:
                return attrs
        else:
            raise serializers.ValidationError("No seat value provided.")

On the view, I am using @transaction.atomic() to make sure the ticket/s are created only if all of them are valid, or don't create ANY ticket if not valid.

@transaction.atomic()
@list_route(
    methods=['POST'],
    permission_classes=[IsAuthenticated],
    url_path='book-tickets-by-show/(?P<show_id>[0-9]+)'
)
def book_tickets_by_show(self, request, show_id=None):
    try:
        show = Show.objects.get(id=show_id)
        user = request.user
        ...
        ...
        data_list = [...]
        with transaction.atomic():
            try:
                serializer = TicketSerializer(data=data_list, many=True)
                if serializer.is_valid():
                    serializer.save()
                    ....
                return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)
            except (Seat.DoesNotExist, ValueError, ConnectionError) as e:
                return Response({'detail': str(e)}, status=status.HTTP_400_BAD_REQUEST)
    except (Show.DoesNotExist, IntegrityError) as e:
        return Response({'detail': str(e)}, status=status.HTTP_400_BAD_REQUEST)

What I would like to know is will it help in preventing when more than one requests are called for creating the ticket/s for same seat/s?

Suppose, User A wants to book ticket for seats 5,6. User B wants to book ticket for seats 3,6, and another User C wants to book the ticket for seats 2,3,4,5,6.

Will the above method prevent booking tickets for their respective seats for all the users, and only create tickets for one user (maybe whose transaction was first)? Or if there is a better way then could you please tell me how to. I hope I was clear. If not please ask.

Answer 1

Will it help in preventing when more than one requests are called for creating the ticket/s for same seat/s.

Yes, it will. The unique_together constraint plus transaction.atomic() will ensure that you cannot create two tickets for the same seat/show.

That said, there are a couple of issues with your current approach:

1. I think it's unnecessary to wrap the whole view as well as the bit that does the saving in atomic() - you don't need to do both and wrapping the whole view in a transaction comes at a performance cost. Wrapping the serializer.save() in a transaction should be sufficient.

2. It's not advisable to catch exceptions inside a transaction - see the warning in the documentation. It is also generally preferable to catch exceptions as close as possible to the code that can generate them, to avoid confusion. I'd suggest refactoring the code to something like this.

   try:
       show = Show.objects.get(id=show_id)
   # Catch this specific exception where it happens, rather than at the bottom.
   except Show.DoesNotExist as e:
       return Response({'detail': str(e)}
   
   user = request.user
   ...
   ...
   data_list = [...]
   
   try:
       serializer = TicketSerializer(data=data_list, many=True)
       if serializer.is_valid():
           try:
               # Note - this is now *inside* a try block, not outside
               with transaction.atomic():
                   serializer.save()
                   ....
           except IntegrityError as e:
               return Response({'detail': str(e), status=status.HTTP_400_BAD_REQUEST}
   
       return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)
   # Retained from your code - althought I am not sure how you would 
   # end up with ever get a Seat.DoesNotExist or ValueError error here
   # Would be better to catch them in the place they can occur.
   except (Seat.DoesNotExist, ValueError, ConnectionError) as e:
       return Response({'detail': str(e)}, status=status.HTTP_400_BAD_REQUEST)
   

Answer 2

You should use an explicit distributed lock to synchronize the requests, rather than relying on transaction.atomic which is not meant to be a lock.

There are various ways to implement a lock but on our Django/Gunicorn project we're using Python's own multiprocessing.Lock to ensure that requests enter a block of code one at a time. It's a relatively simple solution that works for us.

import multiprocessing

_lock = multiprocessing.Lock()

_lock.acquire()
try:
    # Some code that needs to be accessed by one request a time
finally:
    _lock.release()

Answer 3

How about the following creating an intermittent table

class showAndSeat(models.Model):
    show = models.ForeignKey(Show, on_delete=models.CASCADE)
    seat = models.ForeignKey(Seat, on_delete=models.CASCADE)
    showtime = models.DateTimeField(default=timezone.now)
    ...

    class Meta:
        unique_together = ('show', 'seat', 'showtime')

your existing class Ticket will have a foreign key to showAndSeat ( The only constraing being that you have to create the showAndSeat using some cron )

Change the existing view to

def book_tickets_by_show(self, request, show_id=None):
    ....
    ...
    ...
    try:
        with transaction.atomic():
           seat_list_from_user = [1,2,3,4] # get the list from the request
           lock_ticket = showAndSeat.objects.select_for_update(nowait=True).filter(seat__number__in=seat_list_from_user,show = selected_show_timings)
           serializer = TicketSerializer(data=data_list, many=True)
            if serializer.is_valid():
                serializer.save()
           return GOOD_Response()
    except  DatabaseError :
        # Tickets are locked by some one else 
    except (Show.DoesNotExist, IntegrityError) as e:
        return Response({'detail': str(e)}, status=status.HTTP_400_BAD_REQUEST)        
    except :
        # some other unhandled error 
        return BAD_RESPONSE()