YONK-1302: Reduce memory footprint of cache entries. (#52)

completion_aggregator/__init__.py

@@ -4,6 +4,6 @@
 
 from __future__ import absolute_import, unicode_literals
 
-__version__ = '1.5.19'
+__version__ = '1.5.20'
 
 default_app_config = 'completion_aggregator.apps.CompletionAggregatorAppConfig'  # pylint: disable=invalid-name

completion_aggregator/cachegroup.py

@@ -53,6 +53,18 @@ def set(self, group, key, value, timeout):
         cache_entry = _CacheGroupEntry(group, value, cached_at)
         return cache.set(key, cache_entry, timeout)
 
+    def touch(self, key, timeout):
+        """
+        Update the timeout for a given key in the cache.
+
+        Returns True if the cache key was updated.
+
+        This functionality is available in Django 2.1 and above.
+        """
+        if hasattr(cache, 'touch'):
+            return cache.touch(key, timeout=timeout)
+        return False
+
     def delete(self, key):
         """
         Invalidate the entry in the cache.

completion_aggregator/compat.py

@@ -13,7 +13,8 @@
 `StubCompat` is a class which implements all the below methods in a way that
 eliminates external dependencies
 """
-from __future__ import absolute_import, unicode_literals
+
+from __future__ import absolute_import, division, print_function, unicode_literals
 
 from django.conf import settings
 
@@ -54,7 +55,7 @@ def get_item_not_found_error():
     return ItemNotFoundError
 
 
-def init_course_blocks(user, course_block_key):
+def init_course_blocks(user, root_block_key):
     """
     Return a BlockStructure representing the course.
 
@@ -71,7 +72,7 @@ def init_course_blocks(user, course_block_key):
         get_course_block_access_transformers() + [AggregatorAnnotationTransformer()]
     )
 
-    return get_course_blocks(user, course_block_key, transformers)
+    return get_course_blocks(user, root_block_key, transformers)
 
 
 def get_block_completions(user, course_key):
@@ -112,24 +113,20 @@ def course_enrollment_model():
 
 def get_users_enrolled_in(course_key):
     """
-    Return list of users enrolled in supplied course_key.
+    Return a list of users enrolled in supplied course_key.
     """
     return course_enrollment_model().objects.users_enrolled_in(course_key)
 
 
-def get_affected_aggregators(course_blocks, changed_blocks):
+def get_block_aggregators(course_blocks, block):
     """
-    Return the set of aggregator blocks that may need updating.
+    Return a list of aggregator blocks that contain the specified block.
     """
-    affected_aggregators = set()
-    for block in changed_blocks:
-        block_aggregators = course_blocks.get_transformer_block_field(
-            block,
-            AggregatorAnnotationTransformer,
-            AggregatorAnnotationTransformer.AGGREGATORS
-        )
-        affected_aggregators.update(block_aggregators or [])
-    return affected_aggregators
+    return course_blocks.get_transformer_block_field(
+        block,
+        AggregatorAnnotationTransformer,
+        AggregatorAnnotationTransformer.AGGREGATORS
+    ) or []
 
 
 def get_mobile_only_courses(enrollments):

completion_aggregator/core.py

@@ -5,6 +5,8 @@
 converts them to Aggregators.
 """
 
+from __future__ import absolute_import, division, print_function, unicode_literals
+
 import logging
 from collections import namedtuple
 from datetime import datetime
@@ -64,6 +66,12 @@ def set(self, value):
         group = six.text_type(self.course_key)
         CacheGroup().set(group, self.cache_key, value, timeout=UPDATER_CACHE_TIMEOUT)
 
+    def touch(self):
+        """
+        Update the timeout for a given cache key.
+        """
+        CacheGroup().touch(self.cache_key, timeout=UPDATER_CACHE_TIMEOUT)
+
     @property
     def cache_key(self):
         """
@@ -76,6 +84,9 @@ def cache_key(self):
         )
 
 
+CourseBlocksEntry = namedtuple('CourseBlocksEntry', ['children', 'aggregators'])
+
+
 class AggregationUpdater(object):
     """
     Class to update aggregators for a given course and user.
@@ -94,16 +105,20 @@ def __init__(self, user, course_key, modulestore, root_block=None):
 
         cache_entry = self.cache.get()
         if cache_entry:
+            self.using_cache = True
             self.course_blocks = cache_entry.course_blocks
             self.root_block = cache_entry.root_block
         else:
+            self.using_cache = False
             with modulestore.bulk_operations(self.course_key):
                 if self.raw_root_block:
                     self.root_block = self.raw_root_block
                 else:
                     self.root_block = compat.init_course_block_key(modulestore, self.course_key)
-                self.course_blocks = compat.init_course_blocks(self.user, self.root_block)
-
+                self.course_blocks = self.format_course_blocks(
+                    compat.init_course_blocks(self.user, self.root_block),
+                    self.root_block
+                )
         self.aggregators = {
             aggregator.block_key: aggregator for aggregator in Aggregator.objects.filter(
                 user=self.user,
@@ -117,6 +132,32 @@ def __init__(self, user, course_key, modulestore, root_block=None):
             for completion in compat.get_block_completions(self.user, self.course_key)
         }
 
+    def format_course_blocks(self, course_blocks, root_block):
+        """
+        Simplify the BlockStructure to have the following format.
+
+            {
+                block_key: CourseBlocksEntry(
+                    children=block_key[],
+                    aggregators=block_key[]
+                )
+            }
+
+        """
+        structure = {}
+
+        def populate(structure, block):
+            if block not in structure:
+                structure[block] = CourseBlocksEntry(
+                    children=compat.get_children(course_blocks, block),
+                    aggregators=compat.get_block_aggregators(course_blocks, block),
+                )
+                for child in structure[block].children:
+                    populate(structure, child)
+
+        populate(structure, root_block)
+        return structure
+
     def set_cache(self):
         """
         Cache updater values to prevent calling course_blocks api.
@@ -130,6 +171,8 @@ def set_cache(self):
         high, as could the number of course structures cached, especially if
         staff had been making course updates.
         """
+        if self.using_cache:
+            self.cache.touch()
         entry = CacheEntry(course_blocks=self.course_blocks, root_block=self.root_block)
         self.cache.set(entry)
 
@@ -138,7 +181,10 @@ def get_affected_aggregators(self, changed_blocks):
         Return the set of aggregator blocks that may need updating.
         """
         if changed_blocks:
-            return compat.get_affected_aggregators(self.course_blocks, changed_blocks)
+            affected_aggregators = set()
+            for block in changed_blocks:
+                affected_aggregators.update(self.course_blocks[block].aggregators)
+            return affected_aggregators
         else:
             return BagOfHolding()
 
@@ -195,13 +241,10 @@ def update_for_aggregator(self, block, affected_aggregators, force):
         last_modified = OLD_DATETIME
 
         if block not in affected_aggregators:
-            try:
-                obj = Aggregator.objects.get(user=self.user, course_key=self.course_key, block_key=block)
-            except Aggregator.DoesNotExist:
-                pass
-            else:
+            obj = self.aggregators.get(block)
+            if obj:
                 return CompletionStats(earned=obj.earned, possible=obj.possible, last_modified=obj.last_modified)
-        for child in compat.get_children(self.course_blocks, block):
+        for child in self.course_blocks[block].children:
             (earned, possible, modified) = self.update_for_block(child, affected_aggregators, force)
             total_earned += earned
             total_possible += possible

test_utils/compat.py

@@ -30,27 +30,23 @@ def init_course_block_key(self, modulestore, course_key):  # pylint: disable=unu
         """
         return course_key.make_usage_key('course', 'course')
 
-    def init_course_blocks(self, user, course_block_key):  # pylint: disable=unused-argument
+    def init_course_blocks(self, user, root_block_key):  # pylint: disable=unused-argument
         """
         Not actually used in this implementation.
 
         Overridden here to prevent the default behavior, which relies on
         modulestore.
         """
-        root_segments = course_block_key.block_id.split('-')
+        root_segments = root_block_key.block_id.split('-')
         return CompatCourseBlocks(
             *(block for block in self.blocks if block.block_id.split('-')[:len(root_segments)] == root_segments)
         )
 
-    def get_affected_aggregators(self, course_blocks, changed_blocks):
+    def get_block_aggregators(self, course_blocks, block):
         """
-        Get all the aggregator blocks affected by a change to one of the given blocks.
+        Returns a list of aggregator blocks that contain the specified block.
         """
-        affected = set()
-        for block in course_blocks.blocks:
-            if any(changed.block_id.startswith('{}-'.format(block.block_id)) for changed in changed_blocks):
-                affected.add(block)
-        return affected
+        return [agg for agg in course_blocks.blocks if block.block_id.startswith('{}-'.format(agg))]
 
     def get_block_completions(self, user, course_key):
         """

tests/test_batch.py

@@ -5,6 +5,8 @@
 # Redefined outer names are explicitly used by pytest fixtures.
 # pylint: disable=redefined-outer-name
 
+from __future__ import absolute_import, division, print_function, unicode_literals
+
 import pytest
 import six
 from mock import patch

tests/test_core.py

@@ -423,14 +423,13 @@ def test_multiple_partial_updates(self):
                 - auth_user (fetch user details)
                 - completion_aggregator_aggregator (user specific for specific course)
                 - completion_blockcompletion (user specific)
-                - completion_aggregator_aggregator (user specific for specific course and block)
             * Insert or Update Query
                 - completion_aggregator_aggregator (insert aggregation data)
             * Update query
                 - completion_aggregator_stalecompletion (user specific)
         '''   # pylint: disable=pointless-string-statement
 
-        with self.assertNumQueries(6):
+        with self.assertNumQueries(5):
             aggregation_tasks.update_aggregators(self.user.username, six.text_type(self.course_key), {
                 six.text_type(completion.block_key)})
 
@@ -449,7 +448,7 @@ def test_multiple_partial_updates(self):
             ),
         ]
 
-        with self.assertNumQueries(6):
+        with self.assertNumQueries(5):
             aggregation_tasks.update_aggregators(
                 username=self.user.username,
                 course_key=six.text_type(self.course_key),