maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

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