MDEV-32559 failing spider signal_ddl_recovery_done callback should re…

…sult in spider deinit

Since 0930eb8, system table creation
needed for spider init is delayed to the signal_ddl_recovery_done
callback. Since it is part of the init, failure should result in
spider deinit.

We also remove the call to spider_init_system_tables() from
spider_db_init(), as it was removed in the commit mentioned above and
accidentally restored in a merge.

include/mysql/plugin.h

@@ -531,7 +531,13 @@ struct st_mysql_plugin
   const char *author;   /* plugin author (for I_S.PLUGINS)              */
   const char *descr;    /* general descriptive text (for I_S.PLUGINS)   */
   int license;          /* the plugin license (PLUGIN_LICENSE_XXX)      */
-  int (*init)(void *);  /* the function to invoke when plugin is loaded */
+  /*
+    The function to invoke when plugin is loaded. Plugin
+    initialisation done here should defer any ALTER TABLE queries to
+    after the ddl recovery is done, in the signal_ddl_recovery_done()
+    callback called by ha_signal_ddl_recovery_done().
+  */
+  int (*init)(void *);
   int (*deinit)(void *);/* the function to invoke when plugin is unloaded */
   unsigned int version; /* plugin version (for I_S.PLUGINS)             */
   struct st_mysql_show_var *status_vars;
@@ -555,7 +561,13 @@ struct st_maria_plugin
   const char *author;   /* plugin author (for SHOW PLUGINS)             */
   const char *descr;    /* general descriptive text (for SHOW PLUGINS ) */
   int license;          /* the plugin license (PLUGIN_LICENSE_XXX)      */
-  int (*init)(void *);  /* the function to invoke when plugin is loaded */
+  /*
+    The function to invoke when plugin is loaded. Plugin
+    initialisation done here should defer any ALTER TABLE queries to
+    after the ddl recovery is done, in the signal_ddl_recovery_done()
+    callback called by ha_signal_ddl_recovery_done().
+  */
+  int (*init)(void *);
   int (*deinit)(void *);/* the function to invoke when plugin is unloaded */
   unsigned int version; /* plugin version (for SHOW PLUGINS)            */
   struct st_mysql_show_var *status_vars;

sql/handler.cc

@@ -968,7 +968,8 @@ static my_bool signal_ddl_recovery_done(THD *, plugin_ref plugin, void *)
 {
   handlerton *hton= plugin_hton(plugin);
   if (hton->signal_ddl_recovery_done)
-    (hton->signal_ddl_recovery_done)(hton);
+	  if ((hton->signal_ddl_recovery_done)(hton))
+		  plugin_ref_to_int(plugin)->state= PLUGIN_IS_DELETED;
   return 0;
 }
 

sql/handler.h

@@ -1565,7 +1565,7 @@ struct handlerton
                        const LEX_CUSTRING *version, ulonglong create_id);
 
   /* Called for all storage handlers after ddl recovery is done */
-  void (*signal_ddl_recovery_done)(handlerton *hton);
+  int (*signal_ddl_recovery_done)(handlerton *hton);
 
    /*
      Optional clauses in the CREATE/ALTER TABLE

sql/mysqld.cc

@@ -5118,6 +5118,14 @@ static int init_server_components()
 
   tc_log= 0; // ha_initialize_handlerton() needs that
 
+	/*
+	  Plugins may not be completed because system table DDLs are only
+    run after the ddl recovery done. Therefore between the
+    plugin_init() call and the ha_signal_ddl_recovery_done() call
+    below only things related to preparation for recovery should be
+    done and nothing else, and definitely not anything assuming that
+    all plugins have been initialised.
+	*/
   if (plugin_init(&remaining_argc, remaining_argv,
                   (opt_noacl ? PLUGIN_INIT_SKIP_PLUGIN_TABLE : 0) |
                   (opt_abort ? PLUGIN_INIT_SKIP_INITIALIZATION : 0)))

storage/innobase/handler/ha_innodb.cc

@@ -2155,7 +2155,7 @@ static void drop_garbage_tables_after_restore()
   ut_d(purge_sys.resume_FTS());
 }
 
-static void innodb_ddl_recovery_done(handlerton*)
+static int innodb_ddl_recovery_done(handlerton*)
 {
   ut_ad(!ddl_recovery_done);
   ut_d(ddl_recovery_done= true);
@@ -2166,6 +2166,7 @@ static void innodb_ddl_recovery_done(handlerton*)
       drop_garbage_tables_after_restore();
     srv_init_purge_tasks();
   }
+  return 0;
 }
 
 /********************************************************************//**

storage/spider/mysql-test/spider/bugfix/r/signal_ddl_fail.result

@@ -0,0 +1,8 @@
+#
+# MDEV-32559 Move alter table statements in spider init queries to be executed in the signal_ddl_recovery_done callback
+#
+select * from mysql.plugin;
+name	dl
+#
+# end of test signal_ddl_fail
+#

storage/spider/mysql-test/spider/bugfix/t/signal_ddl_fail.opt

@@ -0,0 +1,2 @@
+--plugin-load-add=ha_spider
+--debug-dbug=d,fail_spider_ddl_recovery_done

storage/spider/mysql-test/spider/bugfix/t/signal_ddl_fail.test

@@ -0,0 +1,10 @@
+--source include/have_debug.inc
+--echo #
+--echo # MDEV-32559 Move alter table statements in spider init queries to be executed in the signal_ddl_recovery_done callback
+--echo #
+# This test tests that failure in ddl_recovery callback causes the
+# plugin to be deinitialized.
+select * from mysql.plugin;
+--echo #
+--echo # end of test signal_ddl_fail
+--echo #

storage/spider/spd_table.cc

@@ -7127,9 +7127,10 @@ bool spider_init_system_tables()
   Spider is typically loaded before ddl_recovery, but DDL statements
   cannot be executed before ddl_recovery, so we delay system table creation.
 */
-static void spider_after_ddl_recovery(handlerton *)
+static int spider_after_ddl_recovery(handlerton *)
 {
-  spider_init_system_tables();
+  DBUG_EXECUTE_IF("fail_spider_ddl_recovery_done", return 1;);
+  return spider_init_system_tables();
 }
 
 int spider_db_init(
@@ -7151,16 +7152,6 @@ int spider_db_init(
 #ifdef HTON_CAN_READ_CONNECT_STRING_IN_PARTITION
   spider_hton->flags |= HTON_CAN_READ_CONNECT_STRING_IN_PARTITION;
 #endif
-  /* spider_hton->db_type = DB_TYPE_SPIDER; */
-  /*
-  spider_hton->savepoint_offset;
-  spider_hton->savepoint_set = spider_savepoint_set;
-  spider_hton->savepoint_rollback = spider_savepoint_rollback;
-  spider_hton->savepoint_release = spider_savepoint_release;
-  spider_hton->create_cursor_read_view = spider_create_cursor_read_view;
-  spider_hton->set_cursor_read_view = spider_set_cursor_read_view;
-  spider_hton->close_cursor_read_view = spider_close_cursor_read_view;
-  */
   spider_hton->panic = spider_panic;
   spider_hton->signal_ddl_recovery_done= spider_after_ddl_recovery;
   spider_hton->close_connection = spider_close_connection;
@@ -7233,10 +7224,6 @@ int spider_db_init(
 #ifndef WITHOUT_SPIDER_BG_SEARCH
   if (pthread_attr_init(&spider_pt_attr))
     goto error_pt_attr_init;
-/*
-  if (pthread_attr_setdetachstate(&spider_pt_attr, PTHREAD_CREATE_DETACHED))
-    goto error_pt_attr_setstate;
-*/
 #endif
 
 #if MYSQL_VERSION_ID < 50500