Hi, Nirbhay!

On May 09, Nirbhay Choubey wrote:
> revision-id: 8928497acbe3e17a105897f405d5e5138d57a249
> parent(s): 3674c363a7e77e534c3e0d28659dc614c53fbcbc
> committer: Nirbhay Choubey
> branch nick: 10.1-wsrep
> timestamp: 2015-05-09 16:11:59 -0400
> message:
>
> MDEV-7067: Server outputs Galera (WSREP) information, even if Galera is disabled
>
> * mysqld_safe: Since wsrep_on variable is mandatory in 10.1, skip wsrep
> position recovery if its OFF.
> * mysqld: Remove "-wsrep" from server version
> * mysqld: Remove wsrep patch version from @@version_comment
> * mysqld: Introduce @@wsrep_patch_version

Looks ok, I just didn't like the wsrep.cmake change. See below.

> diff --git a/cmake/wsrep.cmake b/cmake/wsrep.cmake
> index c37cf74..305ab27 100644
> --- a/cmake/wsrep.cmake
> +++ b/cmake/wsrep.cmake
> @@ -28,29 +28,22 @@ OPTION(WITH_WSREP "WSREP replication API (to use, e.g. Galera Replication librar
>  # Set the patch version
>  SET(WSREP_PATCH_VERSION "10")
>
> -# MariaDB addition: Revision number of the last revision merged from
> -# codership branch visible in @@version_comment.
> -# Branch : codership-mysql/5.6
> -SET(WSREP_PATCH_REVNO "4144")  # Should be updated on every merge.
> -
> -# MariaDB addition: Revision number of the last revision merged from
> -# Branch : lp:maria/maria-10.0-galera
> -SET(WSREP_PATCH_REVNO2 "3919")  # Should be updated on every merge.
> -
> -# MariaDB: Obtain patch revision number:
> -# Update WSREP_PATCH_REVNO if WSREP_REV environment variable is set.
> -IF (DEFINED ENV{WSREP_REV})
> -  SET(WSREP_PATCH_REVNO $ENV{WSREP_REV})
> -ENDIF()
>
> -SET(WSREP_INTERFACE_VERSION 25)
> +# Obtain wsrep API version
> +EXECUTE_PROCESS(
> +  COMMAND sh -c "grep WSREP_INTERFACE_VERSION ${MySQL_SOURCE_DIR}/wsrep/wsrep_api.h | cut -d '\"' -f 2"
> +  OUTPUT_VARIABLE WSREP_API_VERSION
> +  RESULT_VARIABLE RESULT
> +)

I don't like calling grep|cut from cmake files. If you want to extract
the version from wsrep_api.h, use cmake commands, e.g.

  FILE(STRINGS "${MySQL_SOURCE_DIR}/wsrep/wsrep_api.h" WSREP_API_H
       LIMIT_COUNT 1 REGEX "WSREP_INTERFACE_VERSION")
  STRING(REGEX MATCH ....

> +STRING(REGEX REPLACE "(\r?\n)+$" "" WSREP_API_VERSION "${WSREP_API_VERSION}")
>
> -SET(WSREP_VERSION
> -    "${WSREP_INTERFACE_VERSION}.${WSREP_PATCH_VERSION}.r${WSREP_PATCH_REVNO}")
> +SET(WSREP_VERSION "${WSREP_API_VERSION}.${WSREP_PATCH_VERSION}"
> +  CACHE STRING "WSREP version")

make it INTERNAL, not STRING

>  SET(WSREP_PROC_INFO ${WITH_WSREP})
>
>  IF(WITH_WSREP)
> -  SET(COMPILATION_COMMENT "${COMPILATION_COMMENT}, wsrep_${WSREP_VERSION}")
> +  SET(WSREP_PATCH_VERSION "wsrep_${WSREP_VERSION}")
>  ENDIF()
>
> diff --git a/mysql-test/suite/sys_vars/r/sysvars_wsrep.result b/mysql-test/suite/sys_vars/r/sysvars_wsrep.result
> index 9c392b1..1763078 100644
> --- a/mysql-test/suite/sys_vars/r/sysvars_wsrep.result
> +++ b/mysql-test/suite/sys_vars/r/sysvars_wsrep.result
> @@ -365,6 +365,20 @@ NUMERIC_BLOCK_SIZE       NULL
>  ENUM_VALUE_LIST      TOI,RSU
>  READ_ONLY    NO
>  COMMAND_LINE_ARGUMENT        OPTIONAL
> +VARIABLE_NAME        WSREP_PATCH_VERSION
> +SESSION_VALUE        NULL
> +GLOBAL_VALUE wsrep_25.10
> +GLOBAL_VALUE_ORIGIN  COMPILE-TIME
> +DEFAULT_VALUE        NULL
> +VARIABLE_SCOPE       GLOBAL
> +VARIABLE_TYPE        VARCHAR
> +VARIABLE_COMMENT     wsrep patch version
> +NUMERIC_MIN_VALUE    NULL
> +NUMERIC_MAX_VALUE    NULL
> +NUMERIC_BLOCK_SIZE   NULL
> +ENUM_VALUE_LIST      NULL
> +READ_ONLY    YES
> +COMMAND_LINE_ARGUMENT        NULL
>  VARIABLE_NAME        WSREP_PROVIDER
>  SESSION_VALUE        NULL
>  GLOBAL_VALUE none
> diff --git a/mysql-test/suite/sys_vars/r/wsrep_patch_version_basic.result b/mysql-test/suite/sys_vars/r/wsrep_patch_version_basic.result
> new file mode 100644
> index 0000000..80f29e6
> --- /dev/null
> +++ b/mysql-test/suite/sys_vars/r/wsrep_patch_version_basic.result

I've actually stopped creating sys_var tests for every new variable
(even though I was the one who introduced sys_vars.all_vars test in the
first place :).

With the new I_S.SYSTEM_VARIABLES table we have all variables listed
there anyway, with all properties. So there's no need to copy these
boilerplate tests over and over.

> diff --git a/mysql-test/suite/wsrep/suite.pm b/mysql-test/suite/wsrep/suite.pm
> index 02dff24..33744a6 100644
> --- a/mysql-test/suite/wsrep/suite.pm
> +++ b/mysql-test/suite/wsrep/suite.pm
> @@ -27,6 +27,7 @@ push @::global_suppressions,
>       qr(WSREP: Could not open saved state file for reading: ),
>       qr(WSREP: option --wsrep-casual-reads is deprecated),
>       qr(WSREP: --wsrep-casual-reads=ON takes precedence over --wsrep-sync-wait=0),
> +     qr|WSREP: access file\(gvwstate.dat\) failed\(No such file or directory\)|,

why?

>     );
>
> diff --git a/sql/mysqld.cc b/sql/mysqld.cc
> index 14d904b..49a639b 100644
> --- a/sql/mysqld.cc
> +++ b/sql/mysqld.cc
> @@ -1902,13 +1902,15 @@ static void __cdecl kill_server(int sig_ptr)
>    }
>  #endif
>
> -  if (WSREP_ON)
> +  /* Stop wsrep threads in case they are running. */
>      wsrep_stop_replication(NULL);

Does this print any messages?

Regards,
Sergei