CPU % runs high even on an idle server caused by Custom Cert email task

[DeRespinis]

Moodle 4.1 and Moodle 4.0 Custom Cert Version 4.02 2022041902

CPU % runs high even on an idle server caused by Custom Cert email task.

The CPU % ranges from 0% to 3% on an idle machine with the Custom Cert email task disabled. Mostly sits at 0%

The CPU % ranges from 8% - 15% on an idle machine with the Custom Cert email task enabled. The CPU never stops and operates at a minimum of 8%

I noticed the change when I updated from 3.7 to 4.0. I went from 3.7 to 3.9 to 3.11 then to 4.0 and 4.1.

The Email task does not report any errors but the time to complete each task is taking about 20 seconds.

I have this running on both an 4-5 year old AWS Linux 2 LAMP and a new AWS Bitnami Moodle site.

This does not seem like correct behavior.

Thank for your help.

[mdjnelson]

It's a pretty complex task that tries to achieve a lot. I would be happy to merge any code that improves this task (maybe adding some caching?).

[Peterburnett]

Hi Folks,

I've stumbled here after an upgrade to 4.1 from 3.9, where I am seeing the email task use 4 million DB queries every run, while not actually issuing any certs at all.

After digging around on the Moodle tracker, it appears that 3.10 changed get_dynamic_info and introduced a fairly heavy overhead.

https://tracker.moodle.org/browse/MDL-72402

After doing a perf dump using tool_excimer, the flamegraph is basically all just stuck calculating uservisible here:

https://github.com/mdjnelson/moodle-mod_customcert/blob/MOODLE_402_STABLE/classes/task/email_certificate_task.php#L127

This is compounded because it needs to run a heavy calculation for visibility for n users, across n certificates, leading to a fairly topheavy n^2 complexity. After digging more into the code it seems fairly obvious we should filter basically everything we can before we check visibility for user, as it is by far the most heavy operation per user.

Additionally, if we can filter the outer loop down, the weight of the task can be greatly reduced. By introducing a time constraint on the certificate activities in scope, the calculation complexity can be limited to just the certificates that may potentially be in use.

In testing I introduced a constraint where at least one user had to have accessed the containing course between the run time of the previous task, and the next one. The results speak for themselves.

Im pretty sure this logic is correct, so I will PR the changes I have made here, but @mdjnelson Some feedback on whether we can actually trust this course access constraint would be appreciated, as I'm not across all the edge cases in customcert.

[mdjnelson]

Thanks @Peterburnett! Will look into this soon. Hoping to get a new release out with some big issues people are wanting resolved.

[mohamedmohamedatia]

I propose considering the following workarounds if you think they are useful or may help optimize this task:

1- Batch Processing: -Modify the task to handle certificates in batches during each run. -Implement a mechanism to resume processing from the last stopped point in subsequent runs.

2- Admin Configurable Options: -Allow administrators to exclude certain older courses from certificate checks, especially those that have been closed for an extended period. -Introduce parameters allowing administrators to specify the number of certificates processed per run, with a default processing rate (e.g., 10 to 50 certificates) that can be handled in a minute before the next run.

3- Two-Task Approach: -Divide the task into two components: a. One task for processing a limited number of certificates at regular intervals. b. Another task for periodic, comprehensive checks of all certificates, perhaps on a weekly basis.

These suggestions aim to enhance the efficiency of the email certificate task, providing administrators with more control over the processing load based on their environment and requirements.

[mohamedmohamedatia]

Hello Everyone,

I've implemented some of the suggestions mentioned in my previous reply here.

While conducting a thorough code review, I identified an opportunity to reduce the number of database calls. In the existing code, there was a loop over all enrolled users in the course for each certificate. However, I modified it to iterate only over users who have access to the certificate. This adjustment significantly reduces the number of database read and write queries, particularly in scenarios where multiple certificates exist per course (e.g., one for students and one as a thank-you certificate for teachers). In such cases, the loop to email thank-you certificates will now only iterate over teachers instead of all users. Additionally, if we have 1000 students enrolled and only 200 users have access to the certificate, the loop will now run over 200 users only.

Additionally, I optimized the retrieval of users with the "manage" capability by fetching them once per certificate and storing them in an array. This approach eliminates the need to fetch users with this capability for each iteration in the user loop.

Although I didn't make changes to the code related to the comment "// Ensure the cert hasn't already been issued, e.g., via the UI (view.php) - a race condition," I believe removing this redundant check could further enhance performance. This check currently contributes to approximately 10% of database reads. However, I would appreciate input on whether this rare condition occurs and if users would receive two different copies of the certificate or two emails with one copy.

You can access the improved code in this GitHub fork: moodle-mod_customcert.

I've attached a PDF detailing the improvements in terms of database read/writes and processing time reductions. Custom Certificate performance comparison - 600 Certificate.pdf

[mdjnelson]

Thanks @mohamedmohamedatia! I have created https://github.com/mdjnelson/moodle-mod_customcert/pull/610 on your behalf. I will leave some comments here for you to have a look at.

For future reference if you are making changes you should create a different branch.

[mohamedmohamedatia]

Thanks, Mike, for creating issue #610 and for your feedback. I'm new to contributing to Moodle plugins, and this was my first attempt. I'll follow your suggestion to create a separate branch for future changes.

[mdjnelson]

You're welcome. Also, just fyi it's Mark - but Mike is close enough. 😄

[mohamedmohamedatia]

My apologies for the mix-up! I'll make sure to remember it's Mark from now on. 😄