Details
Bug
Closed
Major
Description
On our production site our staff programmer Cliff found this issue, when we added the winter/spring 2008 students to the DB, then course level manual backup will not complete. We did some debugging and found that the function user_check_backup in /backup/backuplib.php were not implemented efficiently and would generate big memory footprint:
function user_check_backup($course,$backup_unique_code,$backup_users,$backup_messages) {
//$backup_users=0-->all
// 1-->course (needed + enrolled)
// 2-->none
global $CFG;
global $db;
$count_users = 0;
//If we've selected none, simply return 0
if ($backup_users == 0 or $backup_users == 1) {
//Calculate needed users (calling every xxxx_get_participants function + scales users)
$needed_users = backup_get_needed_users($course, $backup_messages);
//Calculate enrolled users (students + teachers)
$enrolled_users = backup_get_enrolled_users($course);
//Calculate all users (every record in users table)
$all_users = backup_get_all_users();
//Calculate course users (needed + enrolled)
//First, needed
$course_users = $needed_users;
//Now, enrolled
if ($enrolled_users) {
foreach ($enrolled_users as $enrolled_user) {
$course_users[$enrolled_user->id]->id = $enrolled_user->id;
}
}
//Now, depending of parameters, create $backupable_users
if ($backup_users == 0) {
$backupable_users = $all_users;
} else {
$backupable_users = $course_users;
}
We rewrote it a little bit and are still testing the patch (patch is not included here in this ticket right now), but would like to create a bug ticket here so that people who are running large sites would aware of this issue. Thanks!
Attachments
Issue Links
Activity
- All
- Comments
- History
- Activity
- Source
- Test Sessions
Anthony yes you are right about the idea of avoiding populate the $all_users array only when it is in fact REALLY needed. I also like your idea to check the amount of memory available for a script and estimate how much memory it would take, and warn an administrator when they are getting into danger or a possible area of conflict. But I think this would probably take another moodle tracker ticket for "improvement", as I don't see such function in moodlelib.php right now... Merry Christmas! 
OK, the key is to avoid that line of "$all_users = backup_get_all_users();" when you does NOT need to, as this would create a big problem when you have LOTS of users (like 60,000+) and the backup process would break. I have attached a diff file here, we simply rewrite the logic a little bit. Hope this make sense...