Vladislav Vaintroub added a comment - 2019-11-22 08:59 Ok to push

Sergey Vojtovich added a comment - 2019-11-25 15:12 Pushed to 10.5: commit 38c2c16cc4aae4c09910537a53a924f7768726dd Author: Sergey Vojtovich <svoj@mariadb.org> Date: Wed Jun 19 18:20:49 2019 +0400   Removed tc_purge() and purge_tables() argument   It was mistakenly used by tdc_start_shutdown() to make sure TABLE_SHARE gets evicted from table definition cache when it becomes unused. However same effect is achieved by resetting tdc_size and tc_size.   Part of MDEV-17882 - Cleanup refresh version   commit 092834cd2cd90c73623c019eb030f25740843e81 Author: Sergey Vojtovich <svoj@mariadb.org> Date: Wed Jun 19 17:42:37 2019 +0400   Removed kill_delayed_threads_for_table()   After 7fb9d64 it is used only by ALTER/DROP SERVER, which most probably wasn't intentional as Federated never supported delayed inserts anyway.   If delayed inserts will ever become an issue with ALTER/DROP SERVER, we should kill them by acquiring X-lock instead.   Part of MDEV-17882 - Cleanup refresh version   commit 0aa807d10051e1231bf3d4457db16ad8e66111aa Author: Sergey Vojtovich <svoj@mariadb.org> Date: Wed Jun 19 14:59:36 2019 +0400   Removed tdc_increment_refresh_version()   It is never called after 7fb9d64, which makes the whole refresh version infrastructure useless.   Removed: - tdc_version_t - TDC_VERSION_MAX - tdc_version - TDC_element::version - tdc_increment_refresh_version() - tdc_refresh_version() - refresh_version argument of tdc_wait_for_old_version() - Flush_commands status variable - refresh version from COM_STATISTICS - refresh version from dbug printouts   Part of MDEV-17882 - Cleanup refresh version

Sergey Vojtovich added a comment - 2019-12-27 16:51 - edited serg , please review another patch set at https://github.com/MariaDB/server/commits/bb-10.5-svoj-MDEV-17882 It was inspired by midenok's e6d653b448d4d4c327870c87530fc1a45248bbd6. Now tdc_remove_table() has a lot less callers.

Sergey Vojtovich added a comment - 2020-04-03 09:03 Second patch set pushed to 10.5: commit 4197014ba0ba8cb895f3b49b7fdf8bbfe9fca826 Author: Sergey Vojtovich <svoj@mariadb.org> Date: Sat Dec 21 23:51:49 2019 +0400   Yet less TDC hash lookups   Let auto repair table and truncate table routines flush TABLE_SHARE directly.   Part of MDEV-17882 - Cleanup refresh version   commit 7a947614fbf8b925ae3df70ec8df80c745eafd4c Author: Sergey Vojtovich <svoj@mariadb.org> Date: Tue Dec 17 16:25:15 2019 +0400   Split tdc_remove_table()   TDC_RT_REMOVE_ALL -> tdc_remove_table(). Some occurrences replaced with TDC_element::flush() (whenver TABLE_SHARE is available).   TDC_RT_REMOVE_NOT_OWN[_KEEP_SHARE] -> TDC_element::flush(). These modes assume that current thread owns TABLE_SHARE reference, which means we can avoid hash lookup and flush unused TABLE instances directly.   TDC_RT_REMOVE_UNUSED -> TDC_element::flush_unused(). Only [ab]used by mysql_admin_table() currently. Should be removed eventually.   Part of MDEV-17882 - Cleanup refresh version   commit 06fae75859821fe36f68eb2d77f007f014143282 Author: Sergey Vojtovich <svoj@mariadb.org> Date: Wed Dec 18 15:02:55 2019 +0400   tc_remove_all_unused_tables() cleanup   As tc_purge() never marks share flushed, let tdc_remove_table() do it directly.   Part of MDEV-17882 - Cleanup refresh version   commit 14e1385691a4a6c19f26e0ecc9df006dd9196396 Author: Sergey Vojtovich <svoj@mariadb.org> Date: Tue Dec 17 23:00:23 2019 +0400   Proper locking for mysql.gtid_slave_pos truncation   Aim of this patch is to remove tdc_remove_table(TDC_RT_REMOVE_UNUSED), which was mistakenly introduced by 055a3334a.   InnoDB allows only one open TABLE instance while performing table truncation. To fulfill this requirement: 1. MDL_EXCLUSIVE has to be acquired to block concurrent threads from accessing given table 2. cached TABLE instances have to be flushed 3. another InnoDB requirement is such that TABLE_SHARE and remaining TABLE instance have to be invalidated and re-opened after truncation   This goes more or less inline with what regular TRUNCATE TABLE does.   Alternative solution would be handler::ha_delete_all_rows(), but InnoDB doesn't implement it unfortunately.   Part of MDEV-17882 - Cleanup refresh version   commit bfdd30d3e92d3bf5570cc050967098c6b5a2d73a Author: Sergey Vojtovich <svoj@mariadb.org> Date: Sat Dec 21 01:22:09 2019 +0400   Fixed close_cached_connection_tables() flushing   Let DROP SERVER and ALTER SERVER perform fair affected tables flushing. That is acquire MDL_EXCLUSIVE and do tdc_remove_table(TDC_RT_REMOVE_ALL).   Aim of this patch is elimination of another inconsistent use of TDC_RT_REMOVE_UNUSED. It fixes (to some extent) a problem described in the beginning of sql_server.cc, when close_cached_connection_tables() interferes with concurrent transaction.   A better fix should probably introduce proper MDL locks for server objects?   Part of MDEV-17882 - Cleanup refresh version   commit 54c03cb4f073b4c3140e75d1747798bace96fdba Author: Sergey Vojtovich <svoj@mariadb.org> Date: Fri Dec 20 20:33:29 2019 +0400   Cleanup mysql_inplace_alter_table()   Removed redundant tdc_remove_table(TDC_RT_REMOVE_ALL). Share was marked flushed by preceding wait_while_table_is_used() and eventually flushed by close_all_tables_for_name().   Part of MDEV-17882 - Cleanup refresh version   commit 02619ed73bc303e511bb8df3df67120e47886ffb Author: Sergey Vojtovich <svoj@mariadb.org> Date: Fri Dec 20 17:20:56 2019 +0400   Cleanup close_all_tables_for_name()   close_all_tables_for_name() is always preceded by wait_while_table_is_used(), which makes tdc_remove_table() redundant. The only (now fixed) exception was close_cached_tables().   Part of MDEV-17882 - Cleanup refresh version   commit e0743bd1a5523fd65da49fcbe846f207ea95d926 Author: Sergey Vojtovich <svoj@mariadb.org> Date: Wed Dec 18 01:18:19 2019 +0400   Let "FTWRL <table_list>" use extra(HA_EXTRA_FLUSH)   Rather than flushing caches with tdc_remove_table(TDC_RT_REMOVE_UNUSED) flush them with extra(HA_EXTRA_FLUSH) instead. This goes inline with regular FTWRL.   Part of MDEV-17882 - Cleanup refresh version

Vladislav Vaintroub added a comment - 2020-12-15 20:10 Was fixed by svoj in 10.5.0

Sergey Vojtovich added a comment - 2020-12-25 14:30 Well, frankly speaking I didn't complete this effort. There're still quite a few things that could've been simplified away: 1. TDC_element::flushed 2. TDC_element::all_tables_refs 3. TDC_element::m_flush_tickets Along with a few hundreds lines of relevant code. Which is practically only needed to serve one table->table->s->tdc->flush_unused(true); call inside sql_admin.cc.