James' World

At the OSCON 2011 MariaDB Birds-of-a-Feather (BoF) session, I suggested adding a MySQL SHUTDOWN statement to Monty, which was written up as WL#232. Other databases have this feature, and it’s very handy when automating management of a cluster of MySQL servers.

And at the Percona Live MySQL Conference 2013, Monty suggested to MariaDB BOF attendees that a good way to get a new feature added is to to write a patch to pave the way for a committer to start with.

Phase 1

So … I sat down last nite and wrote the patch against MariaDB 5.5.30.

Basically it meant telling mysql’s lex/yacc files to parse “shutdown”, then calling the existing MySQL API shutdown kill_mysql() function.

This code is released under the Open Source BSD-new License, according to the MariaDB Contributor Agreement.

shutdown_0.1.patch.txt – MariaDB 5.5.30:

--- sql_parse.cc	2013-03-11 03:29:13.000000000 -0700
+++ /home/james/mariadb-5.5.30-new/sql/sql_parse.cc	2013-05-15 13:17:05.000000000 -0700
@@ -1305,7 +1305,6 @@
     my_ok(thd);
     break;
   }
-#ifndef EMBEDDED_LIBRARY
   case COM_SHUTDOWN:
   {
     status_var_increment(thd->status_var.com_other);
@@ -1333,7 +1332,6 @@
     error=TRUE;
     break;
   }
-#endif
   case COM_STATISTICS:
   {
     STATUS_VAR *current_global_status_var;      // Big; Don't allocate on stack
@@ -3736,6 +3734,31 @@
                     lex->kill_signal);
     break;
   }
+  case SQLCOM_SHUTDOWN:
+  {
+    // jeb - This code block is copied from COM_SHUTDOWN above. Since kill_mysql(void) {} doesn't take a level argument, the level code is pointless.
+    // jeb - In fact, the level code should be removed and Oracle Database statements implemented: SHUTDOWN, SHUTDOWN IMMEDIATE and SHUTDOWN ABORT. See WL#232.
+
+    status_var_increment(thd->status_var.com_other);
+    if (check_global_access(thd,SHUTDOWN_ACL))
+      break; /* purecov: inspected */
+
+    enum mysql_enum_shutdown_level level;
+    level= SHUTDOWN_DEFAULT;
+    if (level == SHUTDOWN_DEFAULT)
+      level= SHUTDOWN_WAIT_ALL_BUFFERS; // soon default will be configurable
+    else if (level != SHUTDOWN_WAIT_ALL_BUFFERS)
+    {
+      my_error(ER_NOT_SUPPORTED_YET, MYF(0), "this shutdown level");
+      break;
+    }
+    DBUG_PRINT("SQLCOM_SHUTDOWN",("Got shutdown command for level %u", level));
+    my_eof(thd);
+    kill_mysql();
+    res=TRUE;
+    break;
+  }
+
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
   case SQLCOM_SHOW_GRANTS:
   {
--- sql_yacc.yy	2013-03-11 03:29:19.000000000 -0700
+++ /home/james/mariadb-5.5.30-new/sql/sql_yacc.yy	2013-05-15 11:12:03.000000000 -0700
@@ -791,7 +791,7 @@
   Currently there are 174 shift/reduce conflicts.
   We should not introduce new conflicts any more.
 */
-%expect 174
+%expect 196
 
 /*
    Comments for TOKENS.
@@ -1645,6 +1645,7 @@
         definer_opt no_definer definer
         parse_vcol_expr vcol_opt_specifier vcol_opt_attribute
         vcol_opt_attribute_list vcol_attribute
+        shutdown
 END_OF_INPUT
 
 %type  call sp_proc_stmts sp_proc_stmts1 sp_proc_stmt
@@ -1796,6 +1797,7 @@
         | savepoint
         | select
         | set
+        | shutdown
         | signal_stmt
         | show
         | slave
@@ -13715,6 +13717,17 @@
         ;
 
 
+shutdown:
+          SHUTDOWN
+          {
+            LEX *lex=Lex;
+            lex->value_list.empty();
+            lex->users_list.empty();
+            lex->sql_command= SQLCOM_SHUTDOWN;
+          }
+        ;
+
+
 set_expr_or_default:
           expr { $$=$1; }
         | DEFAULT { $$=0; }
--- sql_prepare.cc	2013-03-11 03:29:11.000000000 -0700
+++ /home/james/mariadb-5.5.30-new/sql/sql_prepare.cc	2013-05-15 03:07:00.000000000 -0700
@@ -2173,6 +2173,7 @@
   case SQLCOM_GRANT:
   case SQLCOM_REVOKE:
   case SQLCOM_KILL:
+  case SQLCOM_SHUTDOWN:
     break;
 
   case SQLCOM_PREPARE:
--- mysqld.cc	2013-03-11 03:29:14.000000000 -0700
+++ /home/james/mariadb-5.5.30-new/sql/mysqld.cc	2013-05-15 01:20:11.000000000 -0700
@@ -3333,6 +3333,7 @@
   {"savepoint",            (char*) offsetof(STATUS_VAR, com_stat[(uint) SQLCOM_SAVEPOINT]), SHOW_LONG_STATUS},
   {"select",               (char*) offsetof(STATUS_VAR, com_stat[(uint) SQLCOM_SELECT]), SHOW_LONG_STATUS},
   {"set_option",           (char*) offsetof(STATUS_VAR, com_stat[(uint) SQLCOM_SET_OPTION]), SHOW_LONG_STATUS},
+  {"shutdown",             (char*) offsetof(STATUS_VAR, com_stat[(uint) SQLCOM_SHUTDOWN]), SHOW_LONG_STATUS},
   {"signal",               (char*) offsetof(STATUS_VAR, com_stat[(uint) SQLCOM_SIGNAL]), SHOW_LONG_STATUS},
   {"show_authors",         (char*) offsetof(STATUS_VAR, com_stat[(uint) SQLCOM_SHOW_AUTHORS]), SHOW_LONG_STATUS},
   {"show_binlog_events",   (char*) offsetof(STATUS_VAR, com_stat[(uint) SQLCOM_SHOW_BINLOG_EVENTS]), SHOW_LONG_STATUS},
--- sql_lex.h	2013-03-11 03:29:13.000000000 -0700
+++ /home/james/mariadb-5.5.30-new/sql/sql_lex.h	2013-05-15 01:19:17.000000000 -0700
@@ -193,6 +193,7 @@
   SQLCOM_SHOW_RELAYLOG_EVENTS, 
   SQLCOM_SHOW_USER_STATS, SQLCOM_SHOW_TABLE_STATS, SQLCOM_SHOW_INDEX_STATS,
   SQLCOM_SHOW_CLIENT_STATS,
+  SQLCOM_SHUTDOWN,
 
   /*
     When a command is added here, be sure it's also added in mysqld.cc

To apply:

tar zxvf - < mariadb-5.5.30.tar.gz
cd mariadb-5.5.30/sql
wget http://jebriggs.com/php/shutdown_0.1.patch.txt
patch -b < shutdown_0.1.patch.txt

make.sh:

#!/bin/bash

cd mariadb-5.5.30
cmake . -DCMAKE_INSTALL_PREFIX:PATH=/usr/local/mariadb-5.5.30
make --with-debug
sudo make install

start.sh:

#!/bin/bash

killall mysqld
/usr/local/mariadb-5.5.30/bin/mysqld_safe --user=mysql --debug &
tail -f  /tmp/mysqld.trace | grep Got &
mysql -u root -p

mysql client (with mysqld.log and mysql.trace entries overlaid):

mysql> shutdown;
ERROR 2013 (HY000): Lost connection to MySQL server during query
mysql> 130515 13:20:38 mysqld_safe mysqld from pid file /var/run/mysqld/mysqld.pid ended

/tmp/mysql.trace:

T@4    : | | | >parse_sql
T@4    : | | | <parse_sql
T@4    : | | | >LEX::set_trg_event_type_for_tables
T@4    : | | | <LEX::set_trg_event_type_for_tables
T@4    : | | | >mysql_execute_command
T@4    : | | | | >deny_updates_if_read_only_option
T@4    : | | | | <deny_updates_if_read_only_option
T@4    : | | | | >stmt_causes_implicit_commit
T@4    : | | | | <stmt_causes_implicit_commit
T@4    : | | | | SQLCOM_SHUTDOWN: Got shutdown command for level 16
T@4    : | | | | >set_eof_status
T@4    : | | | | <set_eof_status
T@4    : | | | | >kill_mysql
T@4    : | | | | | quit: After pthread_kill
T@4    : | | | | <kill_mysql
T@4    : | | | | proc_info: /home/james/mariadb-5.5.30/sql/sql_parse.cc:4507  query end

/var/log/mysqld.log:

130515 13:20:08 mysqld_safe Starting mysqld daemon with databases from /var/lib/mysql
130515 13:20:08 InnoDB: !!!!!!!! UNIV_DEBUG switched on !!!!!!!!!
130515 13:20:08 InnoDB: The InnoDB memory heap is disabled
130515 13:20:08 InnoDB: Mutexes and rw_locks use GCC atomic builtins
130515 13:20:08 InnoDB: Compressed tables use zlib 1.2.3
130515 13:20:08 InnoDB: Initializing buffer pool, size = 128.0M
130515 13:20:08 InnoDB: Completed initialization of buffer pool
130515 13:20:08 InnoDB: highest supported file format is Barracuda.
130515 13:20:09  InnoDB: Waiting for the background threads to start
130515 13:20:10 Percona XtraDB (http://www.percona.com) 5.5.30-MariaDB-30.1 started; log sequence number 1597945
130515 13:20:10 [Note] Plugin 'FEEDBACK' is disabled.
130515 13:20:10 [Note] Event Scheduler: Loaded 0 events
130515 13:20:10 [Note] /usr/local/mariadb-5.5.30/bin/mysqld: ready for connections.
Version: '5.5.30-MariaDB-debug'  socket: '/var/lib/mysql/mysql.sock'  port: 3306  Source distribution
130515 13:20:37 [Note] Got signal 15 to shutdown mysqld
130515 13:20:37 [Note] /usr/local/mariadb-5.5.30/bin/mysqld: Normal shutdown

130515 13:20:37 [Note] Event Scheduler: Purging the queue. 0 events
130515 13:20:37  InnoDB: Starting shutdown...
130515 13:20:38  InnoDB: Shutdown completed; log sequence number 1597945
130515 13:20:38 [Note] /usr/local/mariadb-5.5.30/bin/mysqld: Shutdown complete

130515 13:20:38 mysqld_safe mysqld from pid file /var/run/mysqld/mysqld.pid ended

A possible test would be like this, but it would interfere with operation of the test mysqld instance:

mysql-test/t/shutdown.test:

shutdown;

Phase 2

My above patch applies cleanly within the existing MySQL shutdown framework, which implements a feature like Oracle Database's SHUTDOWN IMMEDIATE command.

However, my patch is a Pyrrhic victory, since there's so much wrong with MySQL's existing shutdown framework that it will take an internals committer to sort it out.

The shutdown framework is badly designed, if it was designed at all, since it fails the "does this feel programmed on purpose?" test, and in fact doesn't work reliably:

1. Conceptually, there should be 3 Oracle Database-style SHUTDOWN options: WAIT, IMMEDIATE and ABORT. Implementing SHUTDOWN WAIT would mean intrusive changes to the MySQL source code, while SHUTDOWN ABORT would be easier to program, but at the risk of data integrity.
2. the following bug reports describe a race condition between mysqld threads and the shutdown thread:
   • Bug #56666: Race condition between the server main thread and the kill server thread
   • Bug #29650: Failure of MySQL to shutdown
   • Bug #56324: Race Condition while shutting down MySQL: "PSI_server"
   • Bug #58707: Valgrind warning in PFS

I guess I'll have to pay myself the worklog bounty of $100.

This is actually my second MySQL patch contribution. In 1997 or 1998 I submitted a patch for the installer, which was one of the most troublesome components at that time. Monty rewrote it, but I liked my version better.

Update: Sergei Golubchik committed this patch to MariaDB 10.0.4 on 2013-06-25. Thanks, Sergei!

shutdown_0.1.patch.txt
MySQL's Missing Shutdown Statement
WL#232
Bug #63276: skip sleep in srv_master_thread when shutdown is in progress