Merge lp:~laurynas-biveinis/percona-server/bug1334330 into lp:percona-server/5.6

[Laurynas Biveinis]

Fix bug 1334330 (--tc-heuristic-recover option values are broken).

The issue is that tc_heuristic_recover variable is set up by the
option parser to 0 if COMMIT was specified and 1 if ROLLBACK was, but
the users of this variable were treating 0 as option not given, 1 as
COMMIT, and were expecting 2 for ROLLBACK.

Fix by introducing a new option value NONE, which takes zero, and
bumps the other two as expected.

Add a testcase tc-heuristic-recover.test, re-record
mysqld--help-notwin.

http://jenkins.percona.com/job/percona-server-5.6-param/638/

[Sergei Glushchenko (sergei.glushchenko)]

Shouldn't mysqld--help-win.result be also modified?

review: Needs Fixing (g2)

[Laurynas Biveinis (laurynas-biveinis)]

It should, but for that we either fix Windows build (lots of effort), either update it manually (pointless IMHO). It's currently outdated already, and never run, so...

[Sergei Glushchenko (sergei.glushchenko)]

Ok, I just wanted to make sure it was intentional and not by mistake. It is fine by me to not update help-win. Is it our policy for all subsequent changes?

review: Approve (g2)

[Laurynas Biveinis (laurynas-biveinis)]

Yes.

[Alexey Kopytov (akopytov)]

I disagree about making it a _policy_ for not updating mysqld--help-win.result. Even if we don't support Windows yet, we sometimes may want to contribute patches to MySQL branches that do support Windows. So missing updates to mysqld--help-win by on oversight is one thing, but not updating it deliberately is a bit different.

[Laurynas Biveinis (laurynas-biveinis)]

I am sorry, I thought I replied but I hadn't.

I don't see how requiring maintenance of mysqld--help-win helps in any way, including contributing patches anywhere. The current way to maintain it would be manual, without a possibility to test whether it was updated actually correctly. Any feature we send somewhere would not take this file wholesale, and would need a specific re-record. So unless I'm missing something, it's a wasted work to update it.

[Alexey Kopytov (akopytov)]

It's the same as writing portable code: you write portable code by default, by providing portable alternatives or stubs for non-supported platforms where you know things will break. Claiming that "we deliberately write unportable code, because there's no possibility to test whether code for other platforms was updated correctly, so it's a wasted work" is a bit different to accidentally breaking unsupported platforms.

[Laurynas Biveinis (laurynas-biveinis)]

I'm sorry for causing some confusion here: best-effort attempt to write portable code is obviously a must. I don't see how updating this particular file, and this particular file only, serves this goal. We end up with a hand-maintained file where all that manual maintenance does not pay off in the event we or someone actually needs it and will have to do an MTR --record overwrite anyway.

[Alexey Kopytov (akopytov)]

Just as with the portable code, fixing mysqld--help-win.result will require some kind of analysis of the failures aka the diff with the actual results. Thus by deliberately neglecting its maintenance we increase the efforts to bring it up to date in case we or someone actually needs it. It's called bitrot.

=== modified file 'mysql-test/r/mysqld--help-notwin.result'
--- mysql-test/r/mysqld--help-notwin.result	2014-06-04 14:48:06 +0000
+++ mysql-test/r/mysqld--help-notwin.result	2014-07-09 18:03:50 +0000
@@ -1017,7 +1017,7 @@
  The number of table cache instances
  --tc-heuristic-recover=name 
  Decision to use in heuristic recover process. Possible
- values are COMMIT or ROLLBACK.
+ values are NONE, COMMIT, or ROLLBACK.
  --thread-cache-size=# 
  How many threads we should keep in a cache for reuse
  --thread-handling=name 
@@ -1376,7 +1376,7 @@
 sync-relay-log-info 10000
 sysdate-is-now FALSE
 table-open-cache-instances 1
-tc-heuristic-recover COMMIT
+tc-heuristic-recover NONE
 thread-cache-size 9
 thread-handling one-thread-per-connection
 thread-stack 262144

