MySQL Bugs: #45309: InnoDB does not rollback for delete and update queries if query was killed

Bug #45309	InnoDB does not rollback for delete and update queries if query was killed
Submitted:	3 Jun 19:17	Modified:	14 Jul 16:44
Reporter:	Alexander Rubin
Status:	Closed
Category:	Server: General	Severity:	S2 (Serious)
Version:	5.1, 6.0 bzr	OS:	Any
Assigned to:	Alexey Kopytov	Target Version:	5.1+
Tags:	regression, delete, upade, rollback, innodb
Triage:	Triaged: D2 (Serious)

Description:
InnoDB in MySQL 5.1 does not rollback for delete and update queries if the query was
killed. Test case:

MySQL 5.1, killing DELETE query

mysql> select count(*) from articles;
+----------+
| count(*) |
+----------+
|  1048576 |
+----------+
1 row in set (5.56 sec)

mysql> delete from articles2 where id > 1000000;
Query aborted by Ctrl+C
ERROR 1317 (70100): Query execution was interrupted
mysql>  select count(*) from articles2;
+----------+
| count(*) |
+----------+
|   906447 |
+----------+
1 row in set (0.69 sec)

One more way of killing query:

mysql> show processlist;
+----+------+-----------+------+---------+------+----------+------------------------------------------+
| Id | User | Host      | db   | Command | Time | State    | Info                        
            |
+----+------+-----------+------+---------+------+----------+------------------------------------------+
|  1 | root | localhost | test | Query   |    1 | updating | delete from articles2 where
id > 1000000 |
|  2 | root | localhost | NULL | Query   |    0 | NULL     | show processlist            
            |
+----+------+-----------+------+---------+------+----------+------------------------------------------+
2 rows in set (0.00 sec)

mysql> kill 1;
Query OK, 0 rows affected (0.00 sec)

mysql> show processlist;
+----+------+-----------+------+---------+------+-------+------------------+
| Id | User | Host      | db   | Command | Time | State | Info             |
+----+------+-----------+------+---------+------+-------+------------------+
|  2 | root | localhost | NULL | Query   |    0 | NULL  | show processlist |
|  8 | root | localhost | test | Sleep   |    5 |       | NULL             |
+----+------+-----------+------+---------+------+-------+------------------+
2 rows in set (0.00 sec)

mysql>  select count(*) from articles2;

+----------+
| count(*) |
+----------+
|   739561 |
+----------+
1 row in set (0.47 sec)

MySQL 5.1, killing UPDATE query

mysql> select count(*) from articles where section_id=100;
+----------+
| count(*) |
+----------+
|        0 |
+----------+
1 row in set (0.00 sec)

mysql> update articles set  section_id=100;
Query aborted by Ctrl+C
ERROR 1317 (70100): Query execution was interrupted
mysql> select count(*) from articles where section_id=100;
+----------+
| count(*) |
+----------+
|    65686 |
+----------+
1 row in set (0.05 sec)

This does not apply to insert into ... select from ...

To compare, MySQL 5.0:
mysql> select count(*) from articles;
+----------+
| count(*) |
+----------+
|  1048576 |
+----------+
1 row in set (0.67 sec)

mysql> delete from articles where id >100000;
Query aborted by Ctrl+C
ERROR 1317 (70100): Query execution was interrupted
mysql>
mysql> select count(*) from articles;
+----------+
| count(*) |
+----------+
|  1048576 |
+----------+
1 row in set (0.66 sec)

How to repeat:
See above.

Attaching my.cnf config for mysql 5.1 (nothing special):

skip-name-resolve
back_log = 50
max_connections = 400
max_connect_errors = 10
table_cache = 512
max_allowed_packet = 16M
binlog_cache_size = 1M
max_heap_table_size = 64M
thread_cache_size = 8
thread_concurrency = 8
ft_min_word_len = 2
default_table_type = InnoDB
thread_stack = 192K
tmp_table_size = 64M
server-id = 1
key_buffer_size = 32M
bulk_insert_buffer_size = 300M
myisam_sort_buffer_size = 300M
myisam_max_sort_file_size = 10G
myisam_max_extra_sort_file_size = 10G
myisam_repair_threads = 1

      innodb_additional_mem_pool_size = 16M
      innodb_buffer_pool_size = 1G
      innodb_data_file_path = ibdata1:10M:autoextend
      innodb_file_io_threads = 4
      innodb_thread_concurrency = 16
      innodb_flush_log_at_trx_commit = 1
      innodb_log_buffer_size = 8M
      innodb_log_file_size = 256M
      innodb_log_files_in_group = 3
      innodb_max_dirty_pages_pct = 90
      innodb_flush_method=O_DIRECT

Suggested fix:
Fix MySQL/InnoDB so it will rollback killed queries

From private discussion: autocommit is ON

Thank you for the report.

Verified as described.

test case for the testsuite

Attachment: bug45309.test (application/octet-stream, text), 1.01 KiB.

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().

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.

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)

I imply nothing at all on the patch in question.

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.

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 :).

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.

Hi! 

ok all thanks for explaining. I'm sorry I was just a bit alarmed about what "gamble"
meant. 

Thanks again.

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)

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)

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)

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.

Noted in 5.4.2 changelog because next 5.4 version will be 5.4.2 and not 5.4.4.

Ignore previous comment about 5.4.2.

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)

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)

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)

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)

The 5.4 fix has been pushed to 5.4.2.