From 5ea71beeb18fcf08fed00c4ad91b780d2e6017b1 Mon Sep 17 00:00:00 2001
From: David Ormsbee <dormsbee@edx.org>
Date: Tue, 31 Mar 2020 17:27:59 -0400
Subject: [PATCH] Make XBlock views/handlers non-atomic requests.

We're seeing slow commits on production for courseware_studentmodule
updates. Based on the slow queries during those times, we think it
might be because multiple worker processes are trying to update the
same rows from within long-running transactions (since courseware
is relatively slow).

The risk with this is that since the whole view execution is no
longer wrapped in a big implicit transaction, it's possible that
XBlock state will update and things that key off of that (e.g.
completion progress information or pre-req milestones) will fail
in a way that will leave the database in an unplanned-for state,
though this is already the case for those actions that trigger
asynchronous tasks like grades recalculation.

The query counts for the index view test were adjusted down
because save points count towards the total and we're no longer
setting them at the top level around the view as a whole.
---
 lms/djangoapps/courseware/module_render.py    | 3 +++
 lms/djangoapps/courseware/tests/test_views.py | 4 ++--
 lms/djangoapps/courseware/views/index.py      | 2 ++
 lms/djangoapps/courseware/views/views.py      | 1 +
 4 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py
index 8b16b109f6..154d267ad1 100644
--- a/lms/djangoapps/courseware/module_render.py
+++ b/lms/djangoapps/courseware/module_render.py
@@ -16,6 +16,7 @@ from completion.models import BlockCompletion
 from django.conf import settings
 from django.contrib.auth.models import User
 from django.core.cache import cache
+from django.db import transaction
 from django.http import Http404, HttpResponse, HttpResponseForbidden
 from django.middleware.csrf import CsrfViewMiddleware
 from django.template.context_processors import csrf
@@ -994,6 +995,7 @@ def xqueue_callback(request, course_id, userid, mod_id, dispatch):
 
 @csrf_exempt
 @xframe_options_exempt
+@transaction.non_atomic_requests
 def handle_xblock_callback_noauth(request, course_id, usage_id, handler, suffix=None):
     """
     Entry point for unauthenticated XBlock handlers.
@@ -1008,6 +1010,7 @@ def handle_xblock_callback_noauth(request, course_id, usage_id, handler, suffix=
 
 @csrf_exempt
 @xframe_options_exempt
+@transaction.non_atomic_requests
 def handle_xblock_callback(request, course_id, usage_id, handler, suffix=None):
     """
     Generic view for extensions. This is where AJAX calls go.
diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py
index abb698d1ea..687dbfe23c 100644
--- a/lms/djangoapps/courseware/tests/test_views.py
+++ b/lms/djangoapps/courseware/tests/test_views.py
@@ -273,8 +273,8 @@ class IndexQueryTestCase(ModuleStoreTestCase):
     NUM_PROBLEMS = 20
 
     @ddt.data(
-        (ModuleStoreEnum.Type.mongo, 10, 172),
-        (ModuleStoreEnum.Type.split, 4, 170),
+        (ModuleStoreEnum.Type.mongo, 10, 170),
+        (ModuleStoreEnum.Type.split, 4, 168),
     )
     @ddt.unpack
     def test_index_query_counts(self, store_type, expected_mongo_query_count, expected_mysql_query_count):
diff --git a/lms/djangoapps/courseware/views/index.py b/lms/djangoapps/courseware/views/index.py
index 8197c1c4dd..75c01de4d9 100644
--- a/lms/djangoapps/courseware/views/index.py
+++ b/lms/djangoapps/courseware/views/index.py
@@ -16,6 +16,7 @@ import six.moves.urllib.request  # pylint: disable=import-error
 from django.conf import settings
 from django.contrib.auth.models import User
 from django.contrib.auth.views import redirect_to_login
+from django.db import transaction
 from django.http import Http404
 from django.template.context_processors import csrf
 from django.urls import reverse
@@ -95,6 +96,7 @@ TEMPLATE_IMPORTS = {'urllib': urllib}
 CONTENT_DEPTH = 2
 
 
+@method_decorator(transaction.non_atomic_requests, name='dispatch')
 class CoursewareIndex(View):
     """
     View class for the Courseware page.
diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py
index 9a7d67de64..d2ea3d7959 100644
--- a/lms/djangoapps/courseware/views/views.py
+++ b/lms/djangoapps/courseware/views/views.py
@@ -1613,6 +1613,7 @@ def _track_successful_certificate_generation(user_id, course_id):
 @require_http_methods(["GET", "POST"])
 @ensure_valid_usage_key
 @xframe_options_exempt
+@transaction.non_atomic_requests
 def render_xblock(request, usage_key_string, check_if_enrolled=True):
     """
     Returns an HttpResponse with HTML content for the xBlock with the given usage_key.