Re: bk commit into 6.0 tree (mats:1.2687) BUG#37221

[Mats Kindahl]

Andrei Elkin wrote:
> Mats, hello.
> 
>> Below is the list of changes that have just been committed into a local
>> 6.0 repository of mats.  When mats does a push these changes
>> will be propagated to the main repository and, within 24 hours after the
>> push, to the public repository.
>> For information on how to access the public repository
>> see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
>>
>> ChangeSet@stripped, 2008-06-05 11:57:19+02:00, mats@mats-laptop.(none) +6 -0
>>   Bug #37221: SET AUTOCOMMIT=1 does not commit binary log
>>   
> 
> Actually there are two issues.
> 
> 
> A  SET AUTOCOMMIT=1 does not commit a transaction started with BEGIN and
>    therefore there is nothing in binlog, either engine. This contradicts
>    to the current docs.
> 
> B  If a Falcon transaction is started with SET AUTOCOMMIT= 0 committing with
>    SET AUTOCOMMIT=1 to engine works but nothing in binlog, as you are
>    reporting.
> 
> I think both issues comprise the whole problem.

Agree.

>>   WHEN setting AUTOCOMMIT=1 after starting a transaction, the binary log
>>   did not commit the outstanding transaction. The reason was that the binary
>>   log commit function saw the values of the new settings, deciding that there
>>   were nothing to commit.
>>   
> 
> As we started discussing this issue on #rep on Thu, there is a list of
> sql statements, incl SET AUTOCOMMIT=1, that commits implicitly:
> "Statements That Cause an Implicit Commit".
> Using such opportunity as having one of the list item not complying
> with the specification, I'd suggest to tests the entire implicitly
> committing list, not merely the immediate SET AUTOCOMMIT=1.
> To motivate you in doing that, i let you know my tests already showed that
> UNLOCK TABLES does not commit either.
> 
>>   Fixed the problem by moving the implicit commit to before the thread option
>>   flags were changed, so that the binary log sees the old values of the flags
>>   instead of the values they will take after the statement.
> 
> I doubt about this idea. Please read on.

[snip]

>> +  /*
>> +    If we are not in an explicit transaction (started with a BEGIN)
>> +    and setting AUTOCOMMIT=1, then we need to commit any outstanding
>> +    transactions. If not, the binary log commit function will not
>> +    empty the transaction cache since it does not see the end of a
>> +    transaction.
>> +   */
> 
> In my opinion the A part of the problem needs addressing.
> In your view such transaction that started with BEGIN goes on
> "ignoring" SET AUTOCOMMIT=1.
> 
> I suggest for us to check with Peter Gulutzan how to proceed.
> 
> Obviously, with the change below a transaction
> 
>    begin; 
>    insert into t set a=1;
>    set autocommit=1;
> 
> won't commit with the last statement.
> But it should as the docs say.

I was questioning the rationale in SET AUTOCOMMIT=1 actually committing
an ongoing transaction since I viewed it as inconsistent.

However, after going over the original code, I agree that we should
stick with the documented behavior.

> 
>> +  if (!(thd->options & OPTION_BEGIN) &&
>> +      var->save_result.ulong_value != 0 &&
>> +      ha_commit(thd))
>> +    return 1;
>> +
>>    if (var->save_result.ulong_value != 0)
>>      thd->options&= ~((sys_var_thd_bit*) var->var)->bit_flag;
>>    else
>> @@ -2983,8 +2995,6 @@ static bool set_option_autocommit(THD *t
>>        thd->options&= ~(ulonglong) (OPTION_BEGIN | OPTION_KEEP_LOG);
>>        thd->transaction.all.modified_non_trans_table= FALSE;
>>        thd->server_status|= SERVER_STATUS_AUTOCOMMIT;
>> -      if (ha_commit(thd))
>> -	return 1;
> 
> Let's clear air regarding to A.

Done. :)

Best wishes,
Mats Kindahl

-- 
Mats Kindahl
Lead Software Developer
Replication Team
MySQL AB, www.mysql.com