Re: BUG#29288

[Andrei Elkin]

Sven, hello.



> Hi Andrei,
>
> Andrei Elkin wrote:
>> Basically, my remained concerns wrt to your fixes are:
>>
>> 1. considering CREATE..SELECT on a session that requested
>>    set AUTOCOMMIT=OFF and binlog_format=ROW;
>>
>>    In such case there is no BEGIN after CREATE table in binlog.
>>    This makes injection of set autocommit=ON, which your fixes
>>    enforce, at least not looking correctly
>
> I think this works incorrectly without my fixes, and, I believe,
> slightly more correct with my fixes. The problem is that
> CREATE...SELECT is split into two statements instead of one, where the
> first one (CREATE) does an implicit commit before and after
> execution. Assume the master runs with autocommit off and executes
> CREATE...SELECT. Then
>  - Master will do implicit commit, then create the table, then
> populate the table, the do implicit commit.
>  - Slave without my patch will do implicit commit, create table, do
> implicit commit, populate table.
>  - Slave with my patch will do implicit commit, create table, do
> implicit commit, populate table, and autocommit.
>

I think that's indeed an inessential detail.


> Although the slave's behavior is not correct in any case (because it
> does an implicit commit after create table), I think it is more
> correct with my patch than without my patch, since it commits after
> the statement, like the master does.
>
> Please let me know if I misunderstood something, but this is how the
> design is described in BUG#22864.
>
> I will reply to issue #2 in a separate email, or we can discuss it on Skype.

We agreed for you to remove those comment.

The patch is good to approval.

Thanks for your patience and analysis!

cheers,

Andrei


>
> Cheers,
>
> Sven
>
>
>> 2.  There has been a concern that i mailed about correctness of the
>>    comments.
>>
>> +
>> +    Note: we must explicitly replicate AUTOCOMMIT=1 from master, not
>> +    assume AUTOCOMMIT=1 on slave.  If slave assumed AUTOCOMMIT=1, it
>> +    would not work in upgrade scenarios with an old master (not
>> +    replicating explicit BEGIN/END) and a new slave.  Also, when
>> +    replicating from a new master to an old slave, the old slave will
>> +    use the AUTOCOMMIT mode given by the master.  (General rule:
>> +    everything should be explicitly written in the binlog.)
>>
>> From my another mail:
>>
>>
>>    As I can see there is no such issue, and vice versa the slave would
>>    be rather to assume to avoid this bug's scenarios (please find
>>    below more to that) in replication from the pre-bug29288 master to
>>    the post-bug29288 slave.  Conversely, the assumption seems to be as
>>    the only way to facilitate upgrading.
>>
>>    Let's consider use cases of replicating from a master to the assuming
>>    slave.
>>
>>    In ordinary case such as a master replicates an autocommit query or
>>    a multi-statement transaction (msta) that is not
>>    interrupted/complicated with the CREATE..SELECT statement there is
>>    no issue with either pre-bug#26395 or post-bug#26395 master.
>>    Indeed, the former does not wrap the query but the slave's
>>    assumtion would force to autocommit. For the msta, there are
>>    explict BEGIN/COMMIT in the binlog and thus the bit value does not
>>    matter.
>>
>>> On yesterday's scrum, we decided to let Jason review BUG#29288 instead
>>> of you. I just would like to stress that despite changing reviewer, I
>>> appreciate your previous efforts and sincerely hope there will be no
>>> hard feelings regarding this bug. The only reason we change to Jason
>>> is to get it done despite you are on vacation. If Jason approves it
>>> and it gets pushed before you can say your opinion, feel free to bring
>>> up the discussion again. If we find the patch erroneous, I will of
>>> course revert it.
>>>
>>> Now, enjoy your vacation :-)
>>>
>>
>> Thank you!
>>
>> I still think, the best way to process would be to let us to have a
>> chat on Monday when I will come back.
>>
>> cheers,
>>
>> Andrei
>
> -- 
> Sven Sandberg, Software Engineer
> MySQL AB, www.mysql.com