Commit a14667c6 by Kyle McCormick Committed by Nimisha Asthagiri

MA-1061 Fix IntegrityError caused by race condition in CourseOverview.get_from_id

parent 24bca27b
...@@ -5,6 +5,7 @@ Declaration of CourseOverview model ...@@ -5,6 +5,7 @@ Declaration of CourseOverview model
import json import json
from django.db.models.fields import BooleanField, DateTimeField, DecimalField, TextField, FloatField, IntegerField from django.db.models.fields import BooleanField, DateTimeField, DecimalField, TextField, FloatField, IntegerField
from django.db.utils import IntegrityError
from django.utils.translation import ugettext from django.utils.translation import ugettext
from model_utils.models import TimeStampedModel from model_utils.models import TimeStampedModel
...@@ -172,7 +173,18 @@ class CourseOverview(TimeStampedModel): ...@@ -172,7 +173,18 @@ class CourseOverview(TimeStampedModel):
course = store.get_course(course_id) course = store.get_course(course_id)
if isinstance(course, CourseDescriptor): if isinstance(course, CourseDescriptor):
course_overview = cls._create_from_course(course) course_overview = cls._create_from_course(course)
course_overview.save() try:
course_overview.save()
except IntegrityError:
# There is a rare race condition that will occur if
# CourseOverview.get_from_id is called while a
# another identical overview is already in the process
# of being created.
# One of the overviews will be saved normally, while the
# other one will cause an IntegrityError because it tries
# to save a duplicate.
# (see: https://openedx.atlassian.net/browse/TNL-2854).
pass
return course_overview return course_overview
elif course is not None: elif course is not None:
raise IOError( raise IOError(
......
...@@ -4,9 +4,9 @@ Tests for course_overviews app. ...@@ -4,9 +4,9 @@ Tests for course_overviews app.
import datetime import datetime
import ddt import ddt
import itertools import itertools
import pytz
import math import math
import mock import mock
import pytz
from django.utils import timezone from django.utils import timezone
...@@ -349,3 +349,35 @@ class CourseOverviewTestCase(ModuleStoreTestCase): ...@@ -349,3 +349,35 @@ class CourseOverviewTestCase(ModuleStoreTestCase):
# a call to get_course. # a call to get_course.
with check_mongo_calls_range(max_finds=max_mongo_calls, min_finds=min_mongo_calls): with check_mongo_calls_range(max_finds=max_mongo_calls, min_finds=min_mongo_calls):
_course_overview_2 = CourseOverview.get_from_id(course.id) _course_overview_2 = CourseOverview.get_from_id(course.id)
def test_course_overview_saving_race_condition(self):
"""
Tests that the following scenario will not cause an unhandled exception:
- Multiple concurrent requests are made for the same non-existent CourseOverview.
- A race condition in the django ORM's save method that checks for the presence
of the primary key performs an Insert instead of an Update operation.
- An IntegrityError is raised when attempting to create duplicate entries.
- This should be handled gracefully in CourseOverview.get_from_id.
Created in response to https://openedx.atlassian.net/browse/MA-1061.
"""
course = CourseFactory.create()
# mock the CourseOverview ORM to raise a DoesNotExist exception to force re-creation of the object
with mock.patch(
'openedx.core.djangoapps.content.course_overviews.models.CourseOverview.objects.get'
) as mock_getter:
mock_getter.side_effect = CourseOverview.DoesNotExist
# mock the CourseOverview ORM to not find the primary-key to force an Insert of the object
with mock.patch(
'openedx.core.djangoapps.content.course_overviews.models.CourseOverview._get_pk_val'
) as mock_get_pk_val:
mock_get_pk_val.return_value = None
# verify the CourseOverview is loaded successfully both times,
# including after an IntegrityError exception the 2nd time
for _ in range(2):
self.assertIsInstance(CourseOverview.get_from_id(course.id), CourseOverview)
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment