Hi Serg, Thanks for the review. My comments inline. On Sat, Sep 12, 2015 at 7:12 AM, Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Nirbhay!
On Aug 19, Nirbhay Choubey wrote:
revision-id: 8bf23efc914f18c687067e9314cadf4f36a85eec parent(s): 71d1f35847a575239deff856590bf6f13afd74ed committer: Nirbhay Choubey branch nick: 5.5-galera.b8208 timestamp: 2015-08-19 17:17:56 -0400 message:
MDEV-8208: Sporadic SEGFAULT on startup
A couple of questions:
diff --git a/sql/mysqld.cc b/sql/mysqld.cc index 2008fc5..4c05999 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -4584,6 +4579,29 @@ static int init_server_components() } plugins_are_initialized= TRUE; /* Don't separate from init function */
+#ifdef WITH_WSREP + /* Wait for wsrep threads to get created. */ + if (wsrep_creating_startup_threads == 1) { + mysql_mutex_lock(&LOCK_thread_count); + while (wsrep_running_threads < 2) + { + mysql_cond_wait(&COND_thread_count, &LOCK_thread_count); + } + + /* Now is the time to initialize threads for queries. */ + THD *tmp; + I_List_iterator<THD> it(threads); + while ((tmp= it++)) + if (tmp->wsrep_applier == true) + tmp->init_for_queries();
Do you really need to iterate over all THD's? There is only one applier thread, right? And it's THD is not stored anywhere, only in the global list?
2 actually : applier and replayer.
+ mysql_mutex_unlock(&LOCK_thread_count); + } +#endif + have_csv= plugin_status(STRING_WITH_LEN("csv"), MYSQL_STORAGE_ENGINE_PLUGIN); have_ndbcluster= plugin_status(STRING_WITH_LEN("ndbcluster"), @@ -4890,12 +4902,21 @@ pthread_handler_t start_wsrep_THD(void *arg) //thd->version= refresh_version; thd->proc_info= 0; thd->command= COM_SLEEP; - thd->set_time(); - thd->init_for_queries(); + + if (plugins_are_initialized) + { + thd->init_for_queries(); + }
What happens if plugins are not initialized yet?
In that case we do init_for_queries() only after plugins gets initialized. BTW, this can only happen for the initial 2 wsrep thds. As they are created before the initialization of plugins. For other wsrep threads, requested using wsrep_slave_threads, they get created only after plugins are initialized.
thd->init_for_queries() will be done in that THD loop above? In that case why do you need to do it here?
For wsrep threads requested via wsrep_slave_threads. The clarify the issue further, here is roughly what happens on the server start : ( http://pastie.org/10362496 -> In case the below diagram gets mangled by the mail client) main() | |----- wsrep_init_startup() \ | \ | | | | |- pthread_create(start_wsrep_THD()) <-- applier | |- pthread_create(start_wsrep_THD()) <-- rollbacker | | <-- main thread : wait (COND_wsrep_sst) / / <-- applier thread: signal(COND_wsrep_sst) / | (while the applier/rollbacker threads get initialized, applier later signals COND_wsrep_sst | post SST and main thread of execution continues. Now, the segfault can occur when | the rollbacker thread is still under thd->init_for_queries trying to access maria_thon | while its only partially initialized (e.g. maria_hton != 0 but its other members like slot | are not initialized) under plugin_init(). | |- main thread: plugin_init() || rollbacker thd : THD::init_for_queries() -> ha_maria::implicit_commit Best, Nirbhay
Regards, Sergei