remotelearner / moodle-block_grade_me

14 stars 37 forks source link

Issue #57 correction for unnecessary db operations #58

Open outsei2 opened 2 years ago

outsei2 commented 2 years ago

Corrected the task_scheduled classname and quiz table to be build only when resetting the cache.

I have tested this in my local Moodle installation with only couple of courses. In the real environment these numbers are thousands times bigger.

Before the changes:

\block_grade_me\task\cache_grade_data 5212 reads 231 writes 0,81 secs

\block_grade_me\task\reset_block: 5212 reads 234 writes 0,86 secs

After the changes

\block_grade_me\task\cache_grade_data 27 reads 1 writes 0,01 secs

\block_grade_me\task\reset_block: 5212 reads 234 writes 0,86 secs

As seen the effect to cache_grade_data script is huge. Especially when the script is by default run in cron every 15 minutes. When testing the updates to courses, quizzes and assignments, the module is still working as before.

This update has no effect to reset_block script. This script is run only once per night so the problem is not so acute than with updating. It might be still good idea to refactor the code also from this script's perspective in future.

outsei2 commented 2 years ago

This is the correction for the Issue #57 It is also probably the root cause for the old bug #28 and will correct it too.

outsei2 commented 2 years ago

Just a clarification about this correction. Change log is a bit misleading at this point (shows 104 lines changed, when actually there is only 3). The changes made are these (marked with arrow):

function block_grade_me_cache_grade_data() {
    global $CFG, $DB;
=>    $lastrun = $DB->get_field('task_scheduled', 'lastruntime', array('classname' => '\block_grade_me\task\cache_grade_data'));
    $params = array();

and

=>            if ($coursemod == 0) {
                $sqlquizlist = "SELECT mq.id quizid, mqa.id quiattemptid, mqa.userid, mq.course, mqa.uniqueid,
                                qna.id questionattemptid
                                FROM {quiz} mq
                                JOIN {quiz_attempts} mqa ON mqa.quiz = mq.id
                                JOIN {question_attempts} qna ON qna.questionusageid = mqa.uniqueid
                                JOIN {user} mu  ON mu.id = mqa.userid
                                WHERE course = ?
                                AND behaviour = 'manualgraded'
                                AND mu.deleted = 0";

Could you please consider to accept this change. It will have a huge effect to efficiency and will allow us to start using the grade_me block again.

davidpesce commented 1 year ago

@outsei2 Thanks for the work on this. I don't maintain this plugin, but have a client that's running into these performance issues and wondered if you are using this PR in production? Which version of Moodle are you using?

outsei2 commented 1 year ago

Unfortunately we are not using this in our production environment. The IT department of our university does not allow custom modifications into production Moodle, so currently I can just wait and hope to get this into the official version of Grade Me plugin. We have used these changes in testing environment with no problems and the decrease in database operations is remarkable.

The Moodle version where I have tested this is 3.11.4+ (Build: 20211123)

davidpesce commented 1 year ago

Same here. We tried pushing this to a production environment, and it's unusable. Any teacher with a lot of courses would cause gateawy timeouts. Ended up uninstalling it and all is well.

outsei2 commented 1 year ago

Just for clarification. Do you mean that the original Grade me block was unusable for you and you had to uninstall it? Or that there is something wrong with my fix and this PR is not working?

outsei2 commented 1 year ago

Same here. We tried pushing this to a production environment, and it's unusable. Any teacher with a lot of courses would cause gateawy timeouts. Ended up uninstalling it and all is well.

@davidpesce Could you please clarify your earlier comment: did you tried my correction in your environment and if so, did you encountered any problems?

davidpesce commented 1 year ago

Hi @outsei2 - the plugin was still unusable with this PR. I believe more optimization is needed.