maria-developers team mailing list archive
Mailing list archive
Re: slave_ddl_exec_mode and incompatible change in MariaDB 10.0.8
>>>>> "Pavel" == Pavel Ivanov <pivanof@xxxxxxxxxx> writes:
Pavel> On Wed, Feb 26, 2014 at 8:12 PM, Michael Widenius <monty@xxxxxxxxxxxx> wrote:
Pavel> And then it said that slave died with the stack trace
Pavel> sql/transaction.cc:139(trans_begin(THD*, unsigned int))[0x788e20]
Pavel> sql/slave.cc:3191(apply_event_and_update_pos(Log_event*, THD*,
Pavel> rpl_group_info*, rpl_parallel_thread*))[0x5c0da8]
Pavel> Which means that slave tries to execute BEGIN event while OPTION_BEGIN
Pavel> is set which shouldn't ever happen.
>> The assert you put in the code doesn't show that anything is wrong.
>> The reason is the following code in log_event.cc:
thd-> variables.option_bits|= OPTION_BEGIN | OPTION_GTID_BEGIN;
>> DBUG_PRINT("info", ("Set OPTION_GTID_BEGIN"));
>> trans_begin(thd, 0);
>> In other words, we do set OPTION_BEGIN just before calling
>> trans_begin(), so the assert is wrong.
Pavel> To me this code looks clearly wrong. You set OPTION_BEGIN just before
Pavel> calling trans_begin() which forces trans_begin() to kick off the
Pavel> commit machinery. And even though it ends up doing nothing, I don't
Pavel> know how trivial is the number of CPU cycles it spends on that. But
Pavel> why set OPTION_BEGIN if it will be set in the trans_begin() anyway?
The reason was simply that I wanted almost all lines that was using
OPTION_GTID_BEGIN to set and reset OPTION_BEGIN. This was mainly to
make it trivial to ensure with 'grep' that I did not miss any cases.
What I had missed was that trans_begin() did check for OPTION_BEGIN
and when this was set did a few extra unnecessary tests to commit
non-existing things. I agree that it should not be set here.
Pavel> So I fixed that and made the line to look like
thd-> variables.option_bits|= OPTION_GTID_BEGIN;
Pavel> But testing that more and looking at the code I realized there's
Pavel> another problem in these 3 lines: why did you add the call to
Pavel> trans_begin() at all? Right after it mysql_parse() is called to
Pavel> execute "BEGIN" statement, which again kicks off the commit machinery
Pavel> without any necessity to do that.
The original reason was to not have to call mysql_parse() at all for
this case (no reason to parse something that we know what it is).
Apparently I missed to remove the extra calls. I will fix that now.
Thanks a lot for noticing this.
Pavel> So FYI: I removed setting OPTION_BEGIN here and removed the call to
Pavel> trans_begin() and all tests passed (including my additional assert).
Thanks for testing this!