Unlock the session asap before running any sql

[brendanheywood]

This report churns for a while on big moodles and locks up the session

[danmarsden]

is it the report or the initial get directory size check here? https://github.com/catalyst/moodle-report_coursesize/blob/master/index.php#L57

we run that on schedule and insert it as a separate task on our infrastructure rather than relying on the code from doing it. We should probably have a way to disable it from running - I've also thought about caching this report a bit better in the past too..

[brendanheywood]

I just profiled it, it's 90% sql time. But I don't think it matters either way we should unlock it as soon as possible in index.php and course.php

[brendanheywood]

... which means the set_config should never be called here. Conceptually this seems more like a cached thing and not a real setting so why not just swap it from set_config to a muc set instead?

[danmarsden]

cool - feel free to add some code to tidy this up - we do need to keep the var in config db on our end at this stage because we use other processes (outside moodle) to set it at the db level. happy for there to be a config setting (no ui needed) to control whether to pull from config or muc instead though - and it could default to pulling from muc.

[brendanheywood]

A side thought, get_directory_size in core is ignorant of the new file api stuff, ie objectfs so this is radically wrong too. Not sure how to fix that properly, it probably needs a new method and get_directory_size shouldn't be used?

[danmarsden]

yeah - although running on objectfs is still the exception rather than the norm - when we move to using objectfs we're likely to update our internal processes to retrieve the total disk used and then set it in the config var using other apis. Maybe the objectfs plugins could be checking the total disk used by a client using some form of API call and then storing it in a config var that the coursesize report could look at?

[brendanheywood]

They shouldn't be tightly coupled or have any dependencies. I think the file_store api needs a new method for reporting on disc usage, and the get_directory_size should magically detect if it is requesting something inside filedir vs dataroot and call the file_store api as needed. I'll log that somewhere

[danmarsden]

this should be sorted now that the main query can now be run via a scheduled task.