Hi Jesper,
Thank you for finding BUG#59530. The bug only needs a three-line fix,
see below. I will fix it, but it may take some time for the patch to
come through to the public source trees.
The problem appears when the --server-id option prevents mysqlbinlog
from processing the initial Format_description_log_event. It is
essential to process the Format_description_log_event before other
events are processed. So the solution is to never filter out
Format_description_log_events. Here is the patch:
=== modified file 'client/mysqlbinlog.cc'
--- client/mysqlbinlog.cc 2011-01-18 12:09:03 +0000
+++ client/mysqlbinlog.cc 2011-03-15 15:59:23 +0000
@@ -683,10 +683,10 @@ Exit_status process_event(PRINT_EVENT_IN
*/
start_datetime= 0;
offset= 0; // print everything and protect against cycling rec_count
+ if (server_id && (server_id != ev->server_id))
+ /* skip just this event, continue processing the log. */
+ goto end;
}
- if (server_id && (server_id != ev->server_id))
- /* skip just this event, continue processing the log. */
- goto end;
if (((my_time_t)(ev->when) >= stop_datetime)
|| (pos >= stop_position_mot))
{
You will also have to do something similar in your patch: --thread-id
should not be applied to Format_description_log_events. So your patch
plus the above patch should look like this:
@@ -683,10 +685,21 @@ Exit_status process_event(PRINT_EVENT_IN
*/
start_datetime= 0;
offset= 0; // print everything and protect against cycling rec_count
+ if (server_id && (server_id != ev->server_id))
+ /* skip just this event, continue processing the log. */
+ goto end;
+ if (thread_id) {
+ if (ev_type == QUERY_EVENT)
+ {
+ current_thread_id = ((Query_log_event*)ev)->thread_id;
+ }
+ if (current_thread_id && (thread_id != current_thread_id))
+ {
+ /* skip just this event, continue processing the log. */
+ goto end;
+ }
+ }
}
- if (server_id && (server_id != ev->server_id))
- /* skip just this event, continue processing the log. */
- goto end;
if (((my_time_t)(ev->when) >= stop_datetime)
|| (pos >= stop_position_mot))
{
Best regards,
Sven Sandberg
Software engineer
Oracle / MySQL Replication team
On 03/06/2011 01:20 AM, Jesper Wisborg Krogh wrote:
> Hi Sven,
>
> Thanks for your feedback. Your suggestions sounds reasonable and I will take a look
> at it.
>
> There is one issue with adding the test cases for filtering by server id. I actually
> initially was looking at adding some tests that included filtering by server id, however
> the result was that I discovered BUG#59530, so those tests will in general not pass.
>
> Thanks,
> Jesper
>
> On 01/03/2011, at 8:23 PM, Sven Sandberg wrote:
>
>> Hello Jesper,
>>
>> Thank you for this contribution. It's a good idea to add the
>> --thread-id flag to mysqlbinlog. The source patch is well written, it
>> works for both SBL and RBL, and I see no immediate problems with it.
>> The feature is not very intrusive either; it is unlikely to be in the
>> way of possible future work such as scripted or pluggable filters.
>>
>> I would suggest two improvements:
>>
>> 1. If the binary log contains events originating from more than one
>> server, then it is not safe to filter by only thread_id, because
>> two unrelated threads on two different servers may have the same
>> thread_id. Hence, we should generate a warning when --thread-id
>> is set and --server-id is not set.
>>
>> 2. The test case needs to be improved to cover more cases and to
>> reduce future maintenance. I will be quite rigorous and require
>> several changes. Many of our existing tests don't follow such
>> strict guidelines, but we try to enforce them when we add or
>> update tests. This is important because we spend a lot of time
>> maintaining test cases. So we need to write tests that don't need
>> to be updated when something in the server changes.
>>
>> 1. Test that --thread-id without --server-id gives a warning.
>>
>> 2. Test the same thing for a binary log that contains row events
>> and a binary log that contains statement events. It may be
>> easiest to move the test to the mysql-test/suite/binlog/t;
>> tests in that directory are run once for each binlog_format
>> (you have to remove 'source
>> include/have_binlog_format_statement.inc').
>>
>> 3. Don't assume that the binlog name is master-bin.000001. Read
>> the binlog name like this:
>>
>> --let $binlog_file= query_get_value(SHOW MASTER STATUS, File, 1)
>>
>> 4. Please avoid relying on comparison with the result file. See
>> the suggested fixes below: if the feature does not work as
>> specified, we will get an error in diff_tables instead of a
>> result file mismatch. This makes the test much easier to
>> maintain in the future.
>>
>> 5. Test mysqlbinlog with no flag, with --thread-id alone, with
>> --server-id alone, and with --thread-id and --server-id
>> together; and in each case with ids that exist in the binlog
>> and ids that don't exist in the binlog. I'd suggest something
>> like:
>>
>> CREATE TABLE t1 (_server_id INT, _thread_id INT);
>>
>> FLUSH LOGS;
>> SET @@GLOBAL.SERVER_ID = 1, @@SESSION.PSEUDO_THREAD_ID = 1;
>> INSERT INTO t1 SET _server_id=1, _thread_id=1;
>> SET @@GLOBAL.SERVER_ID = 1, @@SESSION.PSEUDO_THREAD_ID = 2;
>> INSERT INTO t1 SET _server_id=1, _thread_id=2;
>> SET @@GLOBAL.SERVER_ID = 2, @@SESSION.PSEUDO_THREAD_ID = 1;
>> INSERT INTO t1 SET _server_id=2, _thread_id=1;
>> SET @@GLOBAL.SERVER_ID = 2, @@SESSION.PSEUDO_THREAD_ID = 2;
>> INSERT INTO t1 SET _server_id=2, _thread_id=2;
>> FLUSH LOGS;
>>
>> CREATE TABLE data SELECT * FROM t1;
>> CREATE TABLE t2 LIKE t1;
>>
>> # Test all combinations of --server-id and --thread-id.
>> --let $server_id= 0
>> while ($server_id<= 3)
>> {
>> --let $thread_id= 0
>> while ($thread_id<= 3)
>> {
>> --source extra/binlog_tests/binlog_mysqlbinlog_thread_id.inc
>> --inc $thread_id
>> }
>> --inc $server_id
>> }
>>
>> Then, create the file
>> mysql-test/extra/binlog_tests/binlog_mysqlbinlog_thread_id.inc
>> that contains:
>>
>> # ==== Purpose ====
>> #
>> # Auxiliary file used by binlog_mysqlbinlog_thread_id. Runs
>> # mysqlbinlog [--thread-id=X] [--server-id=Y]
>> # and checks that the correct statements were filtered.
>> #
>> # ==== Usage ====
>> #
>> # --let $thread_id= X
>> # --let $server_id= Y
>> # --source extra/binlog_tests/binlog_mysqlbinlog_thread_id.inc
>> #
>> # If $thread_id is set, passes --thread-id=$thread_id to
>> # mysqlbinlog. If $server_id is set, passes
>> # --server-id=$server_id to mysqlbinlog.
>> DELETE FROM t1;
>> DELETE FROM t2;
>> --let $options=
>> --let $where=
>> if ($thread_id)
>> {
>> --let $options= $options --thread-id=$thread_id
>> --let $where= $where _thread_id=$thread_id
>> }
>> if ($server_id)
>> {
>> --let $options= $options --server-id=$server_id
>> --let $where= $where _server_id=$server_id
>> }
>> if ($where)
>> {
>> --let $where= WHERE $where
>> }
>> --exec $MYSQL_BINLOG $options $MYSQLD_DATADIR/$binlog_file | $MYSQL
>> INSERT INTO t2 SELECT * FROM data $where;
>> --let $diff_tables= t1, t2
>> --source include/diff_tables.inc
>>
>> 6. Please add a comment to the top of the test case. Something
>> like:
>>
>> # ==== Purpose ====
>> #
>> # Verify that mysqlbinlog --thread-id works as expected. The
>> # flag is tested both alone and in conjuction with --server-id,
>> # and both with id that exists and that don't exist in the
>> # binary log. Also test that --thread-id without --server-id
>> # gives a warning.
>> #
>> # ==== Method ====
>> #
>> # Create tables t1, t2, and data, with two int columns. Insert
>> # the current values of pseudo_thread_id and server_id into t1
>> # for some different values. Record the inserts in a separate
>> # binlog. Copy the contents of t1 to data and delete all rows
>> # from t1. Run mysqlbinlog with several different combinations
>> # of --thread-id and --server-id and pipe the output to a
>> # client. For each combination, copy the rows that we expect
>> # mysqlbinlog to filter in from data to t2 and then compare
>> # t1 and t2.
>> #
>> # ==== References ====
>> #
>> # BUG#59529: Add support for filtering by thread id in mysqlbinlog
>>
>> 7. We always guarantee that tests start from a clean state, so
>> there is no need to drop tables or call RESET MASTER at the
>> beginning of the test.
>>
>> 8. Please don't disable warnings in the test. In the end of the
>> test, drop exactly the tables that were created, without
>> IF EXISTS.
>>
>> 9. Please restore the value of SERVER_ID at the end of the test.
>>
>> Best regards,
>> Sven Sandberg
>> Software engineer
>> Oracle / MySQL Replication team
>
