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