Details
-
Type:
Bug
-
Status:
Open
-
Priority:
Major
-
Resolution: Unresolved
-
Affects Version/s: 1.9.3, 1.9.4, 1.9.5
-
Fix Version/s: None
-
Component/s: Administration
-
Labels:
-
Environment:LAMP
-
Database:MySQL
-
Affected Branches:MOODLE_19_STABLE
Description
On 1.9.3+ (Build: 20090109) stats are completely unusable for us.
My server tried to runs stats for ONE SINGLE DAY of logs on our site (a pilot of only 30-odd courses but with 30000 mostly-unused rows in the user table). After 8 HOURS, I had to restart MySQL in the morning as it was still locked up at 100% and moodle could not connect to the DB.
MySQL struggled for 6 HOURS on a single query!!
Here is the monstrous query. We showed it to our Computer Science SQL lecturer for a laugh who said 'it is definitely a very bad query. Using cross join on large tables is a definite NO'. Creating 1.2 billion rows from our little DB is insane.
- Time: 090306 6:30:42
- User@Host: moodle[moodle] @ localhost []
- Query_time: 21631 Lock_time: 0 Rows_sent: 0 Rows_examined: 1199857693
UPDATE mdl_stats_daily
SET stat2 = (SELECT COUNT(DISTINCT ra.userid)
FROM mdl_role_assignments ra JOIN mdl_context ctx
ON ctx.id = ra.contextid
CROSS JOIN mdl_course c
JOIN mdl_role_capabilities rc
ON rc.roleid = ra.roleid LEFT OUTER JOIN mdl_course_categories cc1
ON cc1.id = c.category
WHERE ra.roleid = mdl_stats_daily.roleid AND
c.id = mdl_stats_daily.courseid AND
((rc.capability = 'moodle/course:view' )
AND rc.permission = 1 AND rc.contextid = 1
AND (ctx.contextlevel = 10
OR (c.id = ctx.instanceid AND ctx.contextlevel = 50) OR (cc1.id = ctx.instanceid AND ctx.contextlevel = 40))) AND
EXISTS (SELECT 'x'
FROM mdl_log l
WHERE l.course = mdl_stats_daily.courseid AND
l.userid = ra.userid AND l.time >= 1236164400 AND l.time < 1236250800))
WHERE mdl_stats_daily.stattype = 'enrolments' AND
mdl_stats_daily.timeend = 1236250800 AND
mdl_stats_daily.courseid IN
(SELECT DISTINCT l.course
FROM mdl_log l
WHERE l.time >= 1236164400 AND l.time < 1236250800);
Attachments
Issue Links
Activity
- All
- Comments
- History
- Activity
- Source
- Test Sessions
Hi Tony,
have you tried to remove the string "TEMPORARY " from Matt's patch?
Since these 2 new temp tables, tmp_stats_*s, will be explicitly DROPped at the end of the process, TEMPORARY is not required and this could fix your Can't reopen table: 'l' error in MySQL. The same should apply to PostgreSQL too: in both systems, at the very end, a TEMPORARY table is used to strictly definy the life cycle of the table itself just within the user connection session i.e. assuming an automatic DROP statement, here explicitly coded.
Let me know since I'm interested too (+1) 
Matteo
Matteo Hi Matteo,
Sorry but I'm not really involved with this... if I just have time... I'll try to help.
Please, rewrite the query as in [1] and test (run it from mysql cilent, for instance. Do a RESET QUERY CACHE before). Please note that the query may contain repeated rows. I don't know if this can cause some problems in code... please review that 
Maybe not or maybe we can circumvent it from PHP.
[1]
CREATE TABLE mdl_tmp_stats_enrolments AS
SELECT DISTINCT ra.roleid, ra.userid, c.id AS courseid
FROM mdl_role_assignments ra JOIN mdl_context ctx ON ctx.id = ra.contextid
CROSS JOIN mdl_course c
JOIN mdl_role_capabilities rc ON rc.roleid = ra.roleid
LEFT OUTER JOIN mdl_course_categories cc1 ON cc1.id = c.category
WHERE ((rc.capability = 'moodle/course:view' ) AND rc.permission = 1 AND rc.contextid = 1 AND ctx.contextlevel = 10)
UNION
SELECT DISTINCT ra.roleid, ra.userid, c.id AS courseid
FROM mdl_role_assignments ra JOIN mdl_context ctx ON ctx.id = ra.contextid
CROSS JOIN mdl_course c
JOIN mdl_role_capabilities rc ON rc.roleid = ra.roleid
LEFT OUTER JOIN mdl_course_categories cc1 ON cc1.id = c.category
WHERE ((rc.capability = 'moodle/course:view' ) AND rc.permission = 1 AND rc.contextid = 1
AND c.id = ctx.instanceid AND ctx.contextlevel = 50)
UNION
SELECT DISTINCT ra.roleid, ra.userid, c.id AS courseid
FROM mdl_role_assignments ra JOIN mdl_context ctx ON ctx.id = ra.contextid
CROSS JOIN mdl_course c
JOIN mdl_role_capabilities rc ON rc.roleid = ra.roleid
LEFT OUTER JOIN mdl_course_categories cc1 ON cc1.id = c.category
WHERE ((rc.capability = 'moodle/course:view' ) AND rc.permission = 1 AND rc.contextid = 1
AND cc1.id = ctx.instanceid AND ctx.contextlevel = 40);
Maybe not or maybe we can circumvent it from PHP.
[1]
CREATE TABLE mdl_tmp_stats_enrolments AS
SELECT DISTINCT ra.roleid, ra.userid, c.id AS courseid
FROM mdl_role_assignments ra JOIN mdl_context ctx ON ctx.id = ra.contextid
CROSS JOIN mdl_course c
JOIN mdl_role_capabilities rc ON rc.roleid = ra.roleid
LEFT OUTER JOIN mdl_course_categories cc1 ON cc1.id = c.category
WHERE ((rc.capability = 'moodle/course:view' ) AND rc.permission = 1 AND rc.contextid = 1 AND ctx.contextlevel = 10)
UNION
SELECT DISTINCT ra.roleid, ra.userid, c.id AS courseid
FROM mdl_role_assignments ra JOIN mdl_context ctx ON ctx.id = ra.contextid
CROSS JOIN mdl_course c
JOIN mdl_role_capabilities rc ON rc.roleid = ra.roleid
LEFT OUTER JOIN mdl_course_categories cc1 ON cc1.id = c.category
WHERE ((rc.capability = 'moodle/course:view' ) AND rc.permission = 1 AND rc.contextid = 1
AND c.id = ctx.instanceid AND ctx.contextlevel = 50)
UNION
SELECT DISTINCT ra.roleid, ra.userid, c.id AS courseid
FROM mdl_role_assignments ra JOIN mdl_context ctx ON ctx.id = ra.contextid
CROSS JOIN mdl_course c
JOIN mdl_role_capabilities rc ON rc.roleid = ra.roleid
LEFT OUTER JOIN mdl_course_categories cc1 ON cc1.id = c.category
WHERE ((rc.capability = 'moodle/course:view' ) AND rc.permission = 1 AND rc.contextid = 1
AND cc1.id = ctx.instanceid AND ctx.contextlevel = 40);
Mmmm... forget about the repeated rows... it won't contain any
That's great Juan because we still do not have this resolved. We have gone through all patches and guidelines here. The only part we were stuck on was the last comment by yourself. If that could be a patch, that would be amazing thanks - Andrea
- Andrea Hi all... I'm about to finish a new patch. Not heavily reviewed... so I'll need some feedback... stay tunned
This is a preliminary patch to test. It just adresses daily statistics gathering problem... once we get it validated I'll start with weekly and monthly.
Please, if you fell comfortable with SQL, please help to ensure equivalence with the new and old queries and the new enrollment pre-calculated tables.
The patch probably wont work for now in postgrees, but it can be easily adapted.
Thanks
Hi again. I'm trying to manage the whole thing with git:
https://github.com/anariel/moodle/tree/MDL-18484
bye!
Added statslib.php-rev04.diff
This patch adds the following:
. solves a critical problem in rev03 patch
. creates temporary tables using XMLDB
. runs faster than rev03
Please... anyone interested in testing?
bye
Added statslib.php-rev06.php
This fixes some bugs.
I have validated the results against the original statslib.php version on a server with more than 1 million actions per moth.
It runs really quickly. More or less 50 seconds per day.
Please give it a try.
Not tested on servers other than mysql. Runs on moodle 1.9.x
This just optimizes daily statistics. I'll do some code cleanup.
Regards
Thanks Tony I need help with data validation... ie, if we get exactly the same results as with the original library.
Regards
I need help with data validation... ie, if we get exactly the same results as with the original library.
Regards
(sorry for the spam)
Uploaded statslib.php-rev08.diff
Just code cleanup.
Regards
Uploaded statslib.php-rev08.diff
Just code cleanup.
Regards Thanks Tony
Changes in roles may be the cause for the different numbers. The best way to test may be the following:
0. Use a copy of your database not in production. We are going to compare with the same resultset.
1. Calculate the stats with the original library
2. Copy the tables mdl_stats_daily and mdl_stats_user_daily. Just like this:
CREATE TABLE mdl_stats_daily_original LIKE
SELECT * FROM mdl_stats_daily;
CREATE TABLE mdl_stats_user_daily_original LIKE
SELECT * FROM mdl_stats_user_daily;
3. Now delete the rows just inserted in mdl_stats_daily and mdl_stats_user_daily.
4. Run the stats over the same period with the new statslib.php and with the same original data.
Now we can compare rows in both tables like this (sorry but :
SELECT COUNT
FROM mdl_stats_daily s1
WHERE timeend > "put here a reasonable timestamp " AND
EXISTS (
SELECT 1
FROM mdl_stats_daily_original s2
WHERE s1.timeend = s2.timeend AND
s1.userid = s2.userid AND
s1.courseid = s2.courseid AND
s1.roleid = s2.roleid AND
s1.stattype = s2.stattype AND
( s1.stat1 <> s2.stat1 OR
s1.stat2 <> s2.stat2 )
);
This COUNT should be 0
And this one too:
SELECT COUNT
FROM mdl_stats_user_daily s1
WHERE timeend > "put here a reasonable timestamp " AND
EXISTS (
SELECT 1
FROM mdl_stats_user_daily_original s2
WHERE s1.timeend = s2.timeend AND
s1.userid = s2.userid AND
s1.courseid = s2.courseid AND
s1.roleid = s2.roleid AND
s1.stattype = s2.stattype AND
( s1.statsreads <> s2.statsread OR
s1.statswrites <> s2.statswrites )
);
I've written this queries without testing them... so be careful. If I have time I'll do an easy script to automate this tests and others.
Thank you again for your time
Changes in roles may be the cause for the different numbers. The best way to test may be the following:
0. Use a copy of your database not in production. We are going to compare with the same resultset.
1. Calculate the stats with the original library
2. Copy the tables mdl_stats_daily and mdl_stats_user_daily. Just like this:
CREATE TABLE mdl_stats_daily_original LIKE
SELECT * FROM mdl_stats_daily;
CREATE TABLE mdl_stats_user_daily_original LIKE
SELECT * FROM mdl_stats_user_daily;
3. Now delete the rows just inserted in mdl_stats_daily and mdl_stats_user_daily.
4. Run the stats over the same period with the new statslib.php and with the same original data.
Now we can compare rows in both tables like this (sorry but :
SELECT COUNT
FROM mdl_stats_daily s1
WHERE timeend > "put here a reasonable timestamp " AND
EXISTS (
SELECT 1
FROM mdl_stats_daily_original s2
WHERE s1.timeend = s2.timeend AND
s1.userid = s2.userid AND
s1.courseid = s2.courseid AND
s1.roleid = s2.roleid AND
s1.stattype = s2.stattype AND
( s1.stat1 <> s2.stat1 OR
s1.stat2 <> s2.stat2 )
);
This COUNT should be 0
And this one too:
SELECT COUNT
FROM mdl_stats_user_daily s1
WHERE timeend > "put here a reasonable timestamp " AND
EXISTS (
SELECT 1
FROM mdl_stats_user_daily_original s2
WHERE s1.timeend = s2.timeend AND
s1.userid = s2.userid AND
s1.courseid = s2.courseid AND
s1.roleid = s2.roleid AND
s1.stattype = s2.stattype AND
( s1.statsreads <> s2.statsread OR
s1.statswrites <> s2.statswrites )
);
I've written this queries without testing them... so be careful. If I have time I'll do an easy script to automate this tests and others.
Thank you again for your time No problem. I was hoping to get away with not re-running the stats with the original library on my test VM because of the amount of time it would take, but it looks like it's going to be the only way to get accurate results.
I left it going overnight and so far it's managed to process 1 day with the original library - in 56995 seconds! That's even worse than I expected. So I guess we can expect the final results by about the middle of next week.
Hi again. I've validated again in the way similar to that described before. I get exactly the same results with the original library and the new one. So the patch is ok.
Someone is able to test with postgress and others?
I'll port this to 2.x
Regards
Hi Juan,
Sorry about the delay with my results. By the time the re-run with the original library had completed (it took about a week!) I was out of the office on a week's leave. Since I got back I've been trying to compare the 2 sets of results using the method you described above (exact query used below):
SELECT COUNT
FROM mdl_stats_daily s1
WHERE timeend > "1301616000" AND
EXISTS (
SELECT 1
FROM mdl_stats_daily_original s2
WHERE s1.timeend = s2.timeend AND
s1.courseid = s2.courseid AND
s1.roleid = s2.roleid AND
s1.stattype = s2.stattype AND
( s1.stat1 <> s2.stat1 OR
s1.stat2 <> s2.stat2 )
);
This one query has now been running for 517695 seconds (almost 6 days!) and still hasn't finished!
However, I have also done a visual comparison of the statistics report for the week in question, and the 2 sets of results are identical.
Tony
FROM mdl_stats_daily s1
WHERE timeend > "1301616000" AND
EXISTS (
SELECT 1
FROM mdl_stats_daily_original s2
WHERE s1.timeend = s2.timeend AND
s1.courseid = s2.courseid AND
s1.roleid = s2.roleid AND
s1.stattype = s2.stattype AND
( s1.stat1 <> s2.stat1 OR
s1.stat2 <> s2.stat2 )
);
This one query has now been running for 517695 seconds (almost 6 days!) and still hasn't finished!
However, I have also done a visual comparison of the statistics report for the week in question, and the 2 sets of results are identical.
Tony Hi Tyler,
This warning (in 1.9) is shown because get_records_sql returns false and we're giving foreach() a false value You can test $res before giving it to forach().
bye!
You can test $res before giving it to forach().
bye! 
I've been working on this in our test servers and found serveral approaches that can be used in order to improve performance and table (or row) contention. We work with mysql 5.0.x and Innodb. Maybe this can help:
1. Enrollments are calculated in nearly every query in daily stats. They can be precalculated (just one time) in an indexed-temporary table and then use this temporary table in queries.
2. Logs table can be precalculated too. I'm using a temporary table of logs with this time restrictions. This reduces contention in mdl_log table and allows working with less data.
3. The ORs in enrollments calculation can be decompossed in UNIONs. This performs better in mysql because of one-index per table restriction. Don't know postgress or oracle...
Hope this tips help.
Regards