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

explain update ,.set..select freeze replica with autocommit=0

Details

    Description

      Testcase to reproduce:

      master

      create database d1;
      use d1;
       
      create table t1 (id int ,PRIMARY KEY (id) );
      create table t2 (id int ,PRIMARY KEY (id) );
       
      insert into t1 (id) values (1),(2),(3),(4);
      insert into t2 (id) values (1),(2),(3),(4);
      

      Replica

      set autocommit = 0;
      explain update t2 set id = (select id from t1);
      

      Master;

      set autocommit = 0;
      explain update t2 set id = (select id from t1);
      

      Replica:

      MariaDB [d1]> select * from t1;
      +----+
      | id |
      +----+
      |  1 |
      |  2 |
      |  3 |
      |  4 |
      +----+
      4 rows in set (0.000 sec)
      

      After commit , it works

      MariaDB [d1]> commit;
      Query OK, 0 rows affected (0.000 sec)
       
      MariaDB [d1]> select * from t1;
      +----+
      | id |
      +----+
      |  1 |
      |  2 |
      |  3 |
      |  4 |
      |  5 |
      +----+
      5 rows in set (0.000 sec)
      

      It seems related to default isolation level REPEATABLE READ.

      With isolation level READ COMITTED it works,
      but explain should not locks slave threads.

      Attachments

        Issue Links

          Activity

            Right. But making EXPLAIN UPDATE of limited use (because it'll see different data from the real UPDATE and can be optimized differently) is hardly an answer to this

            serg Sergei Golubchik added a comment - Right. But making EXPLAIN UPDATE of limited use (because it'll see different data from the real UPDATE and can be optimized differently) is hardly an answer to this

            Based on the above reasoning, decided to not apply the patch. Closing as Won't fix.

            psergei Sergei Petrunia added a comment - Based on the above reasoning, decided to not apply the patch. Closing as Won't fix.

            I looked at the code and it is not good the EXPLAIN UPDATE is regarded as changing data.
            The implications are:

            • EXPLAIN UPDATE cannot be used with read only slaves
            • We have a counter for select queries that is wrongly incremented for EXPLAIN
            • EXPLAIN <DML statement> counts as write in QUERY_RESPONSE_TIME plugin
            • EXPLAIN UPDATE can cause hangs in innodb as it takes an explicit lock.
            • Galera handles SELECT queries special, which is not needed for EXPLAIN.
            • EXPLAIN UPDATE is handled as an update in Galera, which probably is not desired.

            I do not think that it is important if EXPLAIN reads committed or not committed data based on EXPLAIN SELECT or EXPLAIN UPDATE. Just because InnoDB has a 'feature' that it ignores the isolation level for UPDATE does not mean that EXPLAIN UPDATE means to have it. For EXPLAIN, the possibility that a row may have a different value if there is a commit happening between lock tables and a row read is not important compared to the disadvantages listed above.

            I discussed this with Sergei Petrunia and we agreed that it is more important to fix the disadvantages than adore to exact way that InnoDB reads rows for update.

            The current suggestions is to add a SQLCOM commands for all the different EXPLAIN versions (EXPLAIN_SELECT, EXPLAIN_UPDATE, EXPLAIN_MULTI_UPDATE) and add an CF_EXPLAIN flag. We should also change all multi-if test of sql_command to use sql_command_flags[] to make the code shorter and more resilient for future changes.
            We should introduce a THD::command_flags() and thd_sql_command_flags(thd) to
            make it easier to use the flags everywhere.
            This is to be done in 11.7

            In 10.6 we need still to get rid of the InnoDB lock problem. The suggestion is to add
            THD::command_flags() and thd_sql_command_flags(thd) in 10.6 and use the command flags to simplify ha_innobase::store_lock().
            For explain, we should also fix that the table locks used for EXPLAIN are always TL_READ. Can be easily done in sql_yacc.yy at the end of the describe: handling.

            monty Michael Widenius added a comment - I looked at the code and it is not good the EXPLAIN UPDATE is regarded as changing data. The implications are: EXPLAIN UPDATE cannot be used with read only slaves We have a counter for select queries that is wrongly incremented for EXPLAIN EXPLAIN <DML statement> counts as write in QUERY_RESPONSE_TIME plugin EXPLAIN UPDATE can cause hangs in innodb as it takes an explicit lock. Galera handles SELECT queries special, which is not needed for EXPLAIN. EXPLAIN UPDATE is handled as an update in Galera, which probably is not desired. I do not think that it is important if EXPLAIN reads committed or not committed data based on EXPLAIN SELECT or EXPLAIN UPDATE. Just because InnoDB has a 'feature' that it ignores the isolation level for UPDATE does not mean that EXPLAIN UPDATE means to have it. For EXPLAIN, the possibility that a row may have a different value if there is a commit happening between lock tables and a row read is not important compared to the disadvantages listed above. I discussed this with Sergei Petrunia and we agreed that it is more important to fix the disadvantages than adore to exact way that InnoDB reads rows for update. The current suggestions is to add a SQLCOM commands for all the different EXPLAIN versions (EXPLAIN_SELECT, EXPLAIN_UPDATE, EXPLAIN_MULTI_UPDATE) and add an CF_EXPLAIN flag. We should also change all multi-if test of sql_command to use sql_command_flags[] to make the code shorter and more resilient for future changes. We should introduce a THD::command_flags() and thd_sql_command_flags(thd) to make it easier to use the flags everywhere. This is to be done in 11.7 In 10.6 we need still to get rid of the InnoDB lock problem. The suggestion is to add THD::command_flags() and thd_sql_command_flags(thd) in 10.6 and use the command flags to simplify ha_innobase::store_lock(). For explain, we should also fix that the table locks used for EXPLAIN are always TL_READ. Can be easily done in sql_yacc.yy at the end of the describe: handling.

            The changes needs to be done to allow EXPLAIN UPDATE to be done also on a read only system. This can be needed to understand what a slave is doing when using statement based replication.

            monty Michael Widenius added a comment - The changes needs to be done to allow EXPLAIN UPDATE to be done also on a read only system. This can be needed to understand what a slave is doing when using statement based replication.

            One option is if the server is read-only, issue a warning "cannot set UPDATE locks, the plan might not be the same as what UPDATE will be using" and switch internally to EXPLAIN SELECT.

            serg Sergei Golubchik added a comment - One option is if the server is read-only, issue a warning "cannot set UPDATE locks, the plan might not be the same as what UPDATE will be using" and switch internally to EXPLAIN SELECT.

            People

              psergei Sergei Petrunia
              Richard Richard Stracke
              Votes:
              1 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

                Created:
                Updated:

                Git Integration

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