Hi, Sergey! Yes, that's what I had in mind, thanks! There are few rough edges, see below. Ok to push after you fix that, there's no need for another review round. On Dec 26, svoj@mariadb.org wrote:

revision-id: 72e58dc2b1a0bb859e918de0ff820d5a2a92dc32 parent(s): db89dd3a8f7b0d868946d25ba98c6f88612d309a committer: Sergey Vojtovich branch nick: 10.1 timestamp: 2014-12-26 17:03:45 +0400 message:

MDEV-7273 - 10.1 fails to start up during tc_log initializations on PPC64

log-tc-size is 24K by default. Page size is 64K on PPC64. But log-tc-size must be at least 3 x page size. This is enforced by TC_LOG_MMAP::open() with a comment: to guarantee non-empty pool.

This all makes server not startable in default configuration on PPC64.

Autosize log-tc-size, so that it's min value= page size * 6, default value= page size * 6, block size= page size.

--- mysql-test/r/mysqld--help.result | 1 - mysql-test/suite/sys_vars/r/log_tc_size_basic.result | 4 ++++ .../suite/sys_vars/r/sysvars_server_notembedded.result | 14 ++++++++++++++ mysql-test/suite/sys_vars/t/log_tc_size_basic.test | 5 +++++ mysql-test/t/mysqld--help.test | 2 +- sql/log.cc | 5 ++--- sql/log.h | 3 +-- sql/mysqld.cc | 7 +------ sql/sys_vars.cc | 11 +++++++++++ 9 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/mysql-test/suite/sys_vars/t/log_tc_size_basic.test b/mysql-test/suite/sys_vars/t/log_tc_size_basic.test new file mode 100644 index 0000000..1fce212 --- /dev/null +++ b/mysql-test/suite/sys_vars/t/log_tc_size_basic.test @@ -0,0 +1,5 @@ +--error ER_INCORRECT_GLOBAL_LOCAL_VAR,ER_UNKNOWN_SYSTEM_VARIABLE +SET GLOBAL log_tc_size=1; + +--error ER_INCORRECT_GLOBAL_LOCAL_VAR,ER_UNKNOWN_SYSTEM_VARIABLE

do we ever build on platforms without mmap? if the answer is "no" - you don't need ER_UNKNOWN_SYSTEM_VARIABLE here

+SET SESSION log_tc_size=1; diff --git a/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result b/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result index 006b9a9..5491169 100644 --- a/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result +++ b/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result @@ -1757,6 +1757,20 @@ NUMERIC_BLOCK_SIZE NULL ENUM_VALUE_LIST innodb,query_plan,explain READ_ONLY NO COMMAND_LINE_ARGUMENT REQUIRED +VARIABLE_NAME LOG_TC_SIZE +SESSION_VALUE NULL +GLOBAL_VALUE 24576 +GLOBAL_VALUE_ORIGIN AUTO +DEFAULT_VALUE 24576 +VARIABLE_SCOPE GLOBAL +VARIABLE_TYPE BIGINT UNSIGNED +VARIABLE_COMMENT Size of transaction coordinator log. +NUMERIC_MIN_VALUE 24576 +NUMERIC_MAX_VALUE 18446744073709551615 +NUMERIC_BLOCK_SIZE 4096 +ENUM_VALUE_LIST NULL +READ_ONLY YES +COMMAND_LINE_ARGUMENT REQUIRED

if the answer is "yes" - this test will fail there. even if the answer is "no", the variable value is architecture dependent. But it's good to see all stable variable metadata (e.g. that GLOBAL_VALUE_ORIGIN is AUTO).

VARIABLE_NAME LOG_WARNINGS SESSION_VALUE 1 GLOBAL_VALUE 1 diff --git a/sql/log.cc b/sql/log.cc index 66e1426..6d6512e 100644 --- a/sql/log.cc +++ b/sql/log.cc @@ -8793,7 +8792,7 @@ int TC_LOG_MMAP::unlog(ulong cookie, my_xid xid) mysql_mutex_lock(&LOCK_pending_checkpoint); if (pending_checkpoint == NULL) { - uint32 size= sizeof(*pending_checkpoint); + uint32 size= sizeof(*pending_checkpoint) + tc_log_page_size / sizeof(my_xid);

that's not enough. There are further sizeof's, like if (pending_checkpoint->count == sizeof(pending_checkpoint->cookies) / sizeof(pending_checkpoint->cookies[0])) and for (i= 0; i < sizeof(pending->cookies)/sizeof(pending->cookies[0]); ++i) I've found these two by looking for "sizeof" in the Kristian's patch that introduced pending_checkpoint. There can be more sizeofs added later.

if (!(pending_checkpoint= (pending_cookies *)my_malloc(size, MYF(MY_ZEROFILL)))) { diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc index fb11377..bfaf7d7 100644 --- a/sql/sys_vars.cc +++ b/sql/sys_vars.cc @@ -5202,3 +5202,14 @@ static Sys_var_mybool Sys_strict_password_validation( "that cannot be validated (passwords specified as a hash)", GLOBAL_VAR(strict_password_validation), CMD_LINE(OPT_ARG), DEFAULT(TRUE), NO_MUTEX_GUARD, NOT_IN_BINLOG); + +#ifdef HAVE_MMAP +static Sys_var_ulong Sys_log_tc_size( + "log_tc_size", + "Size of transaction coordinator log.", + READ_ONLY GLOBAL_VAR(opt_tc_log_size), + CMD_LINE(REQUIRED_ARG), + VALID_RANGE(my_getpagesize() * 6, ULONG_MAX), + DEFAULT(my_getpagesize() * 6), + BLOCK_SIZE(my_getpagesize()));

I think, min value could be my_getpagesize()*3 ?

+#endif

Regards, Sergei