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

InnoDB: encapsulate trx_sys.mutex and trx_sys.trx_list into a separate class

Details

    Description

      This should be doable when MDEV-22593 is completed. Please also make them private so that they're not abused for other purposes.

      Attachments

        Activity

          As part of this cleanup, I think that it is acceptable to move trx_sys.mutex to somewhere else, but in 10.5, we must not change either the type of that mutex or the access patterns. In particular, innobase_kill_query() must hold both the equivalent of trx_sys.mutex and the trx_t::mutex. In some cases, the state change of trx_t::commit_in_memory() is only protected indirectly by trx_sys.mutex.

          marko Marko Mäkelä added a comment - As part of this cleanup, I think that it is acceptable to move trx_sys.mutex to somewhere else, but in 10.5, we must not change either the type of that mutex or the access patterns. In particular, innobase_kill_query() must hold both the equivalent of trx_sys.mutex and the trx_t::mutex . In some cases, the state change of trx_t::commit_in_memory() is only protected indirectly by trx_sys.mutex .

          I don't see innobase_kill_query() taking trx_sys.mutex. Or was that added recently?

          svoj Sergey Vojtovich added a comment - I don't see innobase_kill_query() taking trx_sys.mutex . Or was that added recently?

          +  if (trx_t* trx= thd_to_trx(thd))
          +  {
          +    lock_mutex_enter();
          +    trx_sys_mutex_enter();
          +    trx_mutex_enter(trx);
          

          I see. Though I don't see how it fixes the problem: what if trx gets released to the pool before these mutexes are locked?

          svoj Sergey Vojtovich added a comment - + if (trx_t* trx= thd_to_trx(thd)) + { + lock_mutex_enter(); + trx_sys_mutex_enter(); + trx_mutex_enter(trx); I see. Though I don't see how it fixes the problem: what if trx gets released to the pool before these mutexes are locked?

          svoj, yes, the code was changed recently in order to fix MDEV-17092, which is a race condition between KILL and victim connection being disconnected.

          https://github.com/MariaDB/server/commit/96f7a64d8ce4a6ddb84036c024c83a51cc2b8d89 looks OK to me with the following changes:

          • Retain the first comment in trx_t::commit_in_memory(). The trx_t::mutex is necessary protection in some cases. Non-locking auto-commit read-only transactions are generally ‘invisible’ to everything, except the KILL statement. But, read-write or locking transactions will likely need trx_t::mutex to protect trx_t::state changes.
          • Retain the last comment in trx_t::commit_in_memory(), but adjust the reference to trx_sys_t::mutex in it.
          • Retain the comment in innobase_kill_query(), and mention the trx_t::commit_in_memory() handling of auto-commit non-locking read-only transactions.
          marko Marko Mäkelä added a comment - svoj , yes, the code was changed recently in order to fix MDEV-17092 , which is a race condition between KILL and victim connection being disconnected. https://github.com/MariaDB/server/commit/96f7a64d8ce4a6ddb84036c024c83a51cc2b8d89 looks OK to me with the following changes: Retain the first comment in trx_t::commit_in_memory() . The trx_t::mutex is necessary protection in some cases. Non-locking auto-commit read-only transactions are generally ‘invisible’ to everything, except the KILL statement. But, read-write or locking transactions will likely need trx_t::mutex to protect trx_t::state changes. Retain the last comment in trx_t::commit_in_memory() , but adjust the reference to trx_sys_t::mutex in it. Retain the comment in innobase_kill_query() , and mention the trx_t::commit_in_memory() handling of auto-commit non-locking read-only transactions.

          People

            kevg Eugene Kosov (Inactive)
            svoj Sergey Vojtovich
            Votes:
            0 Vote for this issue
            Watchers:
            2 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.