Hi Sergei, On Tue, Jul 22, 2014 at 10:56:31AM +0200, Sergei Golubchik wrote:
Hi, Sergey!
On Jul 07, svoj@mariadb.org wrote:
revision-id: 00ed36700addfbce91940223cce4ccd6ee5823a1 parent(s): f37f19a6c790100cc2ae5f483ba4db34a517f0ae committer: Sergey Vojtovich branch nick: 10.1 timestamp: 2014-07-07 14:32:13 +0400 message:
MDEV-4262 - P_S discovery
Discover P_S tables automatically.
Most of this patch is code clean-up: - removed tests and code responsible for P_S tables correctness verification - always return error from ha_perfschema::create() - install/upgrade scripts won't create P_S tables anymore
Great work, thanks! Thanks!
The patch is pretty much "ok to push", but, first I want to understand why you deleted table_file_summary.cc file.
See other minor comments below.
diff --git a/mysql-test/suite/perfschema/include/upgrade_check.inc b/mysql-test/suite/perfschema/include/upgrade_check.inc index 52d4cfd..1532de8 100644 --- a/mysql-test/suite/perfschema/include/upgrade_check.inc +++ b/mysql-test/suite/perfschema/include/upgrade_check.inc @@ -3,7 +3,6 @@ #
--source include/count_sessions.inc ---error 1 --exec $MYSQL_UPGRADE --skip-verbose --force > $out_file 2> $err_file --source include/wait_until_count_sessions.inc
The rest of this upgrade_check.inc file does (according to the comment) "Verify that mysql_upgrade complained about the performance_schema" perhaps you should change the comment to say "does not complain" ?
Right, I'll fix it.
diff --git a/scripts/mysql_performance_tables.sql b/scripts/mysql_performance_tables.sql index d495578..3061603 100644 --- a/scripts/mysql_performance_tables.sql +++ b/scripts/mysql_performance_tables.sql @@ -53,1427 +53,3 @@ SET @str = IF(@broken_pfs = 0, @cmd, 'SET @dummy = 0'); PREPARE stmt FROM @str; EXECUTE stmt; DROP PREPARE stmt;
Why did you keep the above? I'd think that a simple
DROP DATABASE IF EXISTS performance_schema
should be enough to replace the whole file. And it could be moved back to mysql_system_tables.sql, so this file could be completely removed.
I thought we can't discover database name and it needs to be created manually. But if you say it is possible: I'll try it. There are also additional checks that would prevent dropping P_S database if there are non-PFS tables/views/events/routines. Should we abandon these checks too?
- -# -# From this point, only create the performance schema tables -# if the server is built with performance schema -# - -set @have_pfs= (select count(engine) from information_schema.engines where engine='PERFORMANCE_SCHEMA' and support != 'NO'); - -# -# TABLE COND_INSTANCES -# ... diff --git a/storage/perfschema/pfs_engine_table.cc b/storage/perfschema/pfs_engine_table.cc index 958a2bd..6f0a363 100644 --- a/storage/perfschema/pfs_engine_table.cc +++ b/storage/perfschema/pfs_engine_table.cc @@ -1440,5 +1319,17 @@ end: DBUG_RETURN(false); }
+int pfs_discover_table_names(handlerton *hton, LEX_STRING *db, + MY_DIR *dir,
hm, no "unused argument" compiler warnings? okay...
Strange indeed. It was compiled like: cmake -DCMAKE_BUILD_TYPE=Debug -DMYSQL_MAINTAINER_MODE=ON && make It adds -Wno-unused-parameter gcc flag. Same flag seem to be added in release builds. Did we decide not to hunt for unused arguments?
+ handlerton::discovered_list *result) +{ + if (compare_table_names(db->str, PERFORMANCE_SCHEMA_str.str)) + return 0; + for (size_t i= 0; i < array_elements(all_shares) - 1; i++) + result->add_table(all_shares[i]->m_name.str, + all_shares[i]->m_name.length); + return 0; +} + /** @} */
diff --git a/storage/perfschema/table_file_summary.cc b/storage/perfschema/table_file_summary.cc deleted file mode 100644 index 104fa0f..0000000 --- a/storage/perfschema/table_file_summary.cc +++ /dev/null @@ -1,372 +0,0 @@
Why did you remove this file?
It had file_summary_by_event_name and file_summary_by_instance which were moved to table_file_summary_by_event_name.cc and table_file_summary_by_instance.cc. Otherwise it wasn't even listed in CMakeLists.txt. Thanks, Sergey