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

[Sergei Glushchenko]

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/

[Alexey Kopytov (akopytov)]

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

[Sergei Glushchenko (sergei.glushchenko)]

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

[Sergei Glushchenko (sergei.glushchenko)]

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

[Alexey Kopytov (akopytov)]

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

[Sergei Glushchenko (sergei.glushchenko)]

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)]

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

review: Approve

=== added file 'Percona-Server/mysql-test/include/have_flashcache.inc'
--- Percona-Server/mysql-test/include/have_flashcache.inc	1970-01-01 00:00:00 +0000
+++ Percona-Server/mysql-test/include/have_flashcache.inc	2012-08-27 15:37:32 +0000
@@ -0,0 +1,7 @@
+# check if binary compiled with flashcache support
+
+-- require r/have_flashcache.require
+
+disable_query_log;
+show variables like 'have_flashcache';
+enable_query_log;
 
=== added file 'Percona-Server/mysql-test/r/have_flashcache.require'
--- Percona-Server/mysql-test/r/have_flashcache.require	1970-01-01 00:00:00 +0000
+++ Percona-Server/mysql-test/r/have_flashcache.require	2012-08-27 15:37:32 +0000
@@ -0,0 +1,2 @@
+Variable_name	Value
+have_flashcache	YES
 
=== added file 'Percona-Server/mysql-test/r/percona_bug747032.result'
--- Percona-Server/mysql-test/r/percona_bug747032.result	1970-01-01 00:00:00 +0000
+++ Percona-Server/mysql-test/r/percona_bug747032.result	2012-08-27 15:37:32 +0000
@@ -0,0 +1,1 @@
+Occurrences: 0
 
=== modified file 'Percona-Server/mysql-test/r/percona_server_variables_debug.result'
--- Percona-Server/mysql-test/r/percona_server_variables_debug.result	2012-08-20 05:07:28 +0000
+++ Percona-Server/mysql-test/r/percona_server_variables_debug.result	2012-08-27 15:37:32 +0000
@@ -59,6 +59,7 @@
 HAVE_CRYPT
 HAVE_CSV
 HAVE_DYNAMIC_LOADING
+HAVE_FLASHCACHE
 HAVE_GEOMETRY
 HAVE_INNODB
 HAVE_NDBCLUSTER
 
=== modified file 'Percona-Server/mysql-test/r/percona_server_variables_release.result'
--- Percona-Server/mysql-test/r/percona_server_variables_release.result	2012-08-20 03:14:02 +0000
+++ Percona-Server/mysql-test/r/percona_server_variables_release.result	2012-08-27 15:37:32 +0000
@@ -57,6 +57,7 @@
 HAVE_CRYPT
 HAVE_CSV
 HAVE_DYNAMIC_LOADING
+HAVE_FLASHCACHE
 HAVE_GEOMETRY
 HAVE_INNODB
 HAVE_NDBCLUSTER
 
=== added file 'Percona-Server/mysql-test/t/percona_bug747032.test'
--- Percona-Server/mysql-test/t/percona_bug747032.test	1970-01-01 00:00:00 +0000
+++ Percona-Server/mysql-test/t/percona_bug747032.test	2012-08-27 15:37:32 +0000
@@ -0,0 +1,32 @@
+###################### percona_bug747032.test ########################
+# Bug #747032: Flashcache throws an error on startup when flashcache #
+#              is not used                                           #
+#                                                                    #
+# Test is designed to verify that --flashcache option work properly  #
+# to disable flashcache checks at startup                            #
+######################################################################
+
+--source include/not_embedded.inc
+--source include/have_flashcache.inc
+
+let $log_error_= `SELECT @@GLOBAL.log_error`;
+if(!`select LENGTH('$log_error_')`)
+{
+  # MySQL Server on windows is started with --console and thus
+  # does not know the location of its .err log, use default location
+  let $log_error_ = $MYSQLTEST_VARDIR/log/mysqld.1.err;
+}
+# Assign env variable LOG_ERROR
+let LOG_ERROR=$log_error_;
+
+perl;
+  use strict;
+  my $log_error= $ENV{'LOG_ERROR'} or die "LOG_ERROR not set";
+  open(FILE, "$log_error") or die("Unable to open $log_error: $!\n");
+  my $count = grep(/Flashcache bypass support initialized successfully/g,<FILE>);
+  print "Occurrences: $count\n";
+  close(FILE);
+  # Clean error log file
+  open(FILE, '>', "$log_error");
+  close(FILE);
+EOF
 
=== modified file 'Percona-Server/sql/mysql_priv.h'
--- Percona-Server/sql/mysql_priv.h	2012-08-20 03:14:02 +0000
+++ Percona-Server/sql/mysql_priv.h	2012-08-27 15:37:32 +0000
@@ -2229,6 +2229,7 @@
 extern SHOW_COMP_OPTION have_geometry, have_rtree_keys;
 extern SHOW_COMP_OPTION have_crypt;
 extern SHOW_COMP_OPTION have_compress;
+extern SHOW_COMP_OPTION have_flashcache;
 
 extern int orig_argc;
 extern char **orig_argv;
 
=== modified file 'Percona-Server/sql/mysqld.cc'
--- Percona-Server/sql/mysqld.cc	2012-08-20 03:14:02 +0000
+++ Percona-Server/sql/mysqld.cc	2012-08-27 15:37:32 +0000
@@ -714,6 +714,7 @@
 SHOW_COMP_OPTION have_geometry, have_rtree_keys;
 SHOW_COMP_OPTION have_crypt, have_compress;
 SHOW_COMP_OPTION have_community_features;
+SHOW_COMP_OPTION have_flashcache;
 
 /* Thread specific variables */
 
@@ -4288,24 +4289,23 @@
 
   // disabled by default
   cachedev_fd = -1;
-  cachedev_enabled= FALSE;
 
   if (!mysql_data_home)
   {
-    error_message= "mysql_data_home not set";
+    error_message= "Flashcache setup error (mysql_data_home not set)";
     goto epilogue;
   }
 
   if (statfs(mysql_data_home, &stfs_data_home_dir) < 0)
   {
-    error_message= "statfs failed";
+    error_message= "Flashcache setup error (statfs)";
     goto epilogue;
   }
 
   mounts = setmntent("/etc/mtab", "r");
   if (mounts == NULL)
   {
-    error_message= "setmntent failed";
+    error_message= "Flashcache setup error (setmntent)";
     goto epilogue;
   }
 
@@ -4320,14 +4320,14 @@
 
   if (ent == NULL)
   {
-    error_message= "getmntent loop failed";
+    error_message= "Flashcache setup error (getmntent loop)";
     goto epilogue;
   }
 
   cachedev_fd = open(ent->mnt_fsname, O_RDONLY);
   if (cachedev_fd < 0)
   {
-    error_message= "open flash device failed";
+    error_message= "Flashcache setup error (open flash device)";
     goto epilogue;
   }
 
@@ -4336,18 +4336,17 @@
   {
     close(cachedev_fd);
     cachedev_fd = -1;
-    error_message= "ioctl failed";
+    error_message= "Flashcache setup error (ioctl)";
   } else {
     ioctl(cachedev_fd, FLASHCACHEADDWHITELIST, &pid);
   }
 
 epilogue:
-  sql_print_information("Flashcache bypass: %s",
-      (cachedev_fd > 0) ? "enabled" : "disabled");
-  if (error_message)
-    sql_print_information("Flashcache setup error is : %s\n", error_message);
-  else
-    cachedev_enabled= TRUE;
+  if (error_message) {
+    sql_perror(error_message);
+    unireg_abort(1);
+  }
+  sql_print_information("Flashcache bypass support initialized successfully");
 
 }
 
@@ -4471,7 +4470,11 @@
 #endif
 
 #if defined(__linux__)
-  init_cachedev();
+  have_flashcache= SHOW_OPTION_YES;
+  if (cachedev_enabled)
+    init_cachedev();
+#else
+  have_flashcache= SHOW_OPTION_NO;
 #endif//__linux__
 
   /*
@@ -5925,7 +5928,8 @@
   OPT_BINLOG_DIRECT_NON_TRANS_UPDATE,
   OPT_DEFAULT_CHARACTER_SET_OLD,
   OPT_MAX_LONG_DATA_SIZE,
-  OPT_EXPAND_FAST_INDEX_CREATION
+  OPT_EXPAND_FAST_INDEX_CREATION,
+  OPT_FLASHCACHE
 };
 
 
@@ -7504,6 +7508,12 @@
    &global_system_variables.binlog_direct_non_trans_update,
    &max_system_variables.binlog_direct_non_trans_update,
    0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0},
+  {"flashcache",
+   OPT_FLASHCACHE,
+   "Enable flashcache detection.",
+   &cachedev_enabled,
+   &cachedev_enabled,
+   0, GET_BOOL, OPT_ARG, 0, 0, 0, 0, 0, 0},
   {0, 0, 0, 0, 0, 0, GET_NO_ARG, NO_ARG, 0, 0, 0, 0, 0, 0}
 };
 
 
=== modified file 'Percona-Server/sql/set_var.cc'
--- Percona-Server/sql/set_var.cc	2012-08-20 03:14:02 +0000
+++ Percona-Server/sql/set_var.cc	2012-08-27 15:37:32 +0000
@@ -987,6 +987,7 @@
 static sys_var_have_variable sys_have_community_features(&vars, "have_community_features", &have_community_features);
 static sys_var_have_variable sys_have_rtree_keys(&vars, "have_rtree_keys", &have_rtree_keys);
 static sys_var_have_variable sys_have_symlink(&vars, "have_symlink", &have_symlink);
+static sys_var_have_variable sys_have_flashcache(&vars, "have_flashcache", &have_flashcache);
 /* Global read-only variable describing server license */
 static sys_var_const_str	sys_license(&vars, "license", STRINGIFY_ARG(LICENSE));
 /* Global variables which enable|disable logging */