maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Michael!

On Feb 28, Michael Widenius wrote:
> revision-id: 5ae74b4823a (mariadb-10.5.0-274-g5ae74b4823a)
> parent(s): ee73f2c6e71
> author: Michael Widenius <monty@xxxxxxxxxxx>
> committer: Michael Widenius <monty@xxxxxxxxxxx>
> 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@xxxxxxxxxxx