Handles Azure blob storage failure for large files #706
Conversation
sumaiyamannan
commented
Oct 19, 2025
sumaiyamannan
commented
- The change updates the Azure Blob storage file system to use standard readfile() simplifying the code and aligning to latest PHP capabilities
- get_remote_path_from_storedfile() function calls is_file_readable_locally_by_storedfile() hence calling it again is not needed
- This allows site admin to control whether it should check for local or external first
- The readfile() function in PHP is memory-efficient for large files since 2016 and readfile_allow_large might not be needed
- The change updates the Azure Blob storage file system to use standard readfile() simplifying the code and aligning to latest PHP capabilities - get_remote_path_from_storedfile() function calls is_file_readable_locally_by_storedfile() hence calling it again is not needed - This allows site admin to control whether it should check for local or external first - The readfile() function in PHP is memory-efficient for large files since 2016 and readfile_allow_large might not be needed
sumaiyamannan
force-pushed
the
MOODLE_404_STABLE-large_files
branch
from
ef7a303 to
09f4629
Compare
|
I just want to note that the reason for this patch is that we were seeing downloads over 2GB failing at 323608576 bytes consistently. The issue appears to be that Moodle sends files over 2GB by reading 64KB chunks and echoing them out to the client, reducing that to 8KB chunks works, but this is exactly what PHP's readfile also does too. I'm not exactly clear of exactly why reading 64K chunks from azure fails, however debug logging added to guzzle shows that it's only requesting 8KB chunks from azure anyway, so it makes sense to just use readfile(). |
|
I do think it's probably worth someone spending some time at some point to work out if Moodle core should even have the readfile_allow_large function anymore - maybe we should just be passing all large files to the core php readfile() function now. but in the meantime - +1 from me to merge this. |
|
@matthewhilton @brendanheywood would be good to have your eyes/awareness of this too. |
