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

Proposed by Laurynas Biveinis
Status: Merged
Approved by: Sergei Glushchenko
Approved revision: 621
Merged at revision: 639
Proposed branch: lp:~laurynas-biveinis/percona-server/bug1334330
Merge into: lp:percona-server/5.6
Diff against target: 129 lines (+79/-5)
4 files modified
mysql-test/r/mysqld--help-notwin.result (+2/-2)
mysql-test/r/tc-heuristic-recover.result (+24/-0)
mysql-test/t/tc-heuristic-recover.test (+50/-0)
sql/mysqld.cc (+3/-3)
To merge this branch: bzr merge lp:~laurynas-biveinis/percona-server/bug1334330
Reviewer Review Type Date Requested Status
Sergei Glushchenko (community) g2 Approve
Review via email: mp+226184@code.launchpad.net

Description of the change

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/

To post a comment you must log in.
Revision history for this message
Sergei Glushchenko (sergei.glushchenko) wrote :

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

review: Needs Fixing (g2)
Revision history for this message
Laurynas Biveinis (laurynas-biveinis) wrote :

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...

Revision history for this message
Sergei Glushchenko (sergei.glushchenko) wrote :

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)
Revision history for this message
Laurynas Biveinis (laurynas-biveinis) wrote :

Yes.

Revision history for this message
Alexey Kopytov (akopytov) wrote :

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.

Revision history for this message
Laurynas Biveinis (laurynas-biveinis) wrote :

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.

Revision history for this message
Alexey Kopytov (akopytov) wrote :

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.

Revision history for this message
Laurynas Biveinis (laurynas-biveinis) wrote :

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.

Revision history for this message
Alexey Kopytov (akopytov) wrote :

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.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'mysql-test/r/mysqld--help-notwin.result'
2--- mysql-test/r/mysqld--help-notwin.result 2014-06-04 14:48:06 +0000
3+++ mysql-test/r/mysqld--help-notwin.result 2014-07-09 18:03:50 +0000
4@@ -1017,7 +1017,7 @@
5 The number of table cache instances
6 --tc-heuristic-recover=name
7 Decision to use in heuristic recover process. Possible
8- values are COMMIT or ROLLBACK.
9+ values are NONE, COMMIT, or ROLLBACK.
10 --thread-cache-size=#
11 How many threads we should keep in a cache for reuse
12 --thread-handling=name
13@@ -1376,7 +1376,7 @@
14 sync-relay-log-info 10000
15 sysdate-is-now FALSE
16 table-open-cache-instances 1
17-tc-heuristic-recover COMMIT
18+tc-heuristic-recover NONE
19 thread-cache-size 9
20 thread-handling one-thread-per-connection
21 thread-stack 262144
22
23=== added file 'mysql-test/r/tc-heuristic-recover.result'
24--- mysql-test/r/tc-heuristic-recover.result 1970-01-01 00:00:00 +0000
25+++ mysql-test/r/tc-heuristic-recover.result 2014-07-09 18:03:50 +0000
26@@ -0,0 +1,24 @@
27+CREATE TABLE t1 (a INT) ENGINE=InnoDB;
28+INSERT INTO t1 VALUES (1);
29+BEGIN;
30+INSERT INTO t1 VALUES (2);
31+SET SESSION debug="d,crash_commit_after_prepare";
32+COMMIT;
33+Got one of the listed errors
34+Restarting with --tc-heuristic-recover=ROLLBACK
35+2 should be absent:
36+SELECT * FROM t1;
37+a
38+1
39+BEGIN;
40+INSERT INTO t1 VALUES (2);
41+SET SESSION debug="d,crash_commit_after_prepare";
42+COMMIT;
43+Got one of the listed errors
44+Restarting with --tc-heuristic-recover=COMMIT
45+2 should be present:
46+SELECT * FROM t1;
47+a
48+1
49+2
50+DROP TABLE t1;
51
52=== added file 'mysql-test/t/tc-heuristic-recover.test'
53--- mysql-test/t/tc-heuristic-recover.test 1970-01-01 00:00:00 +0000
54+++ mysql-test/t/tc-heuristic-recover.test 2014-07-09 18:03:50 +0000
55@@ -0,0 +1,50 @@
56+--source include/not_embedded.inc
57+--source include/not_valgrind.inc
58+--source include/have_debug.inc
59+--source include/have_innodb.inc
60+--source include/have_log_bin.inc
61+
62+let $restart_file= $MYSQLTEST_VARDIR/tmp/mysqld.1.expect;
63+let $restart_log= $MYSQLTEST_VARDIR/log/mysqld.1.restart.err;
64+
65+CREATE TABLE t1 (a INT) ENGINE=InnoDB;
66+
67+INSERT INTO t1 VALUES (1);
68+
69+BEGIN;
70+INSERT INTO t1 VALUES (2);
71+SET SESSION debug="d,crash_commit_after_prepare";
72+--exec echo "wait" > $restart_file
73+--error 2006, 2013
74+COMMIT;
75+
76+--source include/wait_until_disconnected.inc
77+--echo Restarting with --tc-heuristic-recover=ROLLBACK
78+--error 1
79+--exec $MYSQLD_CMD --tc-heuristic-recover=ROLLBACK --console > $restart_log 2>&1
80+--exec echo "restart" > $restart_file
81+--enable_reconnect
82+--source include/wait_until_connected_again.inc
83+
84+--echo 2 should be absent:
85+SELECT * FROM t1;
86+
87+BEGIN;
88+INSERT INTO t1 VALUES (2);
89+SET SESSION debug="d,crash_commit_after_prepare";
90+--exec echo "wait" > $restart_file
91+--error 2006, 2013
92+COMMIT;
93+
94+--source include/wait_until_disconnected.inc
95+--echo Restarting with --tc-heuristic-recover=COMMIT
96+--error 1
97+--exec $MYSQLD_CMD --tc-heuristic-recover=COMMIT --console >> $restart_log 2>&1
98+--exec echo "restart" > $restart_file
99+--enable_reconnect
100+--source include/wait_until_connected_again.inc
101+
102+--echo 2 should be present:
103+SELECT * FROM t1;
104+
105+DROP TABLE t1;
106
107=== modified file 'sql/mysqld.cc'
108--- sql/mysqld.cc 2014-06-11 13:33:46 +0000
109+++ sql/mysqld.cc 2014-07-09 18:03:50 +0000
110@@ -297,7 +297,7 @@
111
112 static const char *tc_heuristic_recover_names[]=
113 {
114- "COMMIT", "ROLLBACK", NullS
115+ "NONE", "COMMIT", "ROLLBACK", NullS
116 };
117 static TYPELIB tc_heuristic_recover_typelib=
118 {
119@@ -7497,8 +7497,8 @@
120 &global_system_variables.sysdate_is_now,
121 0, 0, GET_BOOL, NO_ARG, 0, 0, 1, 0, 1, 0},
122 {"tc-heuristic-recover", 0,
123- "Decision to use in heuristic recover process. Possible values are COMMIT "
124- "or ROLLBACK.", &tc_heuristic_recover, &tc_heuristic_recover,
125+ "Decision to use in heuristic recover process. Possible values are NONE, "
126+ "COMMIT, or ROLLBACK.", &tc_heuristic_recover, &tc_heuristic_recover,
127 &tc_heuristic_recover_typelib, GET_ENUM, REQUIRED_ARG, 0, 0, 0, 0, 0, 0},
128 #if defined(ENABLED_DEBUG_SYNC)
129 {"debug-sync-timeout", OPT_DEBUG_SYNC_TIMEOUT,

Subscribers

People subscribed via source and target branches