Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-17603

Allow statement-based replication for REPLACE and INSERT…ON DUPLICATE KEY UPDATE

Details

    Description

      MySQL 5.7.4 changed the behaviour of REPLACE and INSERT…ON DUPLICATE KEY UPDATE in the InnoDB storage engine. Upon encountering a duplicate key, it would no longer directly fall back to UPDATE, but instead it would proceed to acquire an exclusive lock on every index record for the row on which the INSERT failed.

      The extra locking was motivated by a public bug report: MySQL Bug#50413 insert on duplicate key update sometimes writes binlog position incorrectly (Oracle internal BUG#11758237). The fix was followed up by a couple of regression fixes.

      For one MariaDB user, reverting these changes significantly reduced the deadlock rate of INSERT…ON DUPLICATE KEY UPDATE.

      MDEV-17073 disabled the changes except when statement-based replication is being used. Even with statement-based replication, the locking could be replaced with additional logging: extending the replication event with information on which index records were locked when the duplicate was detected. (Note that the dict_table_t::indexes may be sorted differently on master and slave, especially if CREATE UNIQUE INDEX was executed with ALGORITHM=INPLACE.

      The documentation at https://mariadb.com/kb/en/binary-log-formats/ states that even if one is using Statement based logging, it's assumed that the tables are the same on master and slave and both are using the same storage engine. In this case Statement logging should work.
      In mixed mode, we prefer security over speed and in this case statements using ON DUPLICATE KEY UPDATE should use binary logging to ensure it works even if the slave is using different storage engines or different index definitions than the master.

      Attachments

        Issue Links

          Activity

            I reverted MDEV-17073 and the upstream change and implemented and tested a simplified version of what appeared in MySQL 5.7.26.

            Neither our test innodb.auto_increment_dup,log-bin nor the upstream test innodb.iodku would pass with those changes. (OK, I missed follow-up fixes to their test case.)

            What happens in innodb.auto_increment_dup,log-bin is that the newly converted explicit lock for the record will be released when that record is deleted by the rollback moments later.

            The idea of converting implicit locks to explicit on rollback is somewhat expensive. One motivation of MDEV-16232 is to reduce the amount of explicit locks by holding page latches across some handler API calls.

            Ideally, this problem would be fixed by changing something in replication, perhaps simply by refusing statement-based replication for these operations.

            marko Marko Mäkelä added a comment - I reverted MDEV-17073 and the upstream change and implemented and tested a simplified version of what appeared in MySQL 5.7.26 . Neither our test innodb.auto_increment_dup,log-bin nor the upstream test innodb.iodku would pass with those changes. (OK, I missed follow-up fixes to their test case.) What happens in innodb.auto_increment_dup,log-bin is that the newly converted explicit lock for the record will be released when that record is deleted by the rollback moments later. The idea of converting implicit locks to explicit on rollback is somewhat expensive. One motivation of MDEV-16232 is to reduce the amount of explicit locks by holding page latches across some handler API calls. Ideally, this problem would be fixed by changing something in replication, perhaps simply by refusing statement-based replication for these operations.
            Elkin Andrei Elkin added a comment -

            The priority is lowered to focus on MDEV-17614 first which tackles the issue when @@binlog_format is MIXED. (The scope of the current ticket as noted covers STATEMENT format).

            Elkin Andrei Elkin added a comment - The priority is lowered to focus on MDEV-17614 first which tackles the issue when @@binlog_format is MIXED. (The scope of the current ticket as noted covers STATEMENT format).

            Test Case:-

            --source include/master-slave.inc
            --source include/have_binlog_format_statement.inc
            --source include/have_innodb.inc
            CREATE TABLE t1 (a INT PRIMARY KEY, b INT UNIQUE KEY, c INT) ENGINE = InnoDB;
            INSERT INTO t1 VALUES (1, 1, 1);
            BEGIN;
            INSERT INTO t1 VALUES (2, 1, 2) ON DUPLICATE KEY UPDATE b=VALUES(b), c=VALUES(c);
              --connection master1
              INSERT INTO t1 VALUES(2, 2, 3) ON DUPLICATE KEY UPDATE b=VALUES(b), c=VALUES(c);
            --connection master
            COMMIT;
            SELECT * FROM t1;
            --sync_slave_with_master
            SELECT * FROM t1;
             
            --connection master
            drop table t1;
            --sync_slave_with_master
             
            --source include/rpl_end.inc
            
            

            sachin.setiya.007 Sachin Setiya (Inactive) added a comment - Test Case:- --source include/master-slave.inc --source include/have_binlog_format_statement.inc --source include/have_innodb.inc CREATE TABLE t1 (a INT PRIMARY KEY, b INT UNIQUE KEY, c INT) ENGINE = InnoDB; INSERT INTO t1 VALUES (1, 1, 1); BEGIN; INSERT INTO t1 VALUES (2, 1, 2) ON DUPLICATE KEY UPDATE b=VALUES(b), c=VALUES(c); --connection master1 INSERT INTO t1 VALUES(2, 2, 3) ON DUPLICATE KEY UPDATE b=VALUES(b), c=VALUES(c); --connection master COMMIT; SELECT * FROM t1; --sync_slave_with_master SELECT * FROM t1;   --connection master drop table t1; --sync_slave_with_master   --source include/rpl_end.inc

            Now that MDEV-17614 has fixed the correctness issue, I believe that this becomes a mere performance enhancement. I believe that such enhancements that involve file format changes should be avoided in GA versions.

            marko Marko Mäkelä added a comment - Now that MDEV-17614 has fixed the correctness issue, I believe that this becomes a mere performance enhancement. I believe that such enhancements that involve file format changes should be avoided in GA versions.
            sachin.setiya.007 Sachin Setiya (Inactive) added a comment - - edited

            After discussion with Elkin and monty , It is decided that this will no longer be fixed, User are advised to user mixed binlog_format , Since it is most optimized and takes care of these issues

            sachin.setiya.007 Sachin Setiya (Inactive) added a comment - - edited After discussion with Elkin and monty , It is decided that this will no longer be fixed, User are advised to user mixed binlog_format , Since it is most optimized and takes care of these issues

            People

              sachin.setiya.007 Sachin Setiya (Inactive)
              marko Marko Mäkelä
              Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.