Bug #96578 `Insert on duplicate key update` updates the wrong row
Submitted: 16 Aug 2019 22:08 Modified: 4 Feb 19:16
Reporter: Zhicheng Zhu Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: DML Severity:S2 (Serious)
Version:8.0.16, 5.7.27, 8.0.17 OS:Any
Assigned to: CPU Architecture:Any

[16 Aug 2019 22:08] Zhicheng Zhu
Description:
Long story short, as the title said, if we run multiple "insert on duplicated key update" concurrently, then one transaction may update the wrong row which belongs to another transaction. For details, please look at the repro steps. This happens in innodb.  

percona also reported a similar bug with same root cause in myrocks.
https://jira.percona.com/browse/PS-5675

How to repeat:
Repro Script:
# change innodb_autoinc_lock_mode to 2
source include/have_debug.inc;
source include/have_debug_sync.inc;

SET SESSION TRANSACTION ISOLATION LEVEL READ COMMITTED;
connect (con1, localhost, root,,);
SET SESSION TRANSACTION ISOLATION LEVEL READ COMMITTED;
connection default;

create table temp1 (a varchar(36) NOT NULL, b varchar(128) NOT NULL, c varchar(10240) NOT NULL);
insert into temp1 values  (1, 'b1', 'c1'), (2,'b2','c2'),(3,'b3','c3'),(4,'b4','c4'),(5,'b5','c5');
create table temp2 (a varchar(36) NOT NULL, b varchar(128) NOT NULL, c varchar(10240) NOT NULL);
insert into temp2 values (2,'b2','c2'),(3,'b3','c3'),(4,'b4','c4'),(5,'b5','c5');

CREATE TABLE extra (
  id bigint(20) unsigned NOT NULL AUTO_INCREMENT,
  a varchar(36) NOT NULL,
  b varchar(128) NOT NULL,
  c varchar(10240) NOT NULL,
  PRIMARY KEY (id),
  UNIQUE KEY uniq_idx (a,b)
) engine = innodb;
set DEBUG_SYNC = "ha_write_row_end WAIT_FOR flushed EXECUTE 1";
send insert into extra(a, b, c) select * from temp1 on duplicate key update c=values(c); 

connection con1;
insert into extra(a, b, c) select * from temp2 on duplicate key update c=values(c); 
set DEBUG_SYNC = "now SIGNAL flushed";

connection default;
reap;
select id, a, b, c from extra;

drop table extra;

Expect result:
id      a       b       c
1       1       b1      c1
2       2       b2      c2
3       3       b3      c3
4       4       b4      c4
5       5       b5      c5

Actual Result(Debug Mode):
mysqld: /data/users/mung/5.6/sql/handler.cc:3650: int handler::update_auto_increment(): Assertion `next_insert_id >= auto_inc_interval_for_cur_row.minimum()' failed.
Actual Result(Release Mode):
Unfortunately you have to leverage the DEBUG_SYNC to get a consistent repro. To simulate the release mode, just remove the Assertion `next_insert_id >= auto_inc_interval_for_cur_row.minimum()' in handler::update_auto_increment. You can see, the data is different from the expected result. 

Suggested fix:
When restoring the prev_insert_id, if the prev_insert_id has already been taken by other transaction, then we should no restore that insert_id and create a new one.
[17 Aug 2019 6:59] MySQL Verification Team
Hello Zhicheng Zhu,

Thank you for the report.

regards,
Umesh
[17 Aug 2019 7:00] MySQL Verification Team
Test results - 8.0.17, 5.7.27

Attachment: 96578_5.6.45_5.7.27_8.0.17.results (application/octet-stream, text), 200.80 KiB.

[20 Aug 2019 16:17] Zhicheng Zhu
Hello Umesh, 
Thanks for investigating this! I just looked at the result file. Just curious have you setup the DEBUT_SYNC point? This is actual the key step to create a consistent repro
[22 Aug 2019 11:12] MySQL Verification Team
Hi Zhicheng Zhu,

I haven't changed anything from your repro case while running on debug build.
While simulating on release build, as suggested by you - removed the Assertion `next_insert_id >= auto_inc_interval_for_cur_row.minimum()' in handler::update_auto_increment and commented out DEBUG_SYNC references from the mtr test case.

regards,
Umesh
[22 Aug 2019 16:07] Zhicheng Zhu
Hello Umesh, 
I guess there might be some confusion in my comments. To mimic release, just remove Assertion `next_insert_id >= auto_inc_interval_for_cur_row.minimum()', but keep DEBUG_SYNC. Keeping the DEBUG_SYNC is the key, otherwise you need to kept running two queries concurrently for multiple times to repo the issue, which isn't very convenient. **Please don't comment out the DEBUG_SYNC**
Thanks,
Zhicheng
[23 Aug 2019 6:57] MySQL Verification Team
Hi Zhicheng Zhu,

Thank you for the clarification, removed assertion but left the DEBUG_SYNC**.
Please note that this again requires debug build as release builds will complain "ERROR 1193 (HY000): Unknown system variable 'DEBUG_SYNC'".
Joining the test results shortly which confirms the reported issue now.

regards,
Umesh
[23 Aug 2019 6:57] MySQL Verification Team
Test results - 8.0.17

Attachment: 96578_8.0.17.results (application/octet-stream, text), 341.48 KiB.

[23 Aug 2019 15:26] Zhicheng Zhu
Hello Umesh,
Yes. "ERROR 1193 (HY000): Unknown system variable 'DEBUG_SYNC'" is expected if you run it under release mode. This is why we have to **remove assert in Debug mode to mimic the behavior in release mode** :-)
Let me know if you need anything
Thanks,
Zhicheng
[23 Aug 2019 15:26] Zhicheng Zhu
Hello Umesh,
Yes. "ERROR 1193 (HY000): Unknown system variable 'DEBUG_SYNC'" is expected if you run it under release mode. This is why we have to **remove assert in Debug mode to mimic the behavior in release mode** :-)
Let me know if you need anything
Thanks,
Zhicheng
[26 Nov 2019 15:35] Paul Dubois
Posted by developer:
 
Fixed in 5.6.47, 5.7.29, 8.0.19.

With multiple sessions executing concurrent INSERT ... ON DUPLICATE
KEY UPDATE statements into a table with an AUTO_INCREMENT column but
not specifying the AUTO_INCREMENT value, inserts could fail with a
unique index violation.
[26 Nov 2019 19:31] Zhicheng Zhu
Thanks for fixing this! Is there a way I can look at this fix and back port this to our system?
[4 Feb 18:00] Satya Bodapati
https://github.com/mysql/mysql-server/commit/cbd87693248cb4d2a217a8f1c13108e3eca1ecff
[4 Feb 19:16] Zhicheng Zhu
Awesome! Thanks for fixing this!