search

jazkarta / edx-platform

the edX learning management system (LMS) and course authoring tool, Studio
http://code.edx.org/
GNU Affero General Public License v3.0
1 stars 0 forks source link

Race condition while creating CCX can cause perma 500 #81

Closed carsongee closed 9 years ago

carsongee commented 9 years ago

Currently the creation of a CCX is not in a transaction, so race conditions are possible. This happened to me in the "wild" on edx.org, and I'm now no longer able to do anything with that course as I'm getting MultipleObjectsReturned: get() returned more than one CustomCourseForEdX -- it returned 2!. I believe the correction needs to be added around https://github.com/edx/edx-platform/blob/master/lms/djangoapps/ccx/views.py#L124-L129. I have the full stack trace if needed.

cewing commented 9 years ago

@carsongee I'd love to see the full stack trace, but I am also wondering if the work I am starting to do this week on fixing issue #43 and the other issues related to it might make this problem obsolete. How urgent is a fix for this under our current strategy for creating a CCX?

carsongee commented 9 years ago

I sent you the trace via e-mail. I'm not all sure what is involved in changing to a new course key, but if you are no longer using SQL to store CCX names, this would become irrelevant in mysql.

cewing commented 9 years ago

Thanks, @carsongee :+1:!

cewing commented 9 years ago

We will fix this by ensuring that only one ccx is returned.

Following discussion with @pdpinch we will mark this fix so that we can eventually allow more than one CCX per coach per course.

cewing commented 9 years ago

This is also now fixed. In the few remaining places where we look up ccx by course and coach rather than by primary key we now get all ccxs that match course and coach and return the first one.

I've left a marker in the lms.djangoapps.ccx.views module to mark the location where this could be fixed in the future.