Details
-
Type:
Bug
-
Status:
Closed
-
Priority:
Minor
-
Resolution: Fixed
-
Affects Version/s: 2.1.1, 2.2
-
Fix Version/s: 2.1.2
-
Labels:
-
Environment:MySQL server version: 5.0.77
-
Database:MySQL
-
Testing Instructions:
-
Workaround:
-
Affected Branches:MOODLE_21_STABLE, MOODLE_22_STABLE
-
Fixed Branches:MOODLE_21_STABLE
-
Pull from Repository:
-
Pull Master Branch:
-
Pull Master Diff URL:
Description
Example: deleting question attempts on large MySQL database can take up to 10 minutes and is wasting database server resources.
Large database in this case means:
question_attempts table – ~ 1.5M rows
question_attempt_steps table – ~ 4M rows
question_attempt_step_data table – ~ 9M rows
The fix described in Workaround makes this action to take less then some seconds.
Issue Links
Activity
Clarification: I just think that the "EXISTS" alternative is always "faster" than the "IN" one. And how much "faster" it is, depends of the cardinality of each table and, of course, of the type of correlation between both (it can go from marginal faster to way more faster, but never be slower, based in my experience).
Side-silly-note: Is is any index missing in those tables? Perhaps the problem there is that we are using both qa.id and qa.questionusageid in the same query and MySQL only is able to use one index per table on each query (if I'm not wrong), so we are accessing one of them by index but not the other, aka, full scan forced. One trivial way to check if that is the cause would be to create one index on both columns and see if it performs better that way.
Ciao 
DELETE FROM question_attempt_step_data qasd WHERE EXISTS ( SELECT 1 FROM question_attempts qa JOIN question_attempt_steps qas ON qas.questionattemptid = qa.id WHERE qa.questionusageid = ? AND qasd.attemptstepid = qas.id )
There is a problem for MySQL with this one also: http://bugs.mysql.com/bug.php?id=5123 
I tested performance of this with similar SELECT query – that did perform about 2 times faster (original query 1 min 14.27 sec, this one 39.24 sec), but this is still very bad.
Indexes
No missing indexes.
Adding index for testing to see if it helps:
CREATE INDEX performance_test ON question_attempts (id, questionusageid);
- Original "DELETE ... WHERE IN" – 55.57 sec
- "SELECT ... WHERE EXISTS" – 32.86 sec
Little improvement, but still not very helpful.
Mart, thanks for the complete info. 5 more questions:
1) Which cardinality have those 3 tables. How many records are deleted in your tests on each execution (10, 100, 1000 ... 10^n accuracy is enough).
2) Which engine are you using MyISAM / InnoDB. Version? OS?
3) Can you attach / send me privately the complete "Server variables and settings" page from phpmyadmin to see how you've it configured? Pool / caches and friends.
Those were aimed to simulate your situation here. Now:
4) Do you run check / optimize and specially analyze your DB (those tables) from time to time? Any difference after that? It can cause dramatic differences if statistics haven't been run for a log time and numbers (cardinality, grow differences...) have changed a lot.
5) If 1) does not helps, surely we'll end needing to try "DELETE USING" / "DELETE JOIN" (not cross-db, will require conditional coding for each DB), or, alternatively, if continues being so slooow we can try to put target ids into temp table and then delete from it (surely cross-db).
Finally, one side note that we should not forget: DELETE operations are the most heavy operations performed by DBs. Specially if running under ACID transaction modes, where the whole recordset to be deleted must be calculated and kept available for the rest of sessions. It requires time and huge pool sizes. Not to talk about the time needed to rebuild indexes, write logs... just one reminder, not an excuse 
Ciao
Offtopic: Tim, when you want, we can, under standard conditions, weight and compare deleting and updating. But I'm not going to discuss about that here.
I asked for exact configuration to Mart because I created here one quick test, with 3 tables (not the real ones) and having one factor of 10x number of records between them. (10^4, 10^5 and 10^6 records) indexed similarly. And I got constant deletion times of 0.2 secs here (not specially quick notebook running MySQL 5.5). Note that 5.5 introduced some improvements about subquery handling.
So I'm aiming to see if the problem is something in the server (missing analyze / configuration / MySQL version...) before jumping to dbfamily specific alternatives.
Ciao
Thanks Mart!
I've attached above two files:
- testsql_performing_badly.php, one simpletest script used to build the case (the 3 tables) fill them and run all the possible queries under each DB-flavour in order to get the records deleted. You can execute it at any time in your (testing!) servers just putting it into under lib/simpletest and invoking it from admin->dev->unittests. It's not destructive at all, but better keep it away from any production machine.
- testsql_performing_badly.txt, the results of the execution under the 4 DBs (mysl, postgresql, mssql and oracle). For mysql I executed the test with 20000, 100000 and 500000 attempts. For the rest only the 100000 one.
Results are really clear:
1) Subqueries in MySQL are really horrible. Times grow linearly, so any of the IN / EQUAL / EXISTS alternatives are nono there. Only the JOIN or de USING alternatives are viable ones for big tables.
2) The rest of RDBMS perform subqueries really, really better, surely transforming them to sort of JOINs and other mechanishm. In general the worst alternative was the EQUAL one, with the IN being always better and the EXIST one beating them (specially under MSSQL and Oracle).
3) The propietary solutions: USING under PostgreSQL and JOIN under MSSQL also work great but are slightly different from the MySQL ones (also propietary), see .php file.
4) Disclaimer: The MySQL and PostgreSQL test were executed in my laptop natively. The MSSQL and Oracle ones in my laptop using virtual machine.
Summary: my +1 goes to:
1) Implement the JOIN solution for MySQL
2) Implement the EXISTS solution for the other 3 (it's cross-db 100%)
That is! Ciao
I think that, for stable current codebase if should be something like:
$dbfamily = $DB->get_dbfamily();
if ($dbfamily == 'xxxx') {
....
} else if ($dbfamily == 'yyyy') {
....
} else {
....
}
Or alternatively switch / case syntax (if the code to put on each case in not a monster). Both possible syntaxes always with the final (else/default) for the most-used case.
In the mean time, I'm trying to implement sort of delete_records_join() helper / dml method to see if it can be done automatically, so you pass the table to delete from, the complete join + where and it works out the better way for each DB. But that won't land into stables right now.
Ciao
I would +1 to send this to integration if:
1) You change the branch name, LOL.
2) We agree this will be replaced by generic solution once implemented and tested.
Ciao
Tested the fix in our conditions (database as in Description of this issue) and the actions that took multiple minutes before took some seconds now!
Tim, I've seen you've added one more use for the custom mysql deletions: delete_attempt_steps_for_mysql()
It annoyed me a bit to see the 1st param there named "test". Could it be renamed to something more "final"? (sql, attemptslist, qaidssql or whatever)
Ciao
Edited: Also, tiny detail: "This funciton is a work-around..."
yeah, i just ask you for amending those trivial changes to avoid un-useful history (if I add extra commit) or to break your history (if I amend your commit). But I've not problem at all about amending if you give me a global "ok" about that.
With it, I'll be amending these automatically in the future (comments / typos) and adding extra commit if something obvious was wrong in code.
Sounds ok? Do I have your global "ok"? Should I start with this?
About $test, well "risk of breaking anything" by changing two "test" occurrences to something more meaningful... hehe, sounds a bit excessive. In any case it's not a reason to reject the patch, just a suggestion for improve (readability) it. So feel free to ignore it.
Ciao
Deletion is much faster then before
Thanks for fixing this Tim.
Just guessing what happens if one question_attempt_steps has not any child question_attempt_step_data. That seems to be the case here. 
Perhaps outer joining should be provided instead?
Confirmed, outer joining question_attempt_step_data did the trick here. Just guessing if the 2 _mysql() functions are candidates for outer joining... in your hands, ciao
Thanks Tim, applying to master and 21_STABLE and sending upstream...
If in the mean time somebody can test both deletion and regrading is working of with that patch, it will be really welcome!
Trick: try it with one quiz containing some truefalse question, that's enough to reproduce the problem.
Ciao
That is totally ridiculous, but then this is MySQL we are talking about.
Your suggested 'solution' is no good. Not only is it fundamentally bad for performance to drag all the IDs over the network to PHP, only to send them back to the DB server, this will break on some database systems that have a maximum length limit on IN () lists (for example Oracle).
So, the problem seems to be that MySQL thinks that the subquery is a correlated subquery, even though it clearly isn't. I see, according to http://forums.mysql.com/read.php?24,5240,5266#msg-5266 that is build in brain-dead-ness. Can we find a way to rewrite the query that is comprehensible to MySQL's little brain, and which is also works on all our supported databases. I seem to remember that there is some issue with trying to use the DELETE ... JOIN syntax for cross-DB compatibility.
Here's a thought. Does it do any good to change the SQL to IN (SELECT DISTINCT ...). Probably not, but worth a try.
Oh, and by the way, your code is wrong. You should be using placeholders (that is, $DB->get_in_or_equal()) to build the second query.)
Adding Eloy, our DB expert.