Merge lp:~sergei.glushchenko/percona-server/disable-flashcache into lp:percona-server/5.1

Proposed by Sergei Glushchenko on 2012-08-27
Status: Merged
Approved by: Alexey Kopytov on 2012-08-27
Approved revision: 446
Merged at revision: 480
Proposed branch: lp:~sergei.glushchenko/percona-server/disable-flashcache
Merge into: lp:percona-server/5.1
Diff against target: 227 lines (+71/-15)
9 files modified
Percona-Server/mysql-test/include/have_flashcache.inc (+7/-0)
Percona-Server/mysql-test/r/have_flashcache.require (+2/-0)
Percona-Server/mysql-test/r/percona_bug747032.result (+1/-0)
Percona-Server/mysql-test/r/percona_server_variables_debug.result (+1/-0)
Percona-Server/mysql-test/r/percona_server_variables_release.result (+1/-0)
Percona-Server/mysql-test/t/percona_bug747032.test (+32/-0)
Percona-Server/sql/mysql_priv.h (+1/-0)
Percona-Server/sql/mysqld.cc (+25/-15)
Percona-Server/sql/set_var.cc (+1/-0)
To merge this branch: bzr merge lp:~sergei.glushchenko/percona-server/disable-flashcache
Reviewer Review Type Date Requested Status
Alexey Kopytov (community) 2012-08-27 Approve on 2012-08-27
Review via email: mp+121389@code.launchpad.net

Description of the change

Bug #747032: Flashcache throws an error on startup when flashcache is not used
New boolean option --flashcache was introduced. When set to 0, flashcache
checks are disabled. Default is to disable checks.
Also error message been made more verbose including errno and system error
message.

http://jenkins.percona.com/view/PS%205.1/job/percona-server-5.1-param/393/

To post a comment you must log in.
Alexey Kopytov (akopytov) wrote :

Sergei,

   - since after this fix flashcache is disabled by default, failure to
     set it up when it's _explicitly_ enabled should be a fatal error
   - the above also means the error should be printed with
     sql_print_error() rather than sql_print_information()
   - that also means the code printing the "Flashcache bypass" message
     does not have to check if initialization was successful (it should
     just say "flashcache support initialized successfully")
   - strerror() is not portable. which is not that important in this
     case, of course, but I would use sql_perror() for consistency.
   - the command line option is available only on Linux, whereas the
     test case includes "not_windows.inc". Which is not the same set of
     platforms, it will fail on all non-Windows platforms except Linux
   - the test case dependency on external utilities like grep and sed is
     redundant, even though it might work in most cases (since this is a
     Linux-specific test).

   I think a clean way to implement the test case would be to add
   'include/have_flashcache.inc' (which would also require a separate
   status variable). So you can easily check if the code you are going
   to test is present in the binary. You should also test only the
   --flashcache=0 case (i.e. the default) and make sure the
   initialization message is not present in the error log. Then you only
   need a few lines in Perl to implement that (see
   percona_log_warnings_suppress.test for an example).

review: Needs Fixing

Alexey,

> I think a clean way to implement the test case would be to add
> 'include/have_flashcache.inc' (which would also require a separate
> status variable). So you can easily check if the code you are going
> to test is present in the binary. You should also test only the
> --flashcache=0 case (i.e. the default) and make sure the
> initialization message is not present in the error log. Then you only
> need a few lines in Perl to implement that (see
> percona_log_warnings_suppress.test for an example).

There is already status variable Flashcache_enabled, which can be used to check flashcache presence. I'm thinking of having include/not_flashcache.inc.

Testcase will look like:

--include not_flashcache.inc

# Some perl code to check the 'Flashcache bypass ...' message absence

Testcase been reworked as described above.
Other comments been also taken into account.

Alexey Kopytov (akopytov) wrote :

On 27.08.12 16:03, Sergei Glushchenko wrote:
> Alexey,
>
>> I think a clean way to implement the test case would be to add
>> 'include/have_flashcache.inc' (which would also require a separate
>> status variable). So you can easily check if the code you are going
>> to test is present in the binary. You should also test only the
>> --flashcache=0 case (i.e. the default) and make sure the
>> initialization message is not present in the error log. Then you only
>> need a few lines in Perl to implement that (see
>> percona_log_warnings_suppress.test for an example).
>
> There is already status variable Flashcache_enabled, which can be used to check flashcache presence. I'm thinking of having include/not_flashcache.inc.
>

No, Flashcache_enabled is present on all platforms, even on Windows. And
it will always be OFF on all platforms, since the test case does not
enable it explicitly. So not_flashcache.inc does not really check
anything, all platforms and binaries will pass it. Besides, the name of
that check is weird: we actually want to run this test when the
flashcache support is ON, so "include/not_flashcache.inc" is confusing.

I was talking about a separate system variable (not a status one, sorry)
similar to have_query_cache, have_partitioning, etc. that would only be
TRUE if the binary has flashcache code compiled in (i.e. is a Linux binary).

I got it. I added system variable have_flashcache, which indicates presence of flashcache code, which means it is always YES on Linux, and always NO on other platforms. Testcase been reworked to use this variable.

Alexey Kopytov (akopytov) wrote :

Looks good, thanks. Please don't forget to submit a 5.5 MP.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'Percona-Server/mysql-test/include/have_flashcache.inc'
2--- Percona-Server/mysql-test/include/have_flashcache.inc 1970-01-01 00:00:00 +0000
3+++ Percona-Server/mysql-test/include/have_flashcache.inc 2012-08-27 15:37:32 +0000
4@@ -0,0 +1,7 @@
5+# check if binary compiled with flashcache support
6+
7+-- require r/have_flashcache.require
8+
9+disable_query_log;
10+show variables like 'have_flashcache';
11+enable_query_log;
12
13=== added file 'Percona-Server/mysql-test/r/have_flashcache.require'
14--- Percona-Server/mysql-test/r/have_flashcache.require 1970-01-01 00:00:00 +0000
15+++ Percona-Server/mysql-test/r/have_flashcache.require 2012-08-27 15:37:32 +0000
16@@ -0,0 +1,2 @@
17+Variable_name Value
18+have_flashcache YES
19
20=== added file 'Percona-Server/mysql-test/r/percona_bug747032.result'
21--- Percona-Server/mysql-test/r/percona_bug747032.result 1970-01-01 00:00:00 +0000
22+++ Percona-Server/mysql-test/r/percona_bug747032.result 2012-08-27 15:37:32 +0000
23@@ -0,0 +1,1 @@
24+Occurrences: 0
25
26=== modified file 'Percona-Server/mysql-test/r/percona_server_variables_debug.result'
27--- Percona-Server/mysql-test/r/percona_server_variables_debug.result 2012-08-20 05:07:28 +0000
28+++ Percona-Server/mysql-test/r/percona_server_variables_debug.result 2012-08-27 15:37:32 +0000
29@@ -59,6 +59,7 @@
30 HAVE_CRYPT
31 HAVE_CSV
32 HAVE_DYNAMIC_LOADING
33+HAVE_FLASHCACHE
34 HAVE_GEOMETRY
35 HAVE_INNODB
36 HAVE_NDBCLUSTER
37
38=== modified file 'Percona-Server/mysql-test/r/percona_server_variables_release.result'
39--- Percona-Server/mysql-test/r/percona_server_variables_release.result 2012-08-20 03:14:02 +0000
40+++ Percona-Server/mysql-test/r/percona_server_variables_release.result 2012-08-27 15:37:32 +0000
41@@ -57,6 +57,7 @@
42 HAVE_CRYPT
43 HAVE_CSV
44 HAVE_DYNAMIC_LOADING
45+HAVE_FLASHCACHE
46 HAVE_GEOMETRY
47 HAVE_INNODB
48 HAVE_NDBCLUSTER
49
50=== added file 'Percona-Server/mysql-test/t/percona_bug747032.test'
51--- Percona-Server/mysql-test/t/percona_bug747032.test 1970-01-01 00:00:00 +0000
52+++ Percona-Server/mysql-test/t/percona_bug747032.test 2012-08-27 15:37:32 +0000
53@@ -0,0 +1,32 @@
54+###################### percona_bug747032.test ########################
55+# Bug #747032: Flashcache throws an error on startup when flashcache #
56+# is not used #
57+# #
58+# Test is designed to verify that --flashcache option work properly #
59+# to disable flashcache checks at startup #
60+######################################################################
61+
62+--source include/not_embedded.inc
63+--source include/have_flashcache.inc
64+
65+let $log_error_= `SELECT @@GLOBAL.log_error`;
66+if(!`select LENGTH('$log_error_')`)
67+{
68+ # MySQL Server on windows is started with --console and thus
69+ # does not know the location of its .err log, use default location
70+ let $log_error_ = $MYSQLTEST_VARDIR/log/mysqld.1.err;
71+}
72+# Assign env variable LOG_ERROR
73+let LOG_ERROR=$log_error_;
74+
75+perl;
76+ use strict;
77+ my $log_error= $ENV{'LOG_ERROR'} or die "LOG_ERROR not set";
78+ open(FILE, "$log_error") or die("Unable to open $log_error: $!\n");
79+ my $count = grep(/Flashcache bypass support initialized successfully/g,<FILE>);
80+ print "Occurrences: $count\n";
81+ close(FILE);
82+ # Clean error log file
83+ open(FILE, '>', "$log_error");
84+ close(FILE);
85+EOF
86
87=== modified file 'Percona-Server/sql/mysql_priv.h'
88--- Percona-Server/sql/mysql_priv.h 2012-08-20 03:14:02 +0000
89+++ Percona-Server/sql/mysql_priv.h 2012-08-27 15:37:32 +0000
90@@ -2229,6 +2229,7 @@
91 extern SHOW_COMP_OPTION have_geometry, have_rtree_keys;
92 extern SHOW_COMP_OPTION have_crypt;
93 extern SHOW_COMP_OPTION have_compress;
94+extern SHOW_COMP_OPTION have_flashcache;
95
96 extern int orig_argc;
97 extern char **orig_argv;
98
99=== modified file 'Percona-Server/sql/mysqld.cc'
100--- Percona-Server/sql/mysqld.cc 2012-08-20 03:14:02 +0000
101+++ Percona-Server/sql/mysqld.cc 2012-08-27 15:37:32 +0000
102@@ -714,6 +714,7 @@
103 SHOW_COMP_OPTION have_geometry, have_rtree_keys;
104 SHOW_COMP_OPTION have_crypt, have_compress;
105 SHOW_COMP_OPTION have_community_features;
106+SHOW_COMP_OPTION have_flashcache;
107
108 /* Thread specific variables */
109
110@@ -4288,24 +4289,23 @@
111
112 // disabled by default
113 cachedev_fd = -1;
114- cachedev_enabled= FALSE;
115
116 if (!mysql_data_home)
117 {
118- error_message= "mysql_data_home not set";
119+ error_message= "Flashcache setup error (mysql_data_home not set)";
120 goto epilogue;
121 }
122
123 if (statfs(mysql_data_home, &stfs_data_home_dir) < 0)
124 {
125- error_message= "statfs failed";
126+ error_message= "Flashcache setup error (statfs)";
127 goto epilogue;
128 }
129
130 mounts = setmntent("/etc/mtab", "r");
131 if (mounts == NULL)
132 {
133- error_message= "setmntent failed";
134+ error_message= "Flashcache setup error (setmntent)";
135 goto epilogue;
136 }
137
138@@ -4320,14 +4320,14 @@
139
140 if (ent == NULL)
141 {
142- error_message= "getmntent loop failed";
143+ error_message= "Flashcache setup error (getmntent loop)";
144 goto epilogue;
145 }
146
147 cachedev_fd = open(ent->mnt_fsname, O_RDONLY);
148 if (cachedev_fd < 0)
149 {
150- error_message= "open flash device failed";
151+ error_message= "Flashcache setup error (open flash device)";
152 goto epilogue;
153 }
154
155@@ -4336,18 +4336,17 @@
156 {
157 close(cachedev_fd);
158 cachedev_fd = -1;
159- error_message= "ioctl failed";
160+ error_message= "Flashcache setup error (ioctl)";
161 } else {
162 ioctl(cachedev_fd, FLASHCACHEADDWHITELIST, &pid);
163 }
164
165 epilogue:
166- sql_print_information("Flashcache bypass: %s",
167- (cachedev_fd > 0) ? "enabled" : "disabled");
168- if (error_message)
169- sql_print_information("Flashcache setup error is : %s\n", error_message);
170- else
171- cachedev_enabled= TRUE;
172+ if (error_message) {
173+ sql_perror(error_message);
174+ unireg_abort(1);
175+ }
176+ sql_print_information("Flashcache bypass support initialized successfully");
177
178 }
179
180@@ -4471,7 +4470,11 @@
181 #endif
182
183 #if defined(__linux__)
184- init_cachedev();
185+ have_flashcache= SHOW_OPTION_YES;
186+ if (cachedev_enabled)
187+ init_cachedev();
188+#else
189+ have_flashcache= SHOW_OPTION_NO;
190 #endif//__linux__
191
192 /*
193@@ -5925,7 +5928,8 @@
194 OPT_BINLOG_DIRECT_NON_TRANS_UPDATE,
195 OPT_DEFAULT_CHARACTER_SET_OLD,
196 OPT_MAX_LONG_DATA_SIZE,
197- OPT_EXPAND_FAST_INDEX_CREATION
198+ OPT_EXPAND_FAST_INDEX_CREATION,
199+ OPT_FLASHCACHE
200 };
201
202
203@@ -7504,6 +7508,12 @@
204 &global_system_variables.binlog_direct_non_trans_update,
205 &max_system_variables.binlog_direct_non_trans_update,
206 0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0},
207+ {"flashcache",
208+ OPT_FLASHCACHE,
209+ "Enable flashcache detection.",
210+ &cachedev_enabled,
211+ &cachedev_enabled,
212+ 0, GET_BOOL, OPT_ARG, 0, 0, 0, 0, 0, 0},
213 {0, 0, 0, 0, 0, 0, GET_NO_ARG, NO_ARG, 0, 0, 0, 0, 0, 0}
214 };
215
216
217=== modified file 'Percona-Server/sql/set_var.cc'
218--- Percona-Server/sql/set_var.cc 2012-08-20 03:14:02 +0000
219+++ Percona-Server/sql/set_var.cc 2012-08-27 15:37:32 +0000
220@@ -987,6 +987,7 @@
221 static sys_var_have_variable sys_have_community_features(&vars, "have_community_features", &have_community_features);
222 static sys_var_have_variable sys_have_rtree_keys(&vars, "have_rtree_keys", &have_rtree_keys);
223 static sys_var_have_variable sys_have_symlink(&vars, "have_symlink", &have_symlink);
224+static sys_var_have_variable sys_have_flashcache(&vars, "have_flashcache", &have_flashcache);
225 /* Global read-only variable describing server license */
226 static sys_var_const_str sys_license(&vars, "license", STRINGIFY_ARG(LICENSE));
227 /* Global variables which enable|disable logging */

Subscribers

People subscribed via source and target branches