Opened 8 years ago

Last modified 8 years ago

#27762 new Cleanup/optimization

Concurrency Safety Documentation

Reported by: Kevin Mahoney Owned by: nobody
Component: Documentation Version: 1.10
Severity: Normal Keywords:
Cc: Triage Stage: Someday/Maybe
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I'm the author of a recent blog post exploring some common concurrency bugs in Django (http://kevinmahoney.co.uk/articles/django-orm-mistakes/). They are so common, in fact, that your example Polls app contains two of them (https://docs.djangoproject.com/en/1.10/intro/tutorial04/).

Perhaps it would be a good idea to have a section on this in the documentation?

I might attempt this myself if I find the time.

Change History (3)

comment:1 by Aymeric Augustin, 8 years ago

The tutorial is kept naive on purpose. It contains a note about the non-atomic increment (search for "the code for our vote() view does have a small problem" in the page). Where is the second bug?

There are two categories of bugs in the blog post:

  • Bugs related to ORM instances not being singletons: most developers seem to understand these and they don't appear to be too common in real-world code;
  • Bugs related to concurrency or incorrect assumptions about transactions isolation: these are hard and I'm not sure how well we can cover them in Django's documentation, but there's certainly some room for improvement. If I remember correctly, the documentation of transactions describes the toolbox but not the problem it solves.

Finally, the bigger concern in this area (usually) is that the admin isn't safe against concurrent modifications: if user A loads a change page, makes changes, then saves, any changes made by user B to the same object between the time user A loaded the change page and saved the object are overwritten. There's a ticket for that.

comment:2 by Kevin Mahoney, 8 years ago

Yes, I'm referring to the atomic increment and the concurrent modification (bug 2 & 3) in the post. You're right that I didn't spot the note which covers the atomic increment nicely and I'm glad there is a ticket for the other issue.

In my personal opinion you should not show incorrect code to a learner, and shielding them from this complexity may be doing more harm than good. This opinion is reinforced for me when I see how common these bugs are in commercial and open source code. But this can be a discussion for another time...

The main purpose for this ticket is to see if there is an appetite for concurrency documentation covering concurrent modification and maybe a note about how this is affected by isolation levels. I had in mind a section in the ORM guide called something like 'concurrency safety'. What do you think?

comment:3 by Tim Graham, 8 years ago

Triage Stage: UnreviewedSomeday/Maybe
Type: UncategorizedCleanup/optimization

There is some discussion of isolation on the databases reference page. I'm not sure how much more detail is needed. I think if you're at the point of changing the isolation level, you probably know why you need to change it and what the consequences are.

Django 1.11 added an easy option for changing the isolation level on MySQL. Django 2.0 is likely to switch the default level to READ COMMITTED (#27683).

By the way, you'll reach a wider audience if you write to the DevelopersMailingList about your ideas for documentation enhancements.

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