Pull requests

#1 Open
Repository
alubbock alubbock
Branch
default
Repository
codekoala codekoala
Branch
default

Fixed race condition in middleware by using get_or_create

Bitbucket cannot automatically merge this request due to conflicts.

Review the conflicts on the Overview tab. You can then either decline the request or merge it manually on your local system using the following commands:

hg update 
hg pull -r default https://bitbucket.org/alubbock/django-tracking
Author
  1. alubbock
Reviewers
Description

I received a primary key violation from a race condition in middleware.py when used in production, resulting in a server error. This should fix it by using get_or_create to make the Visitor object database operation atomic. Hope it helps.

Comments (3)

  1. alubbock author

    There's a fair few threads discussing if this is the case e.g. on StackOverflow [1] and GitHub [2]. It's not atomic - sorry for previous claim - but it does simplify the django-tracking code and makes use of the most appropriate Django API for this purpose, in lieu of an UPSERT-type method which would be better.

    In practical terms, get_or_create encloses the operation within a transaction which does a rollback if the object already exists [3], using the unique_together constraint in models.py. This keeps the database in a consistent state and seems to fix the error I was getting ('There was a problem saving visitor information', middleware.py line 154, version 0.4.1), which also generates an internal server error and a 'database error: current transaction is aborted'.

    If you have any ideas on how else to fix this problem I'd be happy to hear them, if you think my fix is inappropriate. Thanks!

    [1] http://stackoverflow.com/questions/6416213/is-get-or-create-thread-safe [2] https://github.com/hmarr/mongoengine/issues/478 [3] https://github.com/django/django/blob/master/django/db/models/query.py#L457

  2. alubbock author

    I've updated the pull request to be a minimum-changed version of the original middleware.py. It should at least help to reduce the occurrence of the error message mentioned above. Thanks.