25

I'm trying to implement (what I think is) a pretty simple data model for a counter:

class VisitorDayTypeCounter(models.Model):
    visitType = models.CharField(max_length=60)
    visitDate = models.DateField('Visit Date')
    counter = models.IntegerField()

When someone comes through, it will look for a row that matches the visitType and visitDate; if this row doesn't exist, it will be created with counter=0.

Then we increment the counter and save.

My concern is that this process is totally a race. Two requests could simultaneously check to see if the entity is there, and both of them could create it. Between reading the counter and saving the result, another request could come through and increment it (resulting in a lost count).

So far I haven't really found a good way around this, either in the Django documentation or in the tutorial (in fact, it looks like the tutorial has a race condition in the Vote part of it).

How do I do this safely?

7 Answers 7

29

As of Django 1.1 you can use the ORM's F() expressions.

from django.db.models import F
product = Product.objects.get(name='Venezuelan Beaver Cheese')
product.number_sold = F('number_sold') + 1
product.save()

For more details see the documentation:

https://docs.djangoproject.com/en/1.8/ref/models/instances/#updating-attributes-based-on-existing-fields

https://docs.djangoproject.com/en/1.8/ref/models/expressions/#django.db.models.F

2
  • Cool! That's an awesome approach. If only that had been there when I was working on this project. Dec 25, 2009 at 4:15
  • For modern Django installations, this is the correct answer and should be reflected as such by the OP.
    – claymation
    May 23, 2012 at 18:18
12

If you truly want the counter to be accurate you could use a transaction but the amount of concurrency required will really drag your application and database down under any significant load. Instead think of going with a more messaging style approach and just keep dumping count records into a table for each visit where you'd want to increment the counter. Then when you want the total number of visits do a count on the visits table. You could also have a background process that ran any number of times a day that would sum the visits and then store that in the parent table. To save on space it would also delete any records from the child visits table that it summed up. You'll cut down on your concurrency costs a huge amount if you don't have multiple agents vying for the same resources (the counter).

4
  • Hey good call! I've been doing App Engine work primarily, and I got hung up on "transactions only act on one entry" and "doing aggregate functions is very expensive". That's a really simple way to solve the problem. Thanks! Nov 11, 2008 at 18:27
  • I suppose it does depend on whether the process is going to be read-heavy or write-heavy. Counts are going to be read much more often than they'll be incremented in my system, so for the problem as stated, this might not be the best plan. However, it does solve other concerns I had, so thanks! Nov 11, 2008 at 19:28
  • Depending on how stale the counts can be allowed to be, you could have a background process summing them up every so often. Then you would not be doing the aggregation per request.
    – Sam Corder
    Nov 11, 2008 at 19:35
  • Interesting to know that this journaling method is similar to how transactions are made safe, atomic and recoverable under the hood in many databases (see the implementation details in Postgresql manual for instance).
    – Tom Leys
    Nov 12, 2008 at 3:18
6

You can use patch from http://code.djangoproject.com/ticket/2705 for support database level locking.

With patch this code will be atomic:

visitors = VisitorDayTypeCounter.objects.get(day=curday).for_update()
visitors.counter += 1
visitors.save()
1
  • That is wicked cool. I didn't see that when I first asked the question (3 years ago!) Sep 17, 2011 at 4:58
5

Two suggestions:

Add a unique_together to your model, and wrap the creation in an exception handler to catch duplicates:

class VisitorDayTypeCounter(models.Model):
    visitType = models.CharField(max_length=60)
    visitDate = models.DateField('Visit Date')
    counter = models.IntegerField()
    class Meta:
        unique_together = (('visitType', 'visitDate'))

After this, you could stlll have a minor race condition on the counter's update. If you get enough traffic to be concerned about that, I would suggest looking into transactions for finer grained database control. I don't think the ORM has direct support for locking/synchronization. The transaction documentation is available here.

2
  • The unique_together certainly makes me feel a bit more comfortable. Likely, there won't ever be enough traffic on this to cause the race to get hit, but since I'm learning Django at the same time, I figured I wanted to "do it right". Thanks for the help! Nov 11, 2008 at 6:07
  • Yeah, I hear you. Maybe someone else around here will be aware of an ORM feature for handling this, or can clear up if some of the built-ins are safe against this scenario. Nov 11, 2008 at 6:35
1

This is a bit of a hack. The raw SQL will make your code less portable, but it'll get rid of the race condition on the counter increment. In theory, this should increment the counter any time you do a query. I haven't tested this, so you should make sure the list gets interpolated in the query properly.

class VisitorDayTypeCounterManager(models.Manager):
    def get_query_set(self):
        qs = super(VisitorDayTypeCounterManager, self).get_query_set()

        from django.db import connection
        cursor = connection.cursor()

        pk_list = qs.values_list('id', flat=True)
        cursor.execute('UPDATE table_name SET counter = counter + 1 WHERE id IN %s', [pk_list])

        return qs

class VisitorDayTypeCounter(models.Model):
    ...

    objects = VisitorDayTypeCounterManager()
5
  • Isn't it still possible that the database will execute this query over two separate connections concurrently and still have a (much lower probability) race condition? It all depends on hidden transactions around this query implimented by the connection layer that make op atomic.
    – Tom Leys
    Nov 12, 2008 at 3:23
  • If you watch the "Why I Hate Django" keynote from DjangoCon, this type of query is given as a proper, race condition-free way to do an increment in SQL (the hitch being that Django's ORM can't do it for you).
    – iconoplast
    Nov 12, 2008 at 13:58
  • I'll watch your slides... you've pretty much confirmed my suspicion that the ORM wouldn't do it on its own. Thanks for the help! Nov 12, 2008 at 14:03
  • 1
    If you look at Django Ticket 7210 – "Added expression support for QuerySet.update", it may seem that help is on the way to do this in a portable way using the ORM...
    – Ber
    Nov 14, 2008 at 12:34
  • 1
    This answer may have been valid once but it is outdated for a long time now. Since Django 1.1 this is directly supported by Django's ORM.
    – bjunix
    Aug 24, 2014 at 14:14
1

Why not use the database as the concurrency layer ? Add a primary key or unique constraint the table to visitType and visitDate. If I'm not mistaken, django does not exactly support this in their database Model class or at least I've not seen an example.

Once you've added the constraint/key to the table, then all you have to do is:

  1. check if the row is there. if it is, fetch it.
  2. insert the row. if there's no error you're fine and can move on.
  3. if there's an error (i.e. race condition), re-fetch the row. if there's no row, then it's a genuine error. Otherwise, you're fine.

It's nasty to do it this way, but it seems quick enough and would cover most situations.

1
  • It doesn't handle the case where two people go to update the counter at the same time. Nov 11, 2008 at 6:28
0

Your should use database transactions to avoid this kind of race condition. A transaction lets you perform the whole operation of creating, reading, incrementing and saving the counter on an "all or nothing" base. If anything goes wrong it will roll back the whole thing and you can try again.

Check out the Django docs. There is a transaction middle ware, or you can use decorators around views or methods to create transactions.

2
  • I agree that transactions seem like the answer here, but it's not clear that the functionality will actually solve the increment problem -- the SELECT to get the row will still succeed, and the UPDATE to change the value of the counter will still succeed. If I'm wrong, an example would be awesome. Nov 11, 2008 at 13:39
  • You would need to lock the table during the select to do it this way, and as mentioned by Sam that would drag down your performance. This is the best way if you don't incriment the counter often.
    – Tom Leys
    Nov 12, 2008 at 3:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.