Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #57 correction for unnecessary db operations #58

Open
wants to merge 1 commit into
base: MOODLE_311_STABLE
Choose a base branch
from

Conversation

outsei2
Copy link

@outsei2 outsei2 commented Jun 6, 2022

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
Copy link
Author

outsei2 commented Jun 14, 2022

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
Copy link
Author

outsei2 commented Nov 3, 2022

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
Copy link

@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
Copy link
Author

outsei2 commented Jan 25, 2023

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
Copy link

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
Copy link
Author

outsei2 commented Jan 30, 2023 via email

@outsei2
Copy link
Author

outsei2 commented Mar 21, 2023

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants