maria-developers team mailing list archive

[Michael Widenius <michael.widenius@xxxxxxxxx>]

Hi!

On Fri, Feb 28, 2020 at 4:37 PM Sergei Golubchik <serg@xxxxxxxxxxx> 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