Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions lms/djangoapps/grades/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,18 @@ def _update_cache(cls, user_id, course_key, visible_blocks):
{visible_block.hashed: visible_block for visible_block in visible_blocks}
)

@classmethod
def clear_prefetched_data(cls):
"""
Clears all prefetched visible blocks from the RequestCache.

Unlike the course-keyed grade caches, this one is keyed per
(user, course) and is only ever added to, so it grows without bound
across a task that reads grades for many learners in turn. Callers
iterating over a large population should drop it periodically.
"""
get_cache(cls._CACHE_NAMESPACE).clear()

@classmethod
def _cache_key(cls, user_id, course_key):
return f"visible_blocks_cache.{course_key}.{user_id}"
Expand Down Expand Up @@ -862,3 +874,14 @@ def _prepare_override_params(subsection_grade_model, override_data):
@classmethod
def clear_prefetched_overrides_for_learner(cls, user_id, course_key):
get_cache(cls._CACHE_NAMESPACE).pop((user_id, str(course_key)), None)

@classmethod
def clear_prefetched_data(cls):
"""
Clears all prefetched overrides from the RequestCache.

Like the VisibleBlocks cache, this is keyed per (user, course) and only
ever grows, so a task reading grades for many learners needs to drop it
periodically rather than relying on the per-learner variant above.
"""
get_cache(cls._CACHE_NAMESPACE).clear()
14 changes: 14 additions & 0 deletions lms/djangoapps/grades/models_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,20 @@ def prefetch_grade_overrides_and_visible_blocks(user, course_key):
_VisibleBlocks.bulk_read(user.id, course_key)


def clear_prefetched_grade_overrides_and_visible_blocks():
"""
Clears the caches populated by prefetch_grade_overrides_and_visible_blocks.

Both are keyed per (user, course) rather than per course, so -- unlike the
course-keyed prefetches below, which are replaced wholesale on each call --
they accumulate an entry per learner and are never evicted for the life of
the request or task. Long-running work that walks a large learner
population should call this between batches.
"""
_PersistentSubsectionGradeOverride.clear_prefetched_data()
_VisibleBlocks.clear_prefetched_data()


def prefetch_course_grades(course_key, users):
_PersistentCourseGrade.prefetch(course_key, users)

Expand Down
53 changes: 46 additions & 7 deletions lms/djangoapps/instructor_task/tasks_helper/grades.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,11 @@
from lms.djangoapps.course_blocks.api import get_course_block_access_transformers, get_course_blocks
from lms.djangoapps.course_blocks.transformers import library_content
from lms.djangoapps.courseware.user_state_client import DjangoXBlockUserStateClient
from lms.djangoapps.grades.api import CourseGradeFactory, prefetch_course_and_subsection_grades
from lms.djangoapps.grades.api import (
CourseGradeFactory,
clear_prefetched_grade_overrides_and_visible_blocks,
prefetch_course_and_subsection_grades,
)
from lms.djangoapps.grades.api import context as grades_context
from lms.djangoapps.instructor_analytics.basic import list_problem_responses
from lms.djangoapps.instructor_analytics.csvs import format_dictlist
Expand Down Expand Up @@ -206,7 +210,12 @@ def graded_scorable_blocks_header(self):
headers in the final report.
"""
scorable_blocks_map = OrderedDict()
grading_context = grades_context.grading_context_for_course(self.course)
# Pass the already-loaded structure rather than using
# grading_context_for_course, which calls get_course_in_cache again.
# BlockStructureManager.get_collected() deserializes fresh from the cache
# backend on every call, so the convenience helper would make a large
# course pay that cost -- and its peak allocation -- a second time.
grading_context = grades_context.grading_context(self.course, self.course_structure)
for assignment_type_name, subsection_infos in grading_context['all_graded_subsections_by_type'].items():
for subsection_index, subsection_info in enumerate(subsection_infos, start=1):
for scorable_block in subsection_info['scored_descendants']:
Expand Down Expand Up @@ -266,6 +275,23 @@ def __init__(self, context, users):
self.verified_users = set(IDVerificationService.get_verified_user_ids(users))


class _ProblemGradeBulkContext:
"""
Bulk-loads the per-learner data the problem grade report reads, so that a
batch costs a fixed number of queries instead of scaling with batch size.

Deliberately narrower than _CourseGradeBulkContext: the problem report emits
no cohort, team, certificate or course-tag columns, so prefetching those
would be wasted work. It does read persisted course and subsection grades
(via course_grade.problem_scores, which walks every graded subsection) and
each learner's enrollment status.
"""

def __init__(self, context, users):
prefetch_course_and_subsection_grades(context.course_id, users)
CourseEnrollment.bulk_fetch_enrollment_states(users, context.course_id)


class _CourseGradeBulkContext: # pylint: disable=missing-class-docstring
def __init__(self, context, users):
self.certs = _CertificateBulkContext(context, users)
Expand Down Expand Up @@ -418,6 +444,8 @@ class GradeReportBase:
"""
Base class for grade reports (ProblemGradeReport and CourseGradeReport).
"""
# Batch size for chunking the list of enrollees in the course.
USER_BATCH_SIZE = 100

def __init__(self, context):
self.context = context
Expand Down Expand Up @@ -451,7 +479,7 @@ def _batch_users(self):
"""
Returns a generator of batches of users.
"""
def grouper(iterable, chunk_size=100, fillvalue=None):
def grouper(iterable, chunk_size=self.USER_BATCH_SIZE, fillvalue=None):
args = [iter(iterable)] * chunk_size
return zip_longest(*args, fillvalue=fillvalue)

Expand Down Expand Up @@ -496,9 +524,19 @@ def log_additional_info_for_testing(self, message):

def _clear_caches(self):
"""
Override if a report type wants to clear caches after a batch of learners has
been processed
Clear per-learner caches after a batch of learners has been processed.

RequestCache is only flushed when the task ends, so anything keyed per
learner accumulates for the whole run. The grade prefetches are keyed
per course and replaced on each batch, but the visible-blocks and
subsection-override caches are keyed per (user, course) and are only
ever added to -- and nothing reads an entry again once that learner's
row has been written.

Subclasses that need to drop additional caches should override this and
call super().
"""
clear_prefetched_grade_overrides_and_visible_blocks()

def _batched_rows(self):
"""
Expand All @@ -513,8 +551,6 @@ class CourseGradeReport(GradeReportBase):
"""
Class to encapsulate functionality related to generating user/row had header data for Corse Grade Reports.
"""
# Batch size for chunking the list of enrollees in the course.
USER_BATCH_SIZE = 100

@classmethod
def generate(cls, _xblock_instance_args, _entry_id, course_id, _task_input, action_name):
Expand Down Expand Up @@ -753,6 +789,8 @@ def _rows_for_users(self, users):
"""
Returns a list of rows for the given users for this report.
"""
_ProblemGradeBulkContext(self.context, users)

success_rows, error_rows = [], []
for student, course_grade, error in CourseGradeFactory().iter(
users,
Expand Down Expand Up @@ -793,6 +831,7 @@ def _rows_for_users(self, users):
return success_rows, error_rows

def _clear_caches(self):
super()._clear_caches()
get_cache('get_enrollment').clear()
get_cache(CourseEnrollment.MODE_CACHE_NAMESPACE).clear()

Expand Down
23 changes: 23 additions & 0 deletions lms/djangoapps/instructor_task/tests/test_tasks_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -897,6 +897,29 @@ def setUp(self):
self.student_2 = self.create_student('üser_2')
self.csv_header_row = ['Student ID', 'Email', 'Username', 'Enrollment Status', 'Grade']

def test_query_counts_do_not_scale_with_learner_count(self):
"""
The report's query count should be independent of how many learners are
enrolled, because _ProblemGradeBulkContext prefetches once per batch
rather than reading per learner.

Asserting the same count at two cohort sizes is the point of the test.
Before the bulk prefetch the count fit 2N + 10 -- 20 queries at 5
learners, 110 at 50 -- so a single-size assertion would pass while the
per-learner read crept back in. Measured after: a flat 13 at 5, 10, 25
and 50 learners.
"""
for extra_learners in (3, 20):
for i in range(extra_learners):
self.create_student(f'query_count_üser_{extra_learners}_{i}')

bs_api.update_course_in_cache(self.course.id)
RequestCache.clear_all_namespaces()

with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'):
with self.assertNumQueries(13, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST):
ProblemGradeReport.generate(None, None, self.course.id, {}, 'graded')

@patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task')
@ddt.data(True, False)
def test_no_problems(self, use_tempfile, _): # noqa: PT019
Expand Down
Loading