Fix DB race conditions in actioncounter and contentflagging apps

RESOLVED FIXED in 1.1

Status

P2
normal
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: lorchard, Unassigned)

Tracking

unspecified

Details

(Whiteboard: u=user c=bug p=2)

(Reporter)

Description

7 years ago
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.
(Reporter)

Updated

7 years ago
Whiteboard: c=bug p=2
Target Milestone: --- → 1.0
OS: Mac OS X → All
Priority: -- → P2
Hardware: x86 → All
Whiteboard: c=bug p=2 → u=user c=bug p=2
Target Milestone: 1.0 → 1.1
Should we do this now, or is it okay to bump this to 1.2?
(Reporter)

Comment 2

7 years ago
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.
(Reporter)

Updated

7 years ago
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(Assignee)

Updated

6 years ago
Version: Kuma → unspecified
(Assignee)

Updated

6 years ago
Component: Demos → Demo Studio / Dev Derby
Product: Mozilla Developer Network → Mozilla Developer Network
You need to log in before you can comment on or make changes to this bug.