History | Log In     View a printable version of the current page.  
We are currently focused especially on Moodle 2.0, Moodle 1.9.x bugs and Moodle 1.9.x testing.    Confused? Lost? Please read this introduction to the Tracker.
Issue Details (XML | Word | Printable)

Key: MDL-12594
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Minor Minor
Assignee: Eloy Lafuente (stronk7)
Reporter: Dan Poltawski
Votes: 3
Watchers: 3
Operations

If you were logged in you would be able to see more operations.
Moodle

Restoring a course with 400 users enrolled costs 13k DB queries

Created: 16/Dec/07 09:15 AM   Updated: 13/May/08 04:48 PM
Component/s: Backup
Affects Version/s: 1.9
Fix Version/s: 1.9.1

File Attachments: 1. Zip Archive backup-cf101-20080121-1619.zip (74 kb)
2. File restorelib.php (393 kb)

Issue Links:
Dependency
 

Participants: Dan Poltawski, Eloy Lafuente (stronk7), Pat B. and Petr Škoda
Security Level: None
QA Assignee: Petr Škoda


 Description  « Hide
Ouch, was just playing with a course with 400 users and restoring it cost 13k db queries!

I quickly put a few mtraces in restore_create_users with $PERF->dbqueries and it seems to cost about 6000 queries on itself to end up not doing much.

 All   Comments   Change History   Version Control      Sort Order: Ascending order - Click to sort in descending order
Dan Poltawski - 22/Jan/08 01:24 AM
Attaching a backup file with 2000 users assigned as students to it (and default blocks).

To restore this course takes 56K queries.

Pat B. - 28/Jan/08 11:41 PM
After tried testing restore your backup files with 2000 users, It took over 70000 queries !!
I'm running Moodle 1.9 and I've focused on reducing queries of restoring backup to existing moodle (users already exist). I tracked the queries in restore_create_users function (in backup/restorelib.php). This function alone took around 26000 queries. I found these queries are executed for every user added. a total of 13 queries/user.

1 Query SELECT * FROM mdl_backup_ids WHERE backup_code = '1201177218'
AND table_name = 'user' AND old_id = '1014' LIMIT 1
1 Query DELETE FROM mdl_backup_ids
                               WHERE backup_code = 1201177218 AND
                                     table_name = 'user' AND
                                     old_id = '1014'
1 Query INSERT INTO mdl_backup_ids ( BACKUP_CODE, TABLE_NAME, OLD_ID,
NEW_ID, INFO ) VALUES ( 1201177218, 'user', 1014, 0, 'infile' )

Above queries I thought It can't be reduced, so take a look at these

1 SELECT * FROM mdl_backup_ids WHERE backup_code = '1201177218' AND table_name =
'user' AND old_id = '1015' LIMIT 1
2 SELECT * FROM mdl_user WHERE username = 'firstname1013' AND mnethostid = '1' LIMIT 1
3 DELETE FROM mdl_backup_ids
                        WHERE backup_code = 1201177218 AND
                              table_name = 'user' AND
                              old_id = '1015'
4 INSERT INTO mdl_backup_ids ( BACKUP_CODE, TABLE_NAME, OLD_ID, NEW_ID, INFO )
VALUES ( 1201177218, 'user', 1015, 26, 's:6:"exists";' )
5 SELECT * FROM mdl_backup_ids WHERE backup_code = '1201177218' AND table_name =
'user' AND old_id = '1015' LIMIT 1
6 DELETE FROM mdl_backup_ids
                        WHERE backup_code = 1201177218 AND
                              table_name = 'user' AND
                              old_id = '1015'
7 INSERT INTO mdl_backup_ids ( BACKUP_CODE, TABLE_NAME, OLD_ID, NEW_ID, INFO )
VALUES ( 1201177218, 'user', 1015, 26, 's:14:"exists,needed,";' )
8 SELECT * FROM mdl_backup_ids WHERE backup_code = '1201177218' AND table_name =
'user' AND old_id = '1015' LIMIT 1
9 SELECT * FROM mdl_user_preferences WHERE userid = '26' AND name =
'auth_forcepasswordchange' LIMIT 1
10 SELECT * FROM mdl_user_preferences WHERE userid = '26' AND name =
'create_password' LIMIT 1

Restoring to a new, never-contain-existing-user-before database cost 19 queries/user. But restoring to an existing database (which already contained user) sill cost 13 queries/user.

Line 5-7 will be execute if $create_roles flagged as true. and Line 8-10 will be executed if $create_preference flagged as true.

But If we can know early that roles/preferences we're going to insert is the same as roles/preferences exist. We can reduce this 6 queries/user to something less.

I made a restorelib.php with a modified restore_create_users() function. I just to set $create_role and $create_preference to false if $create_user is false (line 2271 to 2275). However this can't be used since I can't just set those flag to false if $create_user is false. In case that there's already a user exist, the user might has some special preferences or roles assigned after the current database (and need to be inserted though it's already exist).

You may check out a discussion about this at http://code.google.com/p/google-highly-open-participation-moodle/issues/detail?id=104

Pat B. - 28/Jan/08 11:45 PM
My modified restorelib.php file.

Eloy Lafuente (stronk7) - 06/Feb/08 02:49 AM
Hi,

IMO it's perfect to skip creating preferences if the user exists (i.e. if $create_user = false). In fact that's the behaviour I've followed recently for other restore parts (user_tags and user_custom_profile_fields). See MDL-6856.

So I'd really vote +1 for that change (create_preferences=false) will save a ton of queries when restoring in the same server, where the users already exists.

About the role change, it's a bit more complex, because we are supporting two types of roles in the restore process: restore of old roles, produced by Moodle <= 1.6 that are created by the restore_create_users() function, and restore of new roles, capabilities and overrides for Moodle >= 1.7 that are restored in another place in the process.

I wouldn't change that part for now. Recently I've addressed MDL-13296 for that and would leave and test everything carefully to continue supporting those old roles.

So, definitively, my +1 to avoid restoring preferences when user exists. It's a simple change, and seems 100% safe and aligned with other parts of restore. And will have a big impact because each user can have a lot of preferences son number of queries are really big.

Ciao :-)

Pat B. - 07/Feb/08 05:35 PM
Thanks very much, Eloy

I don't know that in restore progress it will overwrite all user data if that username exist or not. If true, then the process need to delete all preferences and reinsert them too. (But again, I don't know much about Moodle database)

If you're right. Now we saved at least 2 queries/user. :D

I've something here. This query I think it's to check user existence. You see the first query of the first part is

SELECT * FROM mdl_backup_ids WHERE backup_code = '1201177218'
AND table_name = 'user' AND old_id = '1014' LIMIT 1

the first query of the second part is
SELECT * FROM mdl_backup_ids WHERE backup_code = '1201177218' AND table_name =
'user' AND old_id = '1015' LIMIT 1

and line 5 of the second part is
SELECT * FROM mdl_backup_ids WHERE backup_code = '1201177218' AND table_name =
'user' AND old_id = '1015' LIMIT 1

All seems Identical. (In this example is not, but just change the first part from old_id = '1014' to 1015. As in actual log, the first 3 queries and last 10 queries are processed for every user). Which mean we're checking user existence for 3 times.

If we already know the user exist, we don't need to recheck over and over again in part 2 anymore. But I'm not sure about this, you should take a look at this.

Cheers,
Pat

Eloy Lafuente (stronk7) - 07/Feb/08 07:07 PM
Hi Pat,

two comments:

1) The "general" police, when restoring users is "not modify them at all if the user existis". So, if a user exists then, the $create_user will be false and all the related information (preferences, custom_fields, tags...) won't be inserted at all. That's the cause for my previous +1 to set $create_preferences=false in my previous post. So we don't need to delete preferences at all (because they only will be restored if don't exist).

2) About the repeated queries you comment....yup, I detected the same some days ago. In fact I saved one of them recently:

http://cvs.moodle.org/moodle/backup/restorelib.php?r1=1.283.2.20&r2=1.283.2.21

There is another one that is right now under examination, as part of one change in the restore of old-roles that will improve a lot the restore of new-roles (that is executed later in the process)... stay tuned about that in MDL-13296

Thanks a lot for your effort, Pat! And of course, feel free to continue examining, detecting and reporting things like this. It's really important to perform this sort of profiling!

Thanks a lot! Ciao :-)

Eloy Lafuente (stronk7) - 13/May/08 02:02 AM
Applied. Now user preferences are not restored if user exists. This will save 2 queries per existing user.

Ciao :-)

Petr Škoda - 13/May/08 04:48 PM
reviewed, thanks