maria-developers team mailing list archive

[Nirbhay Choubey <nirbhay@xxxxxxxxxxx>]

Hi Serg,

Thanks for the review. My comments inline.

On Sat, Sep 12, 2015 at 7:12 AM, Sergei Golubchik <serg@xxxxxxxxxxx> 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
>