EDUCATOR-778: prefetched visible blocks

lms/djangoapps/grades/models.py

@@ -181,6 +181,7 @@ class VisibleBlocks(models.Model):
     in the blocks_json field. A hash of this json array is used for lookup
     purposes.
     """
+    CACHE_NAMESPACE = u"grades.models.VisibleBlocks"
     blocks_json = models.TextField()
     hashed = models.CharField(max_length=100, unique=True)
     course_id = CourseKeyField(blank=False, max_length=255, db_index=True)
@@ -207,27 +208,37 @@ class VisibleBlocks(models.Model):
     @classmethod
     def bulk_read(cls, course_key):
         """
-        Reads all visible block records for the given course.
+        Reads and returns all visible block records for the given course from
+        the cache.  The cache is initialize with the visible blocks for this
+        course if no entry currently exists.has no entry for this course,
+        the cache is updated.
 
         Arguments:
             course_key: The course identifier for the desired records
         """
-        return cls.objects.filter(course_id=course_key)
+        prefetched = get_cache(cls.CACHE_NAMESPACE).get(cls._cache_key(course_key))
+        if not prefetched:
+            prefetched = cls._initialize_cache(course_key)
+        return prefetched
 
     @classmethod
-    def bulk_create(cls, block_record_lists):
+    def bulk_create(cls, course_key, block_record_lists):
         """
         Bulk creates VisibleBlocks for the given iterator of
-        BlockRecordList objects.
+        BlockRecordList objects and updates the VisibleBlocks cache
+        for the block records' course with the new VisibleBlocks.
+        Returns the newly created visible blocks.
         """
-        return cls.objects.bulk_create([
+        created = cls.objects.bulk_create([
             VisibleBlocks(
                 blocks_json=brl.json_value,
                 hashed=brl.hash_value,
-                course_id=brl.course_key,
+                course_id=course_key,
             )
             for brl in block_record_lists
         ])
+        cls._update_cache(course_key, created)
+        return created
 
     @classmethod
     def bulk_get_or_create(cls, block_record_lists, course_key):
@@ -236,9 +247,42 @@ class VisibleBlocks(models.Model):
         BlockRecordList objects for the given course_key, but
         only for those that aren't already created.
         """
-        existent_records = {record.hashed: record for record in cls.bulk_read(course_key)}
+        existent_records = cls.bulk_read(course_key)
         non_existent_brls = {brl for brl in block_record_lists if brl.hash_value not in existent_records}
-        cls.bulk_create(non_existent_brls)
+        cls.bulk_create(course_key, non_existent_brls)
+
+    @classmethod
+    def _initialize_cache(cls, course_key):
+        """
+        Prefetches visible blocks for the given course and stores in the cache.
+        Returns a dictionary mapping hashes of these block records to the
+        block record objects.
+        """
+        prefetched = {record.hashed: record for record in cls.objects.filter(course_id=course_key)}
+        get_cache(cls.CACHE_NAMESPACE)[cls._cache_key(course_key)] = prefetched
+        return prefetched
+
+    @classmethod
+    def _update_cache(cls, course_key, visible_blocks):
+        """
+        Adds a specific set of visible blocks to the request cache.
+        This assumes that prefetch has already been called.
+        """
+        get_cache(cls.CACHE_NAMESPACE)[cls._cache_key(course_key)].update(
+            {visible_block.hashed: visible_block for visible_block in visible_blocks}
+        )
+
+    @classmethod
+    def clear_cache(cls, course_key):
+        """
+        Clears the cache of all contents for a given course.
+        """
+        cache = get_cache(cls.CACHE_NAMESPACE)
+        cache.pop(cls._cache_key(course_key), None)
+
+    @classmethod
+    def _cache_key(cls, course_key):
+        return u"visible_blocks_cache.{}".format(course_key)
 
 
 class PersistentSubsectionGrade(DeleteGradesMixin, TimeStampedModel):

lms/djangoapps/grades/new/course_grade_factory.py

@@ -1,4 +1,5 @@
 from collections import namedtuple
+from contextlib import contextmanager
 from logging import getLogger
 
 import dogstats_wrapper as dog_stats_api
@@ -6,7 +7,7 @@ from openedx.core.djangoapps.signals.signals import COURSE_GRADE_CHANGED
 
 from ..config import assume_zero_if_absent, should_persist_grades
 from ..config.waffle import WRITE_ONLY_IF_ENGAGED, waffle
-from ..models import PersistentCourseGrade
+from ..models import PersistentCourseGrade, VisibleBlocks
 from .course_data import CourseData
 from .course_grade import CourseGrade, ZeroCourseGrade
 
@@ -75,6 +76,14 @@ class CourseGradeFactory(object):
         course_data = CourseData(user, course, collected_block_structure, course_structure, course_key)
         return self._update(user, course_data, read_only=False)
 
+    @contextmanager
+    def _course_transaction(self, course_key):
+        """
+        Provides a transaction context in which GradeResults are created.
+        """
+        yield
+        VisibleBlocks.clear_cache(course_key)
+
     def iter(
             self,
             users,
@@ -100,28 +109,29 @@ class CourseGradeFactory(object):
         course_data = CourseData(
             user=None, course=course, collected_block_structure=collected_block_structure, course_key=course_key,
         )
-        for user in users:
-            with dog_stats_api.timer(
-                    'lms.grades.CourseGradeFactory.iter',
-                    tags=[u'action:{}'.format(course_data.course_key)]
-            ):
-                try:
-                    method = CourseGradeFactory().update if force_update else CourseGradeFactory().create
-                    course_grade = method(
-                        user, course_data.course, course_data.collected_structure, course_key=course_key,
-                    )
-                    yield self.GradeResult(user, course_grade, None)
+        stats_tags = [u'action:{}'.format(course_data.course_key)]
+        with self._course_transaction(course_data.course_key):
+            for user in users:
+                with dog_stats_api.timer('lms.grades.CourseGradeFactory.iter', tags=stats_tags):
+                    yield self._iter_grade_result(user, course_data, force_update)
 
-                except Exception as exc:  # pylint: disable=broad-except
-                    # Keep marching on even if this student couldn't be graded for
-                    # some reason, but log it for future reference.
-                    log.exception(
-                        'Cannot grade student %s in course %s because of exception: %s',
-                        user.id,
-                        course_data.course_key,
-                        exc.message
-                    )
-                    yield self.GradeResult(user, None, exc)
+    def _iter_grade_result(self, user, course_data, force_update):
+        try:
+            method = CourseGradeFactory().update if force_update else CourseGradeFactory().create
+            course_grade = method(
+                user, course_data.course, course_data.collected_structure, course_key=course_data.course_key,
+            )
+            return self.GradeResult(user, course_grade, None)
+        except Exception as exc:  # pylint: disable=broad-except
+            # Keep marching on even if this student couldn't be graded for
+            # some reason, but log it for future reference.
+            log.exception(
+                'Cannot grade student %s in course %s because of exception: %s',
+                user.id,
+                course_data.course_key,
+                exc.message
+            )
+            return self.GradeResult(user, None, exc)
 
     @staticmethod
     def _create_zero(user, course_data):

lms/djangoapps/grades/tests/test_tasks.py

@@ -406,17 +406,6 @@ class ComputeGradesForCourseTest(HasCourseWithProblemsMixin, ModuleStoreTestCase
             min(batch_size, 8)  # No more than 8 due to offset
         )
 
-    @ddt.data(*xrange(1, 12, 3))
-    def test_database_calls(self, batch_size):
-        per_user_queries = 15 * min(batch_size, 6)  # No more than 6 due to offset
-        with self.assertNumQueries(6 + per_user_queries):
-            with check_mongo_calls(1):
-                compute_grades_for_course_v2.delay(
-                    course_key=six.text_type(self.course.id),
-                    batch_size=batch_size,
-                    offset=6,
-                )
-
     @ddt.data(*xrange(1, 12, 3))
     def test_compute_all_grades_for_course(self, batch_size):
         self.set_up_course()