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

MyISAM wrong server status flags

Details

    Description

      created after CONJ-948

      Synchronous connectors can rely on OK_Packet/EOF_Packet server status's SERVER_STATUS_IN_TRANS flag that explicitly indicate if a transaction is currently active.

      Java connector use that information to avoid issuing a rollback/commit command, skipping the command if there is no transaction.

      Problem concerns only MyISAM that never set SERVER_STATUS_IN_TRANS flag. Still there is some case when there is some kind of lock : ( example from serg: )

      create table t1 (a int)  engine=myisam;
      		 
       
      		 
      insert t1 values (1);

      and in another 

      drop table t1;

      this DROP TABLE will wait until the transaction ends.

      • either MyISAM should return SERVER_STATUS_IN_TRANS flag, so connector know there is really a commit to be issued.
      • either MyISAM doesn't set any lock. (It may be just an issue : for some metadata locks are taken on the object name, before the table is opened, before we know it's a MyISAM table.)

      Either way is ok.

      (Another solution would be to always issue a COMMIT/ROLLBACK statement when there is no need, but that would degrade performance dramatically for some application. For example when connections are given back to pools, pools generally issues a connection.rollback() to ensure connection state, and there is lots of application where that command would represent 25% of the commands)

      Attachments

        Issue Links

          causes

          Bug - A problem which impairs or prevents the functions of the product. CONJ-948 Under certain circumstances, Connector J does not send "commit"

          • Critical - Crashes, loss of data, severe memory leak.
          • Closed
          relates to

          Bug - A problem which impairs or prevents the functions of the product. MDEV-30889 Memory leak issues with MariaDB 10.6.12 and OOM Crashes

          • Critical - Crashes, loss of data, severe memory leak.
          • Confirmed

          Activity

            Ascending order - Click to sort in descending order
            sanja Oleksandr Byelkin added a comment -

            I added reporting server status and it is what we have now:

            create table t1 (a int) engine=innodb;
            		 
            set autocommit=0;
            		 
            --- INNODB start transaction
            		 
            start transaction;
            		 
            affected rows: 0
            		 
            status: 1
            		 
            select * from t1;
            		 
            a
            		 
            affected rows: 0
            		 
            status: 21
            		 
            insert t1 values (1);
            		 
            affected rows: 1
            		 
            status: 1
            		 
            select * from t1;
            		 
            a
            		 
            1
            		 
            affected rows: 1
            		 
            status: 21
            		 
            drop table t1;
            		 
            create table t1 (a int) engine=aria;
            		 
            commit;
            		 
            set autocommit=0;
            		 
            --- ARIA start transaction
            		 
            start transaction;
            		 
            affected rows: 0
            		 
            status: 1
            		 
            select * from t1;
            		 
            a
            		 
            affected rows: 0
            		 
            status: 1
            		 
            insert t1 values (1);
            		 
            affected rows: 1
            		 
            status: 1
            		 
            select * from t1;
            		 
            a
            		 
            1
            		 
            affected rows: 1
            		 
            status: 1
            		 
            drop table t1;
            		 
            create table t1 (a int) engine=innodb;
            		 
            commit;
            		 
            set autocommit=0;
            		 
            --- INNODB autmatic transaction
            		 
            select * from t1;
            		 
            a
            		 
            affected rows: 0
            		 
            status: 21
            		 
            insert t1 values (1);
            		 
            affected rows: 1
            		 
            status: 1
            		 
            select * from t1;
            		 
            a
            		 
            1
            		 
            affected rows: 1
            		 
            status: 21
            		 
            drop table t1;
            		 
            create table t1 (a int) engine=aria;
            		 
            commit;
            		 
            set autocommit=0;
            		 
            --- ARIA automatic transaction
            		 
            select * from t1;
            		 
            a
            		 
            affected rows: 0
            		 
            status: 0
            		 
            insert t1 values (1);
            		 
            affected rows: 1
            		 
            status: 0
            		 
            select * from t1;
            		 
            a
            		 
            1
            		 
            affected rows: 1
            		 
            status: 0
            		 
            drop table t1;

            test case:

            --source include/have_innodb.inc
            		 
            create table t1 (a int) engine=innodb;
            		 
            set autocommit=0;
            		 
            --echo --- INNODB start transaction
            		 
            --enable_info
            		 
            start transaction;
            		 
            select * from t1;
            		 
            insert t1 values (1);
            		 
            select * from t1;
            		 
            --disable_info
            		 
            drop table t1;
            		 
            create table t1 (a int) engine=aria;
            		 
            commit;
            		 
            set autocommit=0;
            		 
            --echo --- ARIA start transaction
            		 
            --enable_info
            		 
            start transaction;
            		 
            select * from t1;
            		 
            insert t1 values (1);
            		 
            select * from t1;
            		 
            --disable_info
            		 
            drop table t1;
            		 
            create table t1 (a int) engine=innodb;
            		 
            commit;
            		 
            set autocommit=0;
            		 
            --echo --- INNODB autmatic transaction
            		 
            --enable_info
            		 
            select * from t1;
            		 
            insert t1 values (1);
            		 
            select * from t1;
            		 
            --disable_info
            		 
            drop table t1;
            		 
            create table t1 (a int) engine=aria;
            		 
            commit;
            		 
            set autocommit=0;
            		 
            --echo --- ARIA automatic transaction
            		 
            --enable_info
            		 
            select * from t1;
            		 
            insert t1 values (1);
            		 
            select * from t1;
            		 
            --disable_info
            		 
            drop table t1;

            diff:

            diff --git a/client/mysqltest.cc b/client/mysqltest.cc
            		 
            index 2a6d4265e59..20b7762b0cf 100644
            		 
            --- a/client/mysqltest.cc
            		 
            +++ b/client/mysqltest.cc
            		 
            @@ -7729,11 +7729,13 @@ void append_metadata(DYNAMIC_STRING *ds,
            		 
             */
            		 
             
             void append_info(DYNAMIC_STRING *ds, ulonglong affected_rows,
            		 
            -                 const char *info)
            		 
            +                 const char *info, unsigned int status)
            		 
             {
            		 
               char buf[40], buff2[21];
            		 
               sprintf(buf,"affected rows: %s\n", llstr(affected_rows, buff2));
            		 
               dynstr_append(ds, buf);
            		 
            +  sprintf(buf,"status: %x\n", status);
            		 
            +  dynstr_append(ds, buf);
            		 
               if (info)
            		 
               {
            		 
                 dynstr_append(ds, "info: ");
            		 
            @@ -7985,7 +7987,8 @@ void run_query_normal(struct st_connection *cn, struct st_command *command,
            		 
                     query to find the warnings.
            		 
                   */
            		 
                   if (!disable_info)
            		 
            -	append_info(ds, mysql_affected_rows(mysql), mysql_info(mysql));
            		 
            +	append_info(ds, mysql_affected_rows(mysql), mysql_info(mysql),
            		 
            +                    mysql->server_status);
            		 
             
                   if (display_session_track_info)
            		 
                     append_session_track_info(ds, mysql);
            		 
            @@ -8409,7 +8412,8 @@ void run_query_stmt(struct st_connection *cn, struct st_command *command,
            		 
                   otherwise.
            		 
                 */
            		 
                 if (!disable_info)
            		 
            -      append_info(ds, mysql_stmt_affected_rows(stmt), mysql_info(mysql));
            		 
            +      append_info(ds, mysql_stmt_affected_rows(stmt), mysql_info(mysql),
            		 
            +                  mysql->server_status);
            		 
             
                 if (display_session_track_info)
            		 
                   append_session_track_info(ds, mysql);

            sanja Oleksandr Byelkin added a comment - I added reporting server status and it is what we have now: create table t1 (a int) engine=innodb; set autocommit=0; --- INNODB start transaction start transaction; affected rows: 0 status: 1 select * from t1; a affected rows: 0 status: 21 insert t1 values (1); affected rows: 1 status: 1 select * from t1; a 1 affected rows: 1 status: 21 drop table t1; create table t1 (a int) engine=aria; commit; set autocommit=0; --- ARIA start transaction start transaction; affected rows: 0 status: 1 select * from t1; a affected rows: 0 status: 1 insert t1 values (1); affected rows: 1 status: 1 select * from t1; a 1 affected rows: 1 status: 1 drop table t1; create table t1 (a int) engine=innodb; commit; set autocommit=0; --- INNODB autmatic transaction select * from t1; a affected rows: 0 status: 21 insert t1 values (1); affected rows: 1 status: 1 select * from t1; a 1 affected rows: 1 status: 21 drop table t1; create table t1 (a int) engine=aria; commit; set autocommit=0; --- ARIA automatic transaction select * from t1; a affected rows: 0 status: 0 insert t1 values (1); affected rows: 1 status: 0 select * from t1; a 1 affected rows: 1 status: 0 drop table t1; test case: --source include/have_innodb.inc create table t1 (a int) engine=innodb; set autocommit=0; --echo --- INNODB start transaction --enable_info start transaction; select * from t1; insert t1 values (1); select * from t1; --disable_info drop table t1; create table t1 (a int) engine=aria; commit; set autocommit=0; --echo --- ARIA start transaction --enable_info start transaction; select * from t1; insert t1 values (1); select * from t1; --disable_info drop table t1; create table t1 (a int) engine=innodb; commit; set autocommit=0; --echo --- INNODB autmatic transaction --enable_info select * from t1; insert t1 values (1); select * from t1; --disable_info drop table t1; create table t1 (a int) engine=aria; commit; set autocommit=0; --echo --- ARIA automatic transaction --enable_info select * from t1; insert t1 values (1); select * from t1; --disable_info drop table t1; diff: diff --git a/client/mysqltest.cc b/client/mysqltest.cc index 2a6d4265e59..20b7762b0cf 100644 --- a/client/mysqltest.cc +++ b/client/mysqltest.cc @@ -7729,11 +7729,13 @@ void append_metadata(DYNAMIC_STRING *ds, */ void append_info(DYNAMIC_STRING *ds, ulonglong affected_rows, - const char *info) + const char *info, unsigned int status) { char buf[40], buff2[21]; sprintf(buf,"affected rows: %s\n", llstr(affected_rows, buff2)); dynstr_append(ds, buf); + sprintf(buf,"status: %x\n", status); + dynstr_append(ds, buf); if (info) { dynstr_append(ds, "info: "); @@ -7985,7 +7987,8 @@ void run_query_normal(struct st_connection *cn, struct st_command *command, query to find the warnings. */ if (!disable_info) - append_info(ds, mysql_affected_rows(mysql), mysql_info(mysql)); + append_info(ds, mysql_affected_rows(mysql), mysql_info(mysql), + mysql->server_status); if (display_session_track_info) append_session_track_info(ds, mysql); @@ -8409,7 +8412,8 @@ void run_query_stmt(struct st_connection *cn, struct st_command *command, otherwise. */ if (!disable_info) - append_info(ds, mysql_stmt_affected_rows(stmt), mysql_info(mysql)); + append_info(ds, mysql_stmt_affected_rows(stmt), mysql_info(mysql), + mysql->server_status); if (display_session_track_info) append_session_track_info(ds, mysql);
            sanja Oleksandr Byelkin added a comment -

            Try to fix which does not go well:

            diff --git a/sql/mdl.cc b/sql/mdl.cc
            		 
            index a53a1f08c2c..7f55dbf6fd4 100644
            		 
            --- a/sql/mdl.cc
            		 
            +++ b/sql/mdl.cc
            		 
            @@ -1931,6 +1931,13 @@ MDL_context::try_acquire_lock_impl(MDL_request *mdl_request,
            		 
                 m_tickets[mdl_request->duration].push_front(ticket);
            		 
             
                 mdl_request->ticket= ticket;
            		 
            +
            		 
            +    if (mdl_request->duration == MDL_TRANSACTION &&
            		 
            +        mdl_request->type == MDL_EXCLUSIVE)
            		 
            +    {
            		 
            +      m_owner->get_thd()->server_status|= SERVER_STATUS_IN_TRANS;
            		 
            +    }
            		 
            +
            		 
               }
            		 
               else
            		 
                 *out_ticket= ticket;

            there are asserts which now fires and problem like that : 

            Cannot modify @@session.sql_log_bin inside a transaction

            sanja Oleksandr Byelkin added a comment - Try to fix which does not go well: diff --git a/sql/mdl.cc b/sql/mdl.cc index a53a1f08c2c..7f55dbf6fd4 100644 --- a/sql/mdl.cc +++ b/sql/mdl.cc @@ -1931,6 +1931,13 @@ MDL_context::try_acquire_lock_impl(MDL_request *mdl_request, m_tickets[mdl_request->duration].push_front(ticket); mdl_request->ticket= ticket; + + if (mdl_request->duration == MDL_TRANSACTION && + mdl_request->type == MDL_EXCLUSIVE) + { + m_owner->get_thd()->server_status|= SERVER_STATUS_IN_TRANS; + } + } else *out_ticket= ticket; there are asserts which now fires and problem like that : Cannot modify @@session.sql_log_bin inside a transaction
            sanja Oleksandr Byelkin added a comment -

            I updated time estimate (I have feeling we need to rewrite a lot to make it working as connector wants)

            sanja Oleksandr Byelkin added a comment - I updated time estimate (I have feeling we need to rewrite a lot to make it working as connector wants)
            sanja Oleksandr Byelkin added a comment -

            The fix require rewriting the status usage in the server, because the status flag (SERVER_STATUS_IN_TRANS) also taken into account by many server parts (like releasing transactional locks and so on), I am not sure such rewrite can be done in old versions.

            sanja Oleksandr Byelkin added a comment - The fix require rewriting the status usage in the server, because the status flag (SERVER_STATUS_IN_TRANS) also taken into account by many server parts (like releasing transactional locks and so on), I am not sure such rewrite can be done in old versions.
            sanja Oleksandr Byelkin added a comment -

            OK to push

            sanja Oleksandr Byelkin added a comment - OK to push

            People

              serg Sergei Golubchik
              diego dupin Diego Dupin
              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.