maria-developers team mailing list archive

[Sergey Vojtovich <svoj@xxxxxxxxxxx>]

Hi Sergei,

On Thu, Nov 26, 2015 at 10:09:19AM +0100, Sergei Golubchik wrote:
> Hi, Sergey!
> 
> On Nov 25, Sergey Vojtovich wrote:
> > revision-id: 2b3bffb42cc4adbec2d91c1b9c4028374b63a51a (mariadb-10.1.8-75-g2b3bffb)
> > parent(s): 6019fee7d84e8ec7d64337ad080a04f9c106bb68
> > committer: Sergey Vojtovich
> > timestamp: 2015-11-25 18:12:19 +0400
> > message:
> > 
> > MDEV-8491 - On shutdown, report the user and the host executed that.
> 
> First, a test would be nice. I suppose you can add
> it to the main.shutdown test.
Ok.

> 
> > diff --git a/sql/mysqld.cc b/sql/mysqld.cc
> > index 0502841..0694cb0 100644
> > --- a/sql/mysqld.cc
> > +++ b/sql/mysqld.cc
> > @@ -1806,10 +1806,34 @@ static void close_server_sock()
> >  #endif /*EMBEDDED_LIBRARY*/
> >  
> >  
> > -void kill_mysql(void)
> > +static volatile char *shutdown_user;
> > +static void set_shutdown_user(THD *thd)
> > +{
> > +  char user_host_buff[MAX_USER_HOST_SIZE + 1];
> > +  char *user, *expected_shutdown_user= 0;
> > +  Security_context *sctx= thd->security_ctx;
> > +
> > +  strxnmov(user_host_buff, MAX_USER_HOST_SIZE,
> > +           sctx->priv_user ? sctx->priv_user : "", "[",
> > +           sctx->user ? sctx->user : (thd->slave_thread ? "SQL_SLAVE" : ""),
> > +           "] @ ",
> > +           sctx->host ? sctx->host : "", " [",
> > +           sctx->ip ? sctx->ip : "", "]", NullS);
> 
> this will be the fifth time this expression shows up in the source code.
> turn it into a function, perhaps? Like, an inline method of THD?
> 
> also, sometimes it's "SQL_SLAVE", sometimes it's "", sometimes it's
> "unauthenticated", etc. I hope these differences can be removed and
> there will be one method that works identically for all occasions.
> 
> Btw, you can use safe_str() helper here.
Ok.

> 
> > +
> > +  if ((user= my_strdup(user_host_buff, MYF(0))) &&
> > +      !my_atomic_casptr((void **) &shutdown_user,
> > +                        (void **) &expected_shutdown_user, user))
> > +    my_free(user);
> > +}
> 
> Interesting. Why is that?
It supposed to be safe concurrent shutdown.

> 
> > +
> > +
> > +void kill_mysql(THD *thd)
> >  {
> >    DBUG_ENTER("kill_mysql");
> >  
> > +  if (thd)
> > +    set_shutdown_user(thd);
> > +
> >  #if defined(SIGNALS_DONT_BREAK_READ) && !defined(EMBEDDED_LIBRARY)
> >    abort_loop=1;					// Break connection loops
> >    close_server_sock();				// Force accept to wake up
> > @@ -1888,7 +1912,13 @@ static void __cdecl kill_server(int sig_ptr)
> >    if (sig != 0) // 0 is not a valid signal number
> >      my_sigset(sig, SIG_IGN);                    /* purify inspected */
> >    if (sig == MYSQL_KILL_SIGNAL || sig == 0)
> > -    sql_print_information(ER_DEFAULT(ER_NORMAL_SHUTDOWN),my_progname);
> > +  {
> > +    char *user= (char *) my_atomic_loadptr((void**) &shutdown_user);
> > +    sql_print_information(ER_DEFAULT(ER_NORMAL_SHUTDOWN), my_progname,
> > +                          user ? user : "unknown");
> 
> can it, really, be "unknown" here? when?
Yes, in a few cases. E.g. killed by signal. 

> 
> > +    if (user)
> > +      my_free(user);
> > +  }
> >    else
> >      sql_print_error(ER_DEFAULT(ER_GOT_SIGNAL),my_progname,sig); /* purecov: inspected */
> >
> > diff --git a/sql/share/errmsg-utf8.txt b/sql/share/errmsg-utf8.txt
> > index 59908dc..f65d466 100644
> > --- a/sql/share/errmsg-utf8.txt
> > +++ b/sql/share/errmsg-utf8.txt
> > @@ -1735,29 +1735,29 @@ ER_WRONG_AUTO_KEY 42000 S1009
> >  ER_UNUSED_9
> >          eng "You should never see it"
> >  ER_NORMAL_SHUTDOWN  
> > -        cze "%s: normální ukončení\n"
> > -        dan "%s: Normal nedlukning\n"
> > -        nla "%s: Normaal afgesloten \n"
> > -        eng "%s: Normal shutdown\n"
> > -        est "%s: MariaDB lõpetas\n"
> > -        fre "%s: Arrêt normal du serveur\n"
> > -        ger "%s: Normal heruntergefahren\n"
> > -        greek "%s: Φυσιολογική διαδικασία shutdown\n"
> > -        hun "%s: Normal leallitas\n"
> > -        ita "%s: Shutdown normale\n"
> > -        jpn "%s: 通常シャットダウン\n"
> > -        kor "%s: 정상적인 shutdown\n"
> > -        nor "%s: Normal avslutning\n"
> > -        norwegian-ny "%s: Normal nedkopling\n"
> > -        pol "%s: Standardowe zakończenie działania\n"
> > -        por "%s: 'Shutdown' normal\n"
> > -        rum "%s: Terminare normala\n"
> > -        rus "%s: Корректная остановка\n"
> > -        serbian "%s: Normalno gašenje\n"
> > -        slo "%s: normálne ukončenie\n"
> > -        spa "%s: Apagado normal\n"
> > -        swe "%s: Normal avslutning\n"
> > -        ukr "%s: Нормальне завершення\n"
> > +        cze "%s: normální ukončení %s\n"
> > +        dan "%s: Normal nedlukning %s\n"
> > +        nla "%s: Normaal afgesloten %s\n"
> > +        eng "%s: Normal shutdown by %s\n"
> > +        est "%s: MariaDB lõpetas %s\n"
> > +        fre "%s: Arrêt normal du serveur %s\n"
> > +        ger "%s: Normal heruntergefahren %s\n"
> > +        greek "%s: Φυσιολογική διαδικασία shutdown %s\n"
> > +        hun "%s: Normal leallitas %s\n"
> > +        ita "%s: Shutdown normale %s\n"
> > +        jpn "%s: 通常シャットダウン %s\n"
> > +        kor "%s: 정상적인 shutdown %s\n"
> > +        nor "%s: Normal avslutning %s\n"
> > +        norwegian-ny "%s: Normal nedkopling %s\n"
> > +        pol "%s: Standardowe zakończenie działania %s\n"
> > +        por "%s: 'Shutdown' normal %s\n"
> > +        rum "%s: Terminare normala %s\n"
> > +        rus "%s: Корректная остановка пользователем %s\n"
> > +        serbian "%s: Normalno gašenje %s\n"
> > +        slo "%s: normálne ukončenie %s\n"
> > +        spa "%s: Apagado normal %s\n"
> > +        swe "%s: Normal avslutning %s\n"
> > +        ukr "%s: Нормальне завершення %s\n"
> 
> Here you've fixed only the english and russian messages. Others would
> look pretty weird. Normal procedure in such a case is to remove all
> other languages and only keep those that you've fixed. In this case you
> can, perhaps, fix all of them in some easy way, like
> 
>         ger "%s: Normal heruntergefahren (%s)\n"
>         ger "%s: Normal heruntergefahren / %s\n"
>         ger "%s (%s): Normal heruntergefahren\n"
>         ger "%s / %s: Normal heruntergefahren\n"
> 
> or something else along these lines. See how it'll look like in the log:
> 
> mysqld: Normal heruntergefahren foo [bar] @ localhost [127.0.0.1]
> mysqld: Normal heruntergefahren (foo [bar] @ localhost [127.0.0.1])
> mysqld: Normal heruntergefahren / foo [bar] @ localhost [127.0.0.1]
> mysqld (foo [bar] @ localhost [127.0.0.1]): Normal heruntergefahren
> mysqld / foo [bar] @ localhost [127.0.0.1]: Normal heruntergefahren
Ok.

Thanks,
Sergey