perf: speed up the CCX grade report and fix its CSV output#38916
perf: speed up the CCX grade report and fix its CSV output#38916sergivalero20 wants to merge 1 commit into
Conversation
|
Thanks for the pull request, @sergivalero20! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
Description
This PR improves the synchronous CCX coach grade report (
ccx_grades_csv, CCX Coach dashboard → Student Admin → "Download student grades"). It addresses three independent problems, one per commit:1. Performance — N+1 read of persisted grades (
perf)ccx_grades_csviterated enrolled learners withCourseGradeFactory().iter()without bulk-prefetching persisted course/subsection grades, so each student triggered its own database read. Grade-related query count grew linearly withenrollment, and because the report is synchronous it holds a web worker for the full duration, slow, and prone to timing out on larger classes.
The fix reuses the exact pattern the asynchronous
CourseGradeReportalready relies on (CourseGradeReport._rows_for_users/_CourseGradeBulkContext): callprefetch_course_and_subsection_grades()once for the whole class and wrap the iteration inmodulestore().bulk_operations(). This commit is output-preserving — the CSV is byte-for-byte identical, only faster.2. Bug — bytes repr in the CSV (
fix)student.email.encode('utf-8')/student.username.encode('utf-8')returnbyteson Python 3, andcsv.writerserializes bytes withrepr(), so the downloaded file contained values likeb'learner@example.com'andb'learner'. The.encode()calls were a Python 2 leftover —csv.writerhandles unicode natively. This changes the CSV output (it becomes correct).3. Usability — generic download filename (
feat)Content-Dispositionwas a bareattachmentwith no filename, so every CCX downloaded asccx_grades.csv, making it impossible to tell classes or download dates apart. It now usescourse_filename_prefix_generator()to build<course_prefix>_grade_report_<YYYY-MM-DD-HHMM>.csv, matching the naming convention already used byCourseGradeReportdownloads. This changes the downloaded filename.Supporting information
Fixes #38911
Related existing implementation reused by this PR:
lms/djangoapps/instructor_task/tasks_helper/grades.py—CourseGradeReport._rows_for_users,_CourseGradeBulkContextlms/djangoapps/grades/api—prefetch_course_and_subsection_gradescommon/djangoapps/util/file.py—course_filename_prefix_generatorTesting instructions
Automated
Run the CCX grade report tests:
And the full CCX view test module, to confirm nothing else regressed:
Two tests cover this change:
test_grades_csv— extended to assert the new filename pattern and that email/username are written as plain text (the fixture includes a learner with a unicode username, the case that motivated the original.encode()). The existing grade-breakdown assertions are unchanged, which demonstrates the performance commit does not alter output.test_grades_csv_prefetches_persisted_grades— new; asserts a single bulk prefetch is issued for the CCX covering every enrolled learner (locks in the N+1 fix without brittle exact query counts).Quality gates:
Manual
Setup: enable CCX (
FEATURES['CUSTOM_COURSES_EDX'] = True), create a CCX, and enroll several learners who have attempted graded content.Output correctness
<course_prefix>_grade_report_<timestamp>.csv(previously the genericccx_grades.csv).emailandusernamecolumns contain plain values (previouslyb'learner@example.com'). Include a learner whose username contains non-ASCII characters.Performance
Enable SQL logging in your devstack LMS settings:
(Django Debug Toolbar also works if you have it enabled.)
Download the report and note the number of grade-related queries.
Enroll additional learners in the same CCX and download again. The grade-related query count should stay essentially flat rather than growing with the number of learners. Before this PR it grew linearly.
Edge cases
visible_to_staff_only) remains excluded from the grade columns, as before.