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