Merge pull request #9090 from edx/kyle/course-overviews-race-fix
MA-1061 Fix IntegrityError caused by race condition in CourseOverview.get_from_id
This commit is contained in:
@@ -5,6 +5,7 @@ Declaration of CourseOverview model
|
||||
import json
|
||||
|
||||
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 model_utils.models import TimeStampedModel
|
||||
|
||||
@@ -172,7 +173,18 @@ class CourseOverview(TimeStampedModel):
|
||||
course = store.get_course(course_id)
|
||||
if isinstance(course, CourseDescriptor):
|
||||
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
|
||||
elif course is not None:
|
||||
raise IOError(
|
||||
|
||||
@@ -4,9 +4,9 @@ Tests for course_overviews app.
|
||||
import datetime
|
||||
import ddt
|
||||
import itertools
|
||||
import pytz
|
||||
import math
|
||||
import mock
|
||||
import pytz
|
||||
|
||||
from django.utils import timezone
|
||||
|
||||
@@ -349,3 +349,35 @@ class CourseOverviewTestCase(ModuleStoreTestCase):
|
||||
# a call to get_course.
|
||||
with check_mongo_calls_range(max_finds=max_mongo_calls, min_finds=min_mongo_calls):
|
||||
_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)
|
||||
|
||||
Reference in New Issue
Block a user