maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

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.

> 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.

> +
> +  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?

> +
> +
> +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?

> +    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

Regards,
Sergei