Andrei Elkin wrote:
> Mats,
>
> I wonder if this is the most recent commit ...
> But that's the lattest that I found in commits@.
>
>> #At bzr+ssh://bk-internal.mysql.com/bzrroot/server/mysql-6.0-falcon/
>>
>> 2695 Mats Kindahl 2008-06-09
>> Bug #37221: SET AUTOCOMMIT=1 does not commit binary log
>>
>> 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.
>>
>
> The problem description left unattended although you agreed on the last review mail:
>
> > 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.
Ehw... the problem description stays the same because this is what it
was about. The problem A is is not part of this patch, the engine
committed properly before I applied this patch, it is just problem B,
i.e. that the call of binlog_commit() sees the new values of the options
instead of the old values, and therefore make the wrong decision of not
committing the transaction *to the binary log*.
Look at this trace from BEFORE the patch:
master1> create table t1 (a int) engine=falcon;
Query OK, 0 rows affected (0.17 sec)
master1> show create table t1;
+-------+---------------------------------------------------------------------------------------+
| Table | Create Table
|
+-------+---------------------------------------------------------------------------------------+
| t1 | CREATE TABLE `t1` (
`a` int(11) DEFAULT NULL
) ENGINE=Falcon DEFAULT CHARSET=latin1 |
+-------+---------------------------------------------------------------------------------------+
1 row in set (0.00 sec)
master1> set autocommit = 0;
Query OK, 0 rows affected (0.01 sec)
master1> begin;
Query OK, 0 rows affected (0.00 sec)
master1> insert into t1 values (2);
Query OK, 1 row affected (0.01 sec)
master1> set autocommit = 1;
Query OK, 0 rows affected (0.00 sec)
master2> select * from t1;
+------+
| a |
+------+
| 2 |
+------+
1 row in set (0.00 sec)
master2> show binlog events;
+-------------------+-----+-------------+-----------+-------------+---------------------------------------------------+
| Log_name | Pos | Event_type | Server_id | End_log_pos | Info
|
+-------------------+-----+-------------+-----------+-------------+---------------------------------------------------+
| master-bin.000001 | 4 | Format_desc | 1 | 107 |
Server ver: 6.0.6-alpha-debug-log, Binlog ver: 4 |
| master-bin.000001 | 107 | Query | 1 | 207 | use
`test`; create table t1 (a int) engine=falcon |
+-------------------+-----+-------------+-----------+-------------+---------------------------------------------------+
2 rows in set (0.00 sec)
And here is the same set of commands from AFTER the patch:
master1> create table t1 (a int) engine=falcon;
Query OK, 0 rows affected (0.14 sec)
master1> set autocommit = 0;
Query OK, 0 rows affected (0.00 sec)
master1> begin;
Query OK, 0 rows affected (0.00 sec)
master1> insert into t1 values (2);
Query OK, 1 row affected (0.00 sec)
master1> set autocommit = 1;
Query OK, 0 rows affected (0.04 sec)
master2> show binlog events;
+-------------------+-----+-------------+-----------+-------------+---------------------------------------------------+
| Log_name | Pos | Event_type | Server_id | End_log_pos | Info
|
+-------------------+-----+-------------+-----------+-------------+---------------------------------------------------+
| master-bin.000001 | 4 | Format_desc | 1 | 107 |
Server ver: 6.0.6-alpha-debug-log, Binlog ver: 4 |
| master-bin.000001 | 107 | Query | 1 | 207 | use
`test`; create table t1 (a int) engine=falcon |
| master-bin.000001 | 207 | Query | 1 | 275 | use
`test`; BEGIN |
| master-bin.000001 | 275 | Table_map | 1 | 316 |
table_id: 16 (test.t1) |
| master-bin.000001 | 316 | Write_rows | 1 | 350 |
table_id: 16 flags: STMT_END_F |
| master-bin.000001 | 350 | Query | 1 | 419 | use
`test`; COMMIT |
+-------------------+-----+-------------+-----------+-------------+---------------------------------------------------+
6 rows in set (0.01 sec)
master2> select * from t1;
+------+
| a |
+------+
| 2 |
+------+
1 row in set (0.00 sec)
>
> But what worse, the server code patch leaves the same flaw letting A.
No, as you can see in the trace above, the engine committed both before
and after the patch (which is case A).
[snip]
>> === added file 'mysql-test/extra/binlog_tests/implicit.test'
>
> As a rule, a new file needs some comments. Could you please provide a
> modest heading description for this file?
Sure.
[snip]
>> === modified file 'sql/set_var.cc'
>> --- a/sql/set_var.cc 2008-05-21 10:17:29 +0000
>> +++ b/sql/set_var.cc 2008-06-09 16:58:19 +0000
>> @@ -2970,6 +2970,15 @@ static bool set_option_autocommit(THD *t
>>
>> ulonglong org_options= thd->options;
>>
>> + /*
>> + If we are setting AUTOCOMMIT=1 and it was not already 1, then we
>> + need to commit any outstanding transactions.
>> + */
>> + if (var->save_result.ulong_value != 0 &&
>> + (thd->options & OPTION_NOT_AUTOCOMMIT) &&
>> + ha_commit(thd))
>> + return 1;
>> +
>
> Strange, nothing has chanded since the previous patch regarding to
>
> begin; ... ; set autocommit=1
>
> committing transaction. It can not commit.
>
>> if (var->save_result.ulong_value != 0)
>> thd->options&= ~((sys_var_thd_bit*) var->var)->bit_flag;
>> else
>> @@ -2983,8 +2992,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;
>> }
>> else
>> {
If you compare it with the previous hunk, you will see the difference.
The previous hunk was:
diff -Nrup a/sql/set_var.cc b/sql/set_var.cc
--- a/sql/set_var.cc 2008-05-21 12:04:21 +02:00
+++ b/sql/set_var.cc 2008-06-05 11:57:16 +02:00
@@ -2970,6 +2970,18 @@ static bool set_option_autocommit(THD *t
ulonglong org_options= thd->options;
+ /*
+ 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.
+ */
+ 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;
}
else
{
Just my few cents,
Mats Kindahl
--
Mats Kindahl
Lead Software Developer
Replication Team
MySQL AB, www.mysql.com
