Andrei Elkin wrote:
> A correction ...
>
> >>> +let $pos1_slave= query_get_value(SHOW SLAVE STATUS,
> Exec_Master_Log_Pos, 1);
> >>> +
> >>> +--echo *** the prove: the stopped slave has rolled back the current
> transaction ***
> >>> +
> >>> +--disable_query_log
> >>> +select count(*) as zero from t1i;
> >>> +eval select $pos0_master - $pos0_slave as zero;
> >>> +eval select $pos1_master > $pos0_slave as one;
> >>> +--enable_query_log
> >>> +
> >>> +start slave sql_thread;
> >>> +
> >>
> >
> >> source include/wait_for_slave_sql_to_start.inc;
> >
> > That's a possiblity, i'd rather use it.
>
> I got confused to read include/start_slave_sql in above and that made
> me to write on agreeing.
> As it's actually about to *wait*, I'd rather to refuse, because
> of the following lines.
>
> > Still the patch's way `start slave' does not make
> > the test incorrect.
> >
>
> > I mean `START SLAVE sql_thread' is fully synchronous and if it returns OK
> start_slave_threads() has wait_for_start=1 in the only invocation.
>
Yes, you're right, when start slave sql_thread returns OK, sql thread
must has started
> We'd better to remove the really confising `wait_for_slave_sql_to_start'.
>
I think maybe we can keep it, because there can be other usages of this,
for example, to issue start slave in another thread asynchronously (i.e.
by using send).
But I think we should add a Note in it to say that it is not needed to
call this after issue START SLAVE.
> cheers,
>
> Andrei
