Re: [Maria-developers] 5ae74b4823a: mysqld --help will now load mysqld.options table
Hi, Michael! On Feb 28, Michael Widenius wrote:
revision-id: 5ae74b4823a (mariadb-10.5.0-274-g5ae74b4823a) parent(s): ee73f2c6e71 author: Michael Widenius <monty@mariadb.com> committer: Michael Widenius <monty@mariadb.com> timestamp: 2020-02-26 16:05:54 +0200 message:
mysqld --help will now load mysqld.options table
mysql.plugin
Changes: - Initalize Aria early to allow it to load mysql.plugin table with --help - Don't print 'aborting' when doing --help - Don't write 'loose' error messages on log_warning < 2 (2 is default) - Don't write warnings about disabled plugings when doing --help - Don't write aria_log_control or aria log files when doing --help - When using --help, open all Aria tables in read only mode (safety) - If aria_init() fails, do a cleanup(). (Frees used memory) - If aria_log_control is locked with --help, then don't wait 30 seconds but instead return at once without initialzing Aria plugin.
diff --git a/sql/mysqld.cc b/sql/mysqld.cc index b2f8afca7a6..415a12f4783 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -8511,8 +8511,8 @@ static void option_error_reporter(enum loglevel level, const char *format, ...) va_start(args, format);
/* Don't print warnings for --loose options during bootstrap */ - if (level == ERROR_LEVEL || !opt_bootstrap || - global_system_variables.log_warnings) + if (level == ERROR_LEVEL || + (!opt_bootstrap && global_system_variables.log_warnings > 1))
You've completely suppressed all --loose warnings during bootstrap. Before your patch they were basically always enabled (because of log_warnings==2). I am not sure it's a good idea to disable warnings completely in bootstrap. If fact, I don't see why bootstrap should be special, so I'd simply remove !opt_bootstrap condition completely here. But if you want to keep it you can do something like global_system_variables > opt_bootstrap it'll make bootstrap a bit quieter than normal startup.
{ vprint_msg_to_log(level, format, args); } diff --git a/sql/sql_plugin.cc b/sql/sql_plugin.cc index d7d7fcca4a2..31de259a218 100644 --- a/sql/sql_plugin.cc +++ b/sql/sql_plugin.cc @@ -1679,7 +1680,22 @@ int plugin_init(int *argc, char **argv, int flags) global_system_variables.table_plugin = intern_plugin_lock(NULL, plugin_int_to_ref(plugin_ptr)); DBUG_SLOW_ASSERT(plugin_ptr->ref_count == 1); + } + /* Initialize Aria plugin so that we can load mysql.plugin */ + plugin_ptr= plugin_find_internal(&Aria, MYSQL_STORAGE_ENGINE_PLUGIN); + DBUG_ASSERT(plugin_ptr || !mysql_mandatory_plugins[0]); + if (plugin_ptr) + { + DBUG_ASSERT(plugin_ptr->load_option == PLUGIN_FORCE);
+ if (plugin_initialize(&tmp_root, plugin_ptr, argc, argv, false)) + { + if (!opt_help) + goto err_unlock; + plugin_ptr->state= PLUGIN_IS_DISABLED; + } + else + aria_loaded= 1; } mysql_mutex_unlock(&LOCK_plugin);
I think this should be done differently. In a completely opposite way. I had unfurtunately hard-coded MyISAM here. A proper fix here could be to remove this special treatment of MyISAM instead of adding another special treatment of Aria. Then plugin_init() could work like: * run dd_frm_type() for the mysql.plugin table - like it's done now * instead of hard-coding MyISAM (and Aria), find this engine name in the plugin_array[] (note, all builtin plugins are already there) * initialize it and (if successful) load mysql.plugin table It only concerns the sql_plugin.cc part of your commit. Your aria part of the commit is still needed, because a good-behaving engine has to be read-only in --help. Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
Hi! On Fri, Feb 28, 2020 at 4:37 PM Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Michael!
--- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -8511,8 +8511,8 @@ static void option_error_reporter(enum loglevel level, const char *format, ...) va_start(args, format);
/* Don't print warnings for --loose options during bootstrap */ - if (level == ERROR_LEVEL || !opt_bootstrap || - global_system_variables.log_warnings) + if (level == ERROR_LEVEL || + (!opt_bootstrap && global_system_variables.log_warnings > 1))
You've completely suppressed all --loose warnings during bootstrap. Before your patch they were basically always enabled (because of log_warnings==2).
Yes, and log_warnings == 2 is still default, so normal users will still get loose warnings, except if they put log_warnings explicitly to 1 to make startup more silent.
I am not sure it's a good idea to disable warnings completely in bootstrap.
If fact, I don't see why bootstrap should be special, so I'd simply remove !opt_bootstrap condition completely here. But if you want to keep it you can do something like
The change was not for bootstrap. I just keep it there. However I don't it's important to have loose warnings during bootstrap, as this is something that we only do during test or upgrades.
diff --git a/sql/sql_plugin.cc b/sql/sql_plugin.cc index d7d7fcca4a2..31de259a218 100644 --- a/sql/sql_plugin.cc +++ b/sql/sql_plugin.cc @@ -1679,7 +1680,22 @@ int plugin_init(int *argc, char **argv, int flags) global_system_variables.table_plugin = intern_plugin_lock(NULL, plugin_int_to_ref(plugin_ptr)); DBUG_SLOW_ASSERT(plugin_ptr->ref_count == 1); + } + /* Initialize Aria plugin so that we can load mysql.plugin */ + plugin_ptr= plugin_find_internal(&Aria, MYSQL_STORAGE_ENGINE_PLUGIN); + DBUG_ASSERT(plugin_ptr || !mysql_mandatory_plugins[0]); + if (plugin_ptr) + { + DBUG_ASSERT(plugin_ptr->load_option == PLUGIN_FORCE);
+ if (plugin_initialize(&tmp_root, plugin_ptr, argc, argv, false)) + { + if (!opt_help) + goto err_unlock; + plugin_ptr->state= PLUGIN_IS_DISABLED; + } + else + aria_loaded= 1; } mysql_mutex_unlock(&LOCK_plugin);
I think this should be done differently. In a completely opposite way.
I had unfurtunately hard-coded MyISAM here. A proper fix here could be to remove this special treatment of MyISAM instead of adding another special treatment of Aria.
Then plugin_init() could work like:
* run dd_frm_type() for the mysql.plugin table - like it's done now * instead of hard-coding MyISAM (and Aria), find this engine name in the plugin_array[] (note, all builtin plugins are already there) * initialize it and (if successful) load mysql.plugin table
It only concerns the sql_plugin.cc part of your commit. Your aria part of the commit is still needed, because a good-behaving engine has to be read-only in --help.
I would suggest that we keep things like above for now and then you can hack it when you are enough annoyed about this ;) Regards, Monty
participants (2)
-
Michael Widenius
-
Sergei Golubchik