672875 - Fix DB race conditions in actioncounter and contentflagging apps

Status: RESOLVED FIXED

(developer.mozilla.org Graveyard :: Demo Studio / Dev Derby, defect, P2)

Description

[Les Orchard [:lorchard]]

A site test by :stephend recently revealed that the actioncounters and contentflagging Django apps used on the Demo Studio have race conditions in database handling that can cause persistent errors.

I did a quick fix, which cleans up the error condition but doesn't fix the root cause:
https://github.com/mozilla/kuma/commit/984b7b2276f234f1c85b49bbea9ce5c5db29040a

Notes to myself or whomever takes this bug:

* These two apps rely on a set of DB columns to describe a unique hit - eg. ip, user_agent, user_id.

* But, these DB columns do not have a UNIQUE index in the MySQL table. So, although the code assumes unique records, MySQL will allow multiple duplicates.

* So, when the code uses Django's get_or_create utility, there's a very brief window of time where Django looks for an existing record, finds none, and creates a new one.

* That window between looking for an existing record and creating a new one is where the race condition exists. That allows multiple records for the same unique values to be created, and causes application errors.

* Proposed solution:
** Update the tests that force this condition to happen.
** Migrate both apps to South.
** Remote the session_key field, since it's unused and has caused issues in the past.
** Add a unique_together attrib in Meta for the relevant fields in both models.
** Create a South migration that creates a UNIQUE index for the group of columns.

Comment 1

[Luke Crouch [:groovecoder]]

Should we do this now, or is it okay to bump this to 1.2?

Comment 2

[Les Orchard [:lorchard]]

https://github.com/mozilla/kuma/commit/7eba73f58d94a109775631494d6fa00aafaac72d

    Bug 672875 - Fix DB race conditions in actioncounter and contentflagging apps
    
    * MySQL UNIQUE constraint doesn't work the way I thought it did for NULL
      values in columns. So, moved to calculating a unique hash in Python
      and store that as a unique column
    
    * actioncounters and contentflagging converted to use South migrations
    
    * South schema migrations to add unique_hash column and remove unused
      session_key column.
    
    * South data migration to calculate unique_hash for all existing rows.
    
    * Removed previously implemented bandaid fix that would delete multiple
      columns when integrity issue encountered.
    
    * Cron job for actioncounters now deletes counters over two months old
      for anonymous users as a means of garbage collection.