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

Do not add temporary tables to dict_sys->table_hash

Details

    Description

      InnoDB is adding both persistent and temporary tables to a dict_sys->table_hash, which maps table names to dict_table_t.

      We should only add persistent tables to the hash table, and we should not store any dict_table_t::name for temporary tables. It would be nice to store the user-specified table name instead of some #sql name, but RENAME TABLE is not being propagated to temporary tables.

      The reason why we currently need the table name lookup is that handler::delete_table() can be invoked on temporary tables without prior handler::open(), so the only information that is available is the table name.
      If THD::rm_temporary_table() were invoked with TABLE_SHARE, we could store the dict_table_t* or dict_table_t::id there by invoking handler::set_ha_share_ptr() and handler::get_ha_share_ptr().

      For persistent tables, we will need both name and ID based lookups for the time being. (InnoDB rollback and purge will look up tables by ID, and the SQL layer will have to look up persistent tables by name.)

      As part of this task, please ensure that the table name printed in errors will be the real table name, not the temporary table name:

      InnoDB: Cannot add field `NEW` in table `tmp`.`#sql-temptable-10e7-7aa21-13bb` because after adding it, the row size is 8204 which is greater than maximum allowed size (8126 bytes) for a record

      Attachments

        Issue Links

          is blocked by

          Bug - A problem which impairs or prevents the functions of the product. MDEV-17794 Do not assign persistent ID for temporary tables

          • Major - Major loss of function.
          • Closed

          Bug - A problem which impairs or prevents the functions of the product. MDEV-25064 rpl.rpl_parallel_temptable failed in bb, ASAN heap-use-after-free in std::__atomic_base<long>::store

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

          Bug - A problem which impairs or prevents the functions of the product. MDEV-26152 Assertion: prebuilt->template_type == ROW_MYSQL_WHOLE_ROW fails in row0mysql.cc line 1752

          • Major - Major loss of function.
          • Confirmed

          Bug - A problem which impairs or prevents the functions of the product. MDEV-26433 assertion: table->get_ref_count() == 0 in dict0dict.cc line 1915

          • Blocker - Blocks development and/or testing work, production could not run.
          • Closed

          Bug - A problem which impairs or prevents the functions of the product. MDEV-34049 InnoDB: Failing assertion: table->get_ref_count() == 0 in dict_sys_t::remove on DROP

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

          Bug - A problem which impairs or prevents the functions of the product. MDEV-35154 dict_sys_t::load_table() is holding exclusive dict_sys.latch for unnecessarily long time

          • Major - Major loss of function.
          • Confirmed

          Task - A task that needs to be done. MDEV-12459 The information_schema tables for getting temporary tables info is missing, at least for innodb there is no INNODB_TEMP_TABLE_INFO

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

          Bug - A problem which impairs or prevents the functions of the product. MDEV-34051 SIGSEGV in dict_table_t::is_active_ddl during XA ... INSERT IGNORE

          • Major - Major loss of function.
          • Confirmed

          Activity

            Ascending order - Click to sort in descending order
            marko Marko Mäkelä added a comment - - edited

            svoj provided an untested MDEV-17805-wip.patch, which should provide TABLE_SHARE when dropping temporary tables. The InnoDB part remains to be written.

            marko Marko Mäkelä added a comment - - edited svoj provided an untested MDEV-17805-wip.patch , which should provide TABLE_SHARE when dropping temporary tables. The InnoDB part remains to be written.
            marko Marko Mäkelä added a comment -

            We should also remove the global dict_sys->temp_id_hash that was introduced in MDEV-17794, and instead iterate the THD::temporary_tables, using something similar to THD::find_tmp_table_share_w_base_key(). This should also remove any need to hold dict_sys->mutex or dict_operation_lock during operations with temporary tables.

            marko Marko Mäkelä added a comment - We should also remove the global dict_sys->temp_id_hash that was introduced in MDEV-17794 , and instead iterate the THD::temporary_tables , using something similar to THD::find_tmp_table_share_w_base_key() . This should also remove any need to hold dict_sys->mutex or dict_operation_lock during operations with temporary tables.
            marko Marko Mäkelä added a comment -

            I pushed an initial, non-working version of this to the 10.4-MDEV-17805 branch. svoj, could you help debugging it?

            marko Marko Mäkelä added a comment - I pushed an initial, non-working version of this to the 10.4-MDEV-17805 branch. svoj , could you help debugging it?
            svoj Sergey Vojtovich added a comment -

            serg, please review 5 top patches in bb-10.4-svoj-MDEV17805.
            Starting: "Removed redundant SE lock for tmp tables"
            Ending: "Get rid of rea_create_table()"

            svoj Sergey Vojtovich added a comment - serg , please review 5 top patches in bb-10.4-svoj-MDEV17805. Starting: "Removed redundant SE lock for tmp tables" Ending: "Get rid of rea_create_table()"
            serg Sergei Golubchik added a comment -

            marko what are user visible effects of this change?

            serg Sergei Golubchik added a comment - marko what are user visible effects of this change?
            marko Marko Mäkelä added a comment -

            serg, there should be no user visible effect of fixing this performance bug, other than the improved performance thanks to the removal of some shared data structures.

            I believe that the bug MDEV-10356 a.k.a. MDEV-25064 must be fixed before this simplification can be implemented. Currently, parallel replication seems to assume that temporary tables are accessible from any thread. That is conflicting with the InnoDB assumption that each temporary table is private to the connection that is was created in.

            marko Marko Mäkelä added a comment - serg , there should be no user visible effect of fixing this performance bug, other than the improved performance thanks to the removal of some shared data structures. I believe that the bug MDEV-10356 a.k.a. MDEV-25064 must be fixed before this simplification can be implemented. Currently, parallel replication seems to assume that temporary tables are accessible from any thread. That is conflicting with the InnoDB assumption that each temporary table is private to the connection that is was created in.
            marko Marko Mäkelä added a comment -

            serg, ideally, the storage engine needs to be able to attach its own table handle to handler or TABLE_SHARE, so that the table does not need to be looked up by a name in ha_innobase::delete_table().

            Recently, in MDEV-35000, I realized that some additional refactoring around Handler_share and Partition_share appears to be necessary.

            marko Marko Mäkelä added a comment - serg , ideally, the storage engine needs to be able to attach its own table handle to handler or TABLE_SHARE , so that the table does not need to be looked up by a name in ha_innobase::delete_table() . Recently, in MDEV-35000 , I realized that some additional refactoring around Handler_share and Partition_share appears to be necessary.

            People

              serg Sergei Golubchik
              marko Marko Mäkelä
              Votes:
              1 Vote for this issue
              Watchers:
              11 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.