perf: stream report uploads instead of buffering the whole file in memory

[blarghmatey]

What

Finding 4 of #38943, the piece deliberately left out of #38944 because it touches shared
storage code used by every instructor-task report rather than only the grade reports.

Why

DjangoStorageReportStore.store() read its whole buffer into memory, re-encoded it, and
copied it into a ContentFile before handing it to the storage backend:

buff_contents = buff.read()                       # whole file into RAM
if not isinstance(buff_contents, bytes):
    buff_contents = buff_contents.encode('utf-8') # second copy
buff = ContentFile(buff_contents)                 # third
self.storage.save(path, buff)

Peak memory at the final step was therefore roughly 3x the report size — and it largely
defeated TemporaryFileReportMixin, which exists precisely to keep the report out of
memory. That mixin spills rows to a temp file, and then the upload read the whole thing
straight back in. So enabling instructor_task.use_on_disk_grade_reporting bounded row
generation but left a full-size spike at upload.

The ReportStore docstring has anticipated this for a while:

Should probably refactor later to create a ReportFile object that can simply be appended
to for the sake of memory efficiency, rather than passing in the whole dataset.

Changes

A binary buffer is now passed to the backend as-is, so it streams — S3Boto3Storage
uploads in parts, FileSystemStorage writes in chunks, and peak memory is one chunk
rather than the whole report.

Text buffers still work and still have to be read in full, because the backends require
bytes. Both in-tree callers that pass text (instructor/views/api_v2.py, and the existing
StringIO usage in tests) are unchanged in behaviour. The type is probed with a
zero-length read rather than by inspecting mode attributes, which are not consistently
present across the raw file objects, ContentFile and BytesIO instances that callers
pass.

Two callers move to binary so they take the streaming path:

• TemporaryFileReportMixin opens its temp files 'w+b' and writes through a
  TextIOWrapper. This is what makes the on-disk grade report bounded end to end rather
  than only during row generation.
• store_rows() builds its CSV in a BytesIO by the same means.

Both wrappers use newline='' per the csv module's contract — the writer emits its own
line terminators and must not have them translated again — and are detached rather than
closed, so the underlying file outlives the wrapper.

Testing

• lms/djangoapps/instructor_task/tests/ — 252 passed, up from 243, the nine new
  being three tests across the three report-store configurations.
• Two failures in test_api_helper.py::ScheduledBulkEmailInstructorTaskTests are
  pre-existing: they fail identically on unmodified master at 387ee5d9 (verified
  by stashing this branch's changes and re-running). Unrelated to this change — scheduled
  bulk-email task creation, not report storage.

The new tests assert the streaming behaviourally rather than by inspecting call args. A
streaming backend reads in bounded chunks (File.chunks and upload_fileobj both pass an
explicit size), so an unbounded read() is the signature of the report being slurped into
RAM. They live in ReportStoreTestMixin, so they run against all three configurations
including the S3 stub, plus round-trip assertions for the binary, text and store_rows
paths.

Backward compatibility

CSV bytes are unchanged. store() keeps accepting text buffers, so no caller in or out of
tree needs to change. No schema or data migration.

Notes for reviewers

• instructor/views/api_v2.py still passes a text StringIO for the issued-certificates
  report. It works and is correct; it just does not get the streaming benefit. Left alone
  to keep this diff to the storage layer, and that report is small.
• Stacks conceptually on perf: bulk-prefetch grades in ProblemGradeReport and bound its per-learner caches #38944 but does not depend on it — the two touch different parts
  of grades.py and can land in either order.

[openedx-webhooks]

Thanks for the pull request, @blarghmatey!

This repository is currently maintained by @openedx/wg-maintenance-openedx-platform-oncall.

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 approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

• If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
  • This process (including the steps you'll need to take) is documented here.
• If it doesn't, simply proceed with the next step.

🔘 Provide context

To 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:

• Dependencies

  This PR must be merged before / after / at the same time as ...

• Blockers

  This PR is waiting for OEP-1234 to be accepted.

• Timeline information

  This PR must be merged by XX date because ...

• Partner information

  This is for a course on edx.org.

• Supporting documentation
• Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details

---

Where 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:

• Overview of Review Process for Community Contributions
• Pull Request Status Guide
• Making changes to your pull request

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:

• The size and impact of the changes that it introduces
• The need for product review
• Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

[blarghmatey]

@openedx/wg-maintenance-openedx-platform-oncall — this is ready for engineering review, with one caveat on the build noted below.

Checklist from the bot above:

• Product approval — not applicable. CSV bytes are unchanged and store() keeps accepting text buffers, so no caller in or out of tree needs to change. No schema or data migration.
• Context — in the description, including why the on-disk report path was only half-effective before this, and the linked issue (Performance: ProblemGradeReport reads persisted grades per-learner (no bulk prefetch), plus four resource issues in instructor_task grade reporting #38943).
• Green build — 41 of 42 passing. The one failure needs a maintainer's hand; details below.

The coverage failure is an artifact flake, not this PR

coverage (3.12) fails with:

Couldn't combine from non-existent path 'reports/*'

Evidence that this is infrastructure rather than the diff:

• Every coverage-producing shard in that same run succeeded — shared-with-lms-1, shared-with-lms-2 and shared-with-cms-1 are all green. The combine step simply found no artifacts to combine.
• coverage (3.12) passes on perf: bulk-prefetch grades in ProblemGradeReport and bound its per-learner caches #38944, which runs the identical workflow from the same fork.
• This diff touches only lms/djangoapps/instructor_task/ and cannot affect artifact upload or retention.

I can't re-run it myself — gh run rerun reports Must have admin rights to Repository. Could someone with rerun rights kick job 91265942653? I'd rather not push a no-op commit just to retrigger the workflow.

Short version of the change

DjangoStorageReportStore.store() read its whole buffer into memory, re-encoded it, and copied it into a ContentFile before handing it to the storage backend — roughly 3x the report size in RAM at the final step. That largely defeated TemporaryFileReportMixin, which spills rows to a temp file precisely to keep the report out of memory, only for the upload to read the whole thing straight back in.

Binary buffers now stream to the backend, and the two callers that can supply binary (the temp-file mixin and store_rows) were switched over. Text buffers still work unchanged. The new tests assert the streaming behaviourally rather than by inspecting call args, and run against all three report-store configurations including the S3 stub.

Related: #38944 covers the other four findings from #38943. Independent of this one — different parts of grades.py, either order works.

[ormsbee]

Just as a sanity check:

1. How big do these reports get?
2. Has the memory impact of this fix been verified on a running instance?