Bug #45309 | InnoDB does not rollback for delete and update queries if query was killed | ||
---|---|---|---|
Submitted: | 3 Jun 2009 17:17 | Modified: | 14 Jul 2009 14:44 |
Reporter: | Alexander Rubin | Email Updates: | |
Status: | Closed | Impact on me: | |
Category: | MySQL Server: General | Severity: | S2 (Serious) |
Version: | 5.1, 6.0 bzr | OS: | Any |
Assigned to: | Alexey Kopytov | CPU Architecture: | Any |
Tags: | delete, innodb, regression, rollback, upade |
[3 Jun 2009 17:17]
Alexander Rubin
[3 Jun 2009 20:42]
Sveta Smirnova
From private discussion: autocommit is ON
[3 Jun 2009 20:54]
Sveta Smirnova
Thank you for the report. Verified as described.
[3 Jun 2009 21:04]
Sveta Smirnova
test case for the testsuite
Attachment: bug45309.test (application/octet-stream, text), 1.01 KiB.
[16 Jun 2009 12:05]
Marko Mäkelä
I am assigning this bug away from me, because this does not look like a storage engine bug after all. Storage engines perform DELETE and UPDATE statements row-by-row. Between rows, MySQL has the chance to observe thd->killed. InnoDB only observes thd->killed in CHECK TABLE, which is implemented by a single call to ha_innobase::check().
[23 Jun 2009 6:15]
Bugs System
A patch for this bug has been committed. After review, it may be pushed to the relevant source trees for release in the next version. You can access the patch from: http://lists.mysql.com/commits/76872 2968 Alexey Kopytov 2009-06-23 Bug #45309: InnoDB does not rollback for delete and update queries if query was killed Since we rely on thd->is_error() to decide whether we should COMMIT or ROLLBACK after a query execution, check the query 'killed' state and throw an error before calling ha_autocommit_or_rollback(), not after. The patch was tested manually. For reliable results, the test case would have to KILL QUERY while a DELETE/UPDATE query in another thread is still running. I don't see a way to achieve this kind of synchronization in our test suite (no debug_sync in 5.1). @ sql/sql_parse.cc Since we rely on thd->is_error() to decide whether we should COMMIT or ROLLBACK after query execution, check the query 'killed' state and throw an error before calling ha_autocommit_or_rollback(), not after.
[3 Jul 2009 14:45]
Roland Bouman
Hi All! in bug #45923(duplicate of this, 45309) Michael Izioumtchenko commented: "Regarding giving people chance to rollback, turning off autocommit should do it. With autocommit on, rollback on ctrl-c will still be a gamble even with the base bug fixed." This seems an unacceptable proposition. Firstly, even with autocommit off, I strongly feel the user should not be expected to rollback manually. To me, an unsuccessful query due to an abort is still an unsuccessful query - the behaviour should be no different from the statement failing due to a constraint violation (in which case an automatic rollback of the *statement* should be done, and any changes made in the same transaction prior to the failing statement should be preserved until rolled back or committed. Secondly, Michael seems to imply that with autocommit on, the patch committed for this bug does not guarantee to solve the problem. I mean "...will still be a gamble even with the base bug fixed..." still sounds like a bug to me - basically it means you cannot trust autocommit, ever, because you never know if you need to abort a statement. So the question is, what does this patch fix then ? Finally, I'd like to state my worries that an aborted statement that still commits changes could break replication. I mean, I think the aborted statement won't be written to the binlog, so there will be inconsistency, no? Anyway - please let us know if Michaels statement is correct here. If it is, it does not seem a good idea to submit any fix until a real fix is available. (and the current workaround would be to not use autocommit)
[3 Jul 2009 15:06]
Mikhail Izioumtchenko
I imply nothing at all on the patch in question.
[3 Jul 2009 19:36]
Davi Arnaut
By the time someone hits control-c (or attempt to kill the query), the query might have already been committed. This is the gamble because the commit happens automatically at the end of the query and the response might still be traveling back once the client hits the control-c. Kill is sent in a separate connection. What the patch tries to accomplish is to check right before autocommit if the thread/query is killed. If not, commit otherwise rollback.
[4 Jul 2009 8:06]
Konstantin Osipov
It's a regression that I introduced in the fix for Bug#12713. Now it's fixed. I don't think that there is much to discuss about it :).
[6 Jul 2009 14:14]
Mikhail Izioumtchenko
Roland, the patch will fix the perceived partial commit problem, based on the patch description in the base bug: check the query 'killed' state and throw an error before calling ha_autocommit_or_rollback(), not after.
[6 Jul 2009 20:27]
Roland Bouman
Hi! ok all thanks for explaining. I'm sorry I was just a bit alarmed about what "gamble" meant. Thanks again.
[8 Jul 2009 13:30]
Bugs System
Pushed into 5.1.37 (revid:joro@sun.com-20090708131116-kyz8iotbum8w9yic) (version source revid:alexey.kopytov@sun.com-20090626093256-tp8e2e2ed7xxn1s5) (merge vers: 5.1.37) (pib:11)
[9 Jul 2009 7:37]
Bugs System
Pushed into 5.1.37 (revid:joro@sun.com-20090708131116-kyz8iotbum8w9yic) (version source revid:alexey.kopytov@sun.com-20090626093256-tp8e2e2ed7xxn1s5) (merge vers: 5.1.37) (pib:11)
[10 Jul 2009 11:21]
Bugs System
Pushed into 5.4.4-alpha (revid:anozdrin@bk-internal.mysql.com-20090710111017-bnh2cau84ug1hvei) (version source revid:alexey.kopytov@sun.com-20090626093418-u8lihel5p2vnyfmk) (merge vers: 5.4.4-alpha) (pib:11)
[14 Jul 2009 14:44]
Paul DuBois
Noted in 5.1.37, 5.4.4 changelogs. If autocommit was enabled, InnoDB did not roll back DELETE or UPDATE statements if the statement was killed.
[12 Aug 2009 22:17]
Paul DuBois
Noted in 5.4.2 changelog because next 5.4 version will be 5.4.2 and not 5.4.4.
[14 Aug 2009 23:07]
Paul DuBois
Ignore previous comment about 5.4.2.
[26 Aug 2009 13:46]
Bugs System
Pushed into 5.1.37-ndb-7.0.8 (revid:jonas@mysql.com-20090826132541-yablppc59e3yb54l) (version source revid:jonas@mysql.com-20090826132541-yablppc59e3yb54l) (merge vers: 5.1.37-ndb-7.0.8) (pib:11)
[26 Aug 2009 13:46]
Bugs System
Pushed into 5.1.37-ndb-6.3.27 (revid:jonas@mysql.com-20090826105955-bkj027t47gfbamnc) (version source revid:jonas@mysql.com-20090826105955-bkj027t47gfbamnc) (merge vers: 5.1.37-ndb-6.3.27) (pib:11)
[26 Aug 2009 13:48]
Bugs System
Pushed into 5.1.37-ndb-6.2.19 (revid:jonas@mysql.com-20090825194404-37rtosk049t9koc4) (version source revid:jonas@mysql.com-20090825194404-37rtosk049t9koc4) (merge vers: 5.1.37-ndb-6.2.19) (pib:11)
[27 Aug 2009 16:33]
Bugs System
Pushed into 5.1.35-ndb-7.1.0 (revid:magnus.blaudd@sun.com-20090827163030-6o3kk6r2oua159hr) (version source revid:jonas@mysql.com-20090826132541-yablppc59e3yb54l) (merge vers: 5.1.37-ndb-7.0.8) (pib:11)
[7 Oct 2009 18:35]
Paul DuBois
The 5.4 fix has been pushed to 5.4.2.
[30 Jan 2017 19:11]
Sachin Tiwari
I know it's very old bug and my question is probably not relevant but shouldn't this issue (bug) be caught in the testing phase itself as it seems to be very basic test scenario that must exist and pass.