Opened 6 years ago

Closed 6 years ago

#29786 closed New feature (wontfix)

Add option to lock rows with select_for_update() immediately

Reported by: ovalseven8 Owned by: nobody
Component: Database layer (models, ORM) Version: 2.1
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by ovalseven8)

In my web application I usually follow the "fat models, lightweight views" philosophy.

To put it in a nutshell, I have two models in my app called "Event" and "Registration (for Event)":

class Event(models.Model):
    capacity = models.PositiveSmallIntegerField()

    def get_number_of_registered_tickets():
        return EventRegistration.objects.filter(event__exact=self).aggregate(total=Coalesce(Sum('number_tickets'), 0))['total']
    
    @transaction.atomic
    def reserve_tickets(self, number_tickets):
        Event.objects.filter(id=self.id).select_for_update()
        if self.get_number_of_registered_tickets() + number_tickets <= self.capacity:
            # create EventRegistration
        else:
            # handle error


class EventRegistration(models.Model):
    time = models.DateTimeField(auto_now_add=True)
    event = models.ForeignKey(Event, on_delete=models.CASCADE)
    number_tickets = models.PositiveSmallIntegerField(validators=[MinValueValidator(1)])

Now, let's say for a specific event only one ticket is left and two users want to buy that ticket concurrently. Under unfortunate circumstances (when I do not use locking), it would be possible that both get the ticket and now I have a problem. So, I need to make sure that the registration for an event happens in sequence. That's the reason why I use select_for_update() in the method reserve_tickets() above.

Unfortunately, the row is only locked when the QuerySet is evaluates what's not the case here. So, I need more or less a "dirty hack" like printing the queryset or creating a list etc.

While this works, I do not think it's a nice solution. Especially for locking rows, I think it would be a good think if Django had a "lock/evaluate immediately" option.

Different ideas how the solution API could look like:
select_for_update().evaluate()
select_for_update(evaluate_immediately=True)

Of course, there're more possibilities. I think it would be a nice little feature and lazy evaluation is something you perhaps do not want for locking.

So my question is if you're open for that, if yes I could look into it.

Change History (4)

comment:1 by ovalseven8, 6 years ago

I missed the @transaction.atomic() decorator in the method above. Unfortunately, I cannot edit it because your SpamProtection system prevents it. :/

Version 0, edited 6 years ago by ovalseven8 (next)

comment:2 by ovalseven8, 6 years ago

Description: modified (diff)

Add decorator

comment:3 by Tom Forbes, 6 years ago

In your example above, would it not be possible to do:

def reserve_tickets(self, number_tickets):
    event = Event.objects.select_for_update().get(id=self.id)
    if event.blah ...

As a workaround? Rather than add an 'immediate' parameter to select_for_update perhaps a context manager might be more appropriate?

comment:4 by Simon Charette, 6 years ago

Resolution: wontfix
Status: newclosed

I agree with Tom,

Adding a kwarg to select_for_update() or another method to break the lazy nature of a queryset seems unnecessary when get() works just fine the use case reported here.

Note: See TracTickets for help on using tickets.
Back to Top