=== added file 'mysql-test/r/tc-heuristic-recover.result'
--- mysql-test/r/tc-heuristic-recover.result	1970-01-01 00:00:00 +0000
+++ mysql-test/r/tc-heuristic-recover.result	2014-07-09 18:03:50 +0000
@@ -0,0 +1,24 @@
+CREATE TABLE t1 (a INT) ENGINE=InnoDB;
+INSERT INTO t1 VALUES (1);
+BEGIN;
+INSERT INTO t1 VALUES (2);
+SET SESSION debug="d,crash_commit_after_prepare";
+COMMIT;
+Got one of the listed errors
+Restarting with --tc-heuristic-recover=ROLLBACK
+2 should be absent:
+SELECT * FROM t1;
+a
+1
+BEGIN;
+INSERT INTO t1 VALUES (2);
+SET SESSION debug="d,crash_commit_after_prepare";
+COMMIT;
+Got one of the listed errors
+Restarting with --tc-heuristic-recover=COMMIT
+2 should be present:
+SELECT * FROM t1;
+a
+1
+2
+DROP TABLE t1;

=== added file 'mysql-test/t/tc-heuristic-recover.test'
--- mysql-test/t/tc-heuristic-recover.test	1970-01-01 00:00:00 +0000
+++ mysql-test/t/tc-heuristic-recover.test	2014-07-09 18:03:50 +0000
@@ -0,0 +1,50 @@
+--source include/not_embedded.inc
+--source include/not_valgrind.inc
+--source include/have_debug.inc
+--source include/have_innodb.inc
+--source include/have_log_bin.inc
+
+let $restart_file= $MYSQLTEST_VARDIR/tmp/mysqld.1.expect;
+let $restart_log= $MYSQLTEST_VARDIR/log/mysqld.1.restart.err;
+
+CREATE TABLE t1 (a INT) ENGINE=InnoDB;
+
+INSERT INTO t1 VALUES (1);
+
+BEGIN;
+INSERT INTO t1 VALUES (2);
+SET SESSION debug="d,crash_commit_after_prepare";
+--exec echo "wait" > $restart_file
+--error 2006, 2013
+COMMIT;
+
+--source include/wait_until_disconnected.inc
+--echo Restarting with --tc-heuristic-recover=ROLLBACK
+--error 1
+--exec $MYSQLD_CMD --tc-heuristic-recover=ROLLBACK --console > $restart_log 2>&1
+--exec echo "restart" > $restart_file
+--enable_reconnect
+--source include/wait_until_connected_again.inc
+
+--echo 2 should be absent:
+SELECT * FROM t1;
+
+BEGIN;
+INSERT INTO t1 VALUES (2);
+SET SESSION debug="d,crash_commit_after_prepare";
+--exec echo "wait" > $restart_file
+--error 2006, 2013
+COMMIT;
+
+--source include/wait_until_disconnected.inc
+--echo Restarting with --tc-heuristic-recover=COMMIT
+--error 1
+--exec $MYSQLD_CMD --tc-heuristic-recover=COMMIT --console >> $restart_log 2>&1
+--exec echo "restart" > $restart_file
+--enable_reconnect
+--source include/wait_until_connected_again.inc
+
+--echo 2 should be present:
+SELECT * FROM t1;
+
+DROP TABLE t1;

=== modified file 'sql/mysqld.cc'
--- sql/mysqld.cc	2014-06-11 13:33:46 +0000
+++ sql/mysqld.cc	2014-07-09 18:03:50 +0000
@@ -297,7 +297,7 @@
 
 static const char *tc_heuristic_recover_names[]=
 {
-  "COMMIT", "ROLLBACK", NullS
+  "NONE", "COMMIT", "ROLLBACK", NullS
 };
 static TYPELIB tc_heuristic_recover_typelib=
 {
@@ -7497,8 +7497,8 @@
    &global_system_variables.sysdate_is_now,
    0, 0, GET_BOOL, NO_ARG, 0, 0, 1, 0, 1, 0},
   {"tc-heuristic-recover", 0,
-   "Decision to use in heuristic recover process. Possible values are COMMIT "
-   "or ROLLBACK.", &tc_heuristic_recover, &tc_heuristic_recover,
+   "Decision to use in heuristic recover process. Possible values are NONE, "
+   "COMMIT, or ROLLBACK.", &tc_heuristic_recover, &tc_heuristic_recover,
    &tc_heuristic_recover_typelib, GET_ENUM, REQUIRED_ARG, 0, 0, 0, 0, 0, 0},
 #if defined(ENABLED_DEBUG_SYNC)
   {"debug-sync-timeout", OPT_DEBUG_SYNC_TIMEOUT,