Merge lp:~tsarev/percona-server/5.1-18205_01_mysqlbinlog_fix.patch into lp:percona-server/5.1

[Oleg Tsarev]

http://jenkins.percona.com/view/Percona%20Server%205.1/job/percona-server-5.1-param/250/

Make mysqlbinlog omit redundant `use` around BEGIN/SAVEPOINT/COMMIT/ROLLBACK in 5.0 binlogs
This is a merge of lp:percona-server/rnt-5.1 patch mysqlbinlog_fix.patch.

MySQL 5.0 does not flag BEGIN/SAVEPOINT/COMMIT/ROLLBACK statements in its binlogs with LOG_EVENT_SUPPRESS_USE_F like 5.1+ does. This causes unnecessary `use` statements around such statements when the binlog is dumped by mysqlbinlog.

Fix by always suppressing the output of `use` for these statements.

[Laurynas Biveinis (laurynas-biveinis)]

Please describe the safe_connect change, what does it fix?

Line 20: s/binary log 5.0/5.0 binary log
Line 31: s/and/any

review: Needs Information

[Oleg Tsarev (tsarev)]

According to comments safe_connect "Create and initialize the global mysql object, and connect to the server".

Without this change safe_connect will overwrite available connection (see line 35 in MP) and connection will be leaked.

review: Needs Resubmitting

[Oleg Tsarev (tsarev)]

I fixed misstyping in comments

[Alexey Kopytov (akopytov)]

safe_connect() is called only once in mysqlbinlog, so it's not clear under what circumstances it could overwrite the available connection.

[Oleg Tsarev (tsarev)]

Original patch "mysqlbinlog_fix.patch" contain this, and MariaDB also merted it with this changes.
What you proposed? Remove this change?

[Alexey Kopytov (akopytov)]

Yes, it doesn't seem to be necessary.

[Laurynas Biveinis (laurynas-biveinis)]

LGTM, thank you.

review: Approve

[Alexey Kopytov (akopytov)]

But shouldn't we also have a bug report or BP for this? So it's reflected in the changelog.

review: Needs Information

[Oleg Tsarev (tsarev)]

According to customer issue 18205 fix should be included to lp:percona-server/5.1

review: Needs Resubmitting

[Laurynas Biveinis (laurynas-biveinis)]

It will be included but we do need to have a bug report or a blueprint (probably the former in this case), so we can track it in milestone, write release notes for it, etc.

review: Needs Information

[Alexey Kopytov (akopytov)]

Excellent, but how Hrvoje will know this fix should be included into release notes?

review: Needs Information

[Oleg Tsarev (tsarev)]

Description from commit it's enough.

review: Needs Resubmitting

[Oleg Tsarev (tsarev)]

Probably Stewart solve this missunderstanding from both sides

review: Needs Information

[Oleg Tsarev (tsarev)]

Linked bug #702303 related to feature

review: Needs Resubmitting

[Laurynas Biveinis (laurynas-biveinis)]

Is bug 702303 really related: it concerns with --rewrite-db option, while this MP looks like it's relevant for 5.0 binlogs without such option. The fix is also completely different. I'm not saying that it's not that bug, I don't know that, but that it needs clarification.

review: Needs Information

[Alexey Kopytov (akopytov)]

It's an unrelated bug. Which is easy to find out by looking at the fix for that bug.

[Oleg Tsarev (tsarev)]

You are right, this bug unrelated.

I have no idea which bug originally related to proposed fix.
This fix required for customer.

Perhaps avoid the burocracy? This is fix. Fix does better for Percona-Server.
What you need? I have no idea.

review: Needs Resubmitting

[Laurynas Biveinis (laurynas-biveinis)]

This is not a bureaucracy but quite important part of trunk merge and release process. Do you propose to write in release notes "Some fixes that do better for Percona Server"?

If there is no bug, make one.

review: Needs Information

[Oleg Tsarev (tsarev)]

bug #929521

Now all it is ok?

review: Needs Resubmitting

[Alexey Kopytov (akopytov)]

After referencing the bug # in revision comments -- yes.

review: Needs Fixing

[Oleg Tsarev (tsarev)]

And now?

review: Needs Resubmitting

[Alexey Kopytov (akopytov)]

And now I still don't see the bug # in revision comments.

review: Needs Fixing

[Oleg Tsarev (tsarev)]

oleg.tsarev (0) /storage/dev/percona/18205/final$ bzr log lp:~tsarev/percona-server/5.1-18205_01_mysqlbinlog_fix.patch | head -n 15
------------------------------------------------------------
revno: 424
fixes bug(s): https://launchpad.net/bugs/929521
committer: Oleg Tsarev <email address hidden>
branch nick: final
timestamp: Thu 2012-02-09 22:59:45 +0900
message:
  Make mysqlbinlog omit redundant around BEGIN/SAVEPOINT/COMMIT/ROLLBACK in 5.0 binlogs.
  This is a merge of lp:percona-server/rnt-5.1 patch mysqlbinlog_fix.patch.

  MySQL 5.0 does not flag BEGIN/SAVEPOINT/COMMIT/ROLLBACK statements in its binlogs with LOG_EVENT_SUPPRESS_USE_F like 5.1+ does. This causes unnecessary statements around such statements when the binlog is dumped by mysqlbinlog.

  Fix by always suppressing the output of for these statements.
------------------------------------------------------------

review: Needs Resubmitting

[Alexey Kopytov (akopytov)]

Woohoo! You did it! Congratulations!

Truly yours,
"Pure Mathematician"

review: Approve

[Vadim Tkachenko (vadim-tk)]

I see that requirements for fixes were quite reasonable and explainable.

=== modified file 'Percona-Server/client/mysqlbinlog.cc'
--- Percona-Server/client/mysqlbinlog.cc	2011-07-11 16:13:27 +0000
+++ Percona-Server/client/mysqlbinlog.cc	2012-02-09 14:07:20 +0000
@@ -741,9 +741,19 @@
 
     switch (ev_type) {
     case QUERY_EVENT:
-      if (!((Query_log_event*)ev)->is_trans_keyword() &&
-          shall_skip_database(((Query_log_event*)ev)->db))
-        goto end;
+      if (!((Query_log_event*)ev)->is_trans_keyword())
+      {
+        if (shall_skip_database(((Query_log_event*)ev)->db))
+          goto end;
+      }
+      else
+      {
+        /*
+          In case the event for one of these statements is obtained
+          from 5.0 binary log, make it compatible with 5.1.
+        */
+        ev->flags|= LOG_EVENT_SUPPRESS_USE_F;
+      }
       if (opt_base64_output_mode == BASE64_OUTPUT_ALWAYS)
       {
         if ((retval= write_event_header_and_base64(ev, result_file,

=== modified file 'Percona-Server/mysql-test/r/mysqlbinlog.result'
--- Percona-Server/mysql-test/r/mysqlbinlog.result	2011-03-25 14:16:13 +0000
+++ Percona-Server/mysql-test/r/mysqlbinlog.result	2012-02-09 14:07:20 +0000
@@ -220,7 +220,6 @@
 /*!50003 SET @OLD_COMPLETION_TYPE=@@COMPLETION_TYPE,COMPLETION_TYPE=0*/;
 DELIMITER /*!*/;
 ROLLBACK/*!*/;
-use test/*!*/;
 SET TIMESTAMP=1108844556/*!*/;
 SET @@session.pseudo_thread_id=999999999/*!*/;
 SET @@session.auto_increment_increment=1, @@session.auto_increment_offset=1/*!*/;
@@ -228,6 +227,7 @@
 SET @@session.collation_database=DEFAULT/*!*/;
 BEGIN
 /*!*/;
+use test/*!*/;
 SET TIMESTAMP=1108844555/*!*/;
 insert t1 values (1)
 /*!*/;
@@ -239,7 +239,6 @@
 /*!40019 SET @@session.max_insert_delayed_threads=0*/;
 /*!50003 SET @OLD_COMPLETION_TYPE=@@COMPLETION_TYPE,COMPLETION_TYPE=0*/;
 DELIMITER /*!*/;
-use test/*!*/;
 SET TIMESTAMP=1108844556/*!*/;
 SET @@session.pseudo_thread_id=999999999/*!*/;
 SET @@session.auto_increment_increment=1, @@session.auto_increment_offset=1/*!*/;
@@ -247,6 +246,7 @@
 SET @@session.collation_database=DEFAULT/*!*/;
 BEGIN
 /*!*/;
+use test/*!*/;
 SET TIMESTAMP=1108844555/*!*/;
 insert t1 values (1)
 /*!*/;
@@ -581,7 +581,6 @@
 SET @@session.collation_database=DEFAULT/*!*/;
 BEGIN
 /*!*/;
-use test/*!*/;
 SET TIMESTAMP=1266652094/*!*/;
 SavePoint mixed_cases
 /*!*/;
@@ -592,11 +591,9 @@
 SET TIMESTAMP=1266652094/*!*/;
 INSERT INTO db1.t1 VALUES(40)
 /*!*/;
-use test/*!*/;
 SET TIMESTAMP=1266652094/*!*/;
 ROLLBACK TO mixed_cases
 /*!*/;
-use db1/*!*/;
 SET TIMESTAMP=1266652094/*!*/;
 INSERT INTO db1.t2 VALUES("after rollback to")
 /*!*/;
@@ -624,7 +621,6 @@
 SET @@session.collation_database=DEFAULT/*!*/;
 BEGIN
 /*!*/;
-use test/*!*/;
 SET TIMESTAMP=1266652094/*!*/;
 SavePoint mixed_cases
 /*!*/;