- developers - lists.mariadb.org

Re: [Maria-developers] b745ea489f0: MDEV-11170: MariaDB 10.2 cannot start on MySQL 5.7 datadir
by Sergei Golubchik 13 Feb '17

13 Feb '17

Hi, Vicentiu! Pretty nice. I like the how it looks now. Thanks! Few small comments below, really minor. On Feb 13, vicentiu wrote: > revision-id: b745ea489f0698c328383d9b8ec20d2f9777b1b8 (mariadb-10.2.3-197-gb745ea489f0) > parent(s): 52e0b585aaedd0f8f7191e0c0768d4be3c1c3496 > author: Vicențiu Ciorbaru > committer: Vicențiu Ciorbaru > timestamp: 2017-02-13 12:13:36 +0200 > message: > > MDEV-11170: MariaDB 10.2 cannot start on MySQL 5.7 datadir > > PART 1 of the fix requires a bit of refactoring to not use hard-coded > field indices any more. Create classes that express the grant tables structure, > without exposing the underlying field indices. > > Most of the code is converted to use these classes, except parts which > are not directly affected by the MDEV-11170. These however are TODO > items for subsequent refactoring. > > --- > sql/sql_acl.cc | 1457 ++++++++++++++++++++++++++++++++++++++------------------ > sql/structs.h | 2 +- > 2 files changed, 996 insertions(+), 463 deletions(-) > > diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc > index b8a06c22b62..568aff9ac92 100644 > --- a/sql/sql_acl.cc > +++ b/sql/sql_acl.cc > +#ifdef HAVE_REPLICATION > + if (lock_type >= TL_WRITE_ALLOW_WRITE && thd->slave_thread && !thd->spcont) > + { > + /* > + GRANT and REVOKE are applied the slave in/exclusion rules as they are > + some kind of updates to the mysql.% tables. > + */ > + // TODO(remove-after-review) Note: there was a bug here in previous > + // open_grant_tables version. The linking of tables to be opened was > + // done according to a mask, starting from the end of the TABLES_LIST[TABLES_MAX] > + // array. rpl_filter->tables_ok(0, tables) would always start from > + // TABLES_LIST[0] and lookup the chain from there. If we would not open > + // the User table (first table in the array) > + // (as it happened in grant_reload()), the chain would stop there, hence > + // the rpl_filter check would not lookup all the tables opened I see. Right, it's a bug. > + // > + // This is the reason for the first_table_in_list member variable. > + Rpl_filter *rpl_filter= thd->system_thread_info.rpl_sql_info->rpl_filter; > + if (rpl_filter->is_on() && > + !rpl_filter->tables_ok(0, &first_table_in_list->tl)) > + DBUG_RETURN(1); > + } > +#endif > + if (open_and_lock_tables(thd, &first_table_in_list->tl, FALSE, > + MYSQL_LOCK_IGNORE_TIMEOUT)) > + DBUG_RETURN(-1); > + > + /* The privilge columns vary based on MariaDB version. Figure out > + how many we have after we've opened the table. */ > + if (which_tables & Table_user) > + m_user_table.compute_num_privilege_cols(); > + if (which_tables & Table_db) > + m_db_table.compute_num_privilege_cols(); > + if (which_tables & Table_tables_priv) > + m_tables_priv_table.compute_num_privilege_cols(); > + if (which_tables & Table_columns_priv) > + m_columns_priv_table.compute_num_privilege_cols(); > + if (which_tables & Table_host) > + m_host_table.compute_num_privilege_cols(); > + if (which_tables & Table_procs_priv) > + m_procs_priv_table.compute_num_privilege_cols(); > + if (which_tables & Table_proxies_priv) > + m_proxies_priv_table.compute_num_privilege_cols(); > + if (which_tables & Table_roles_mapping) > + m_roles_mapping_table.compute_num_privilege_cols(); couldn't you just walk the list (from first_table_in_list) and do compute_num_privilege_cols() on every table? Like ((Grant_tables*)tl)->compute_num_privilege_cols(); > + DBUG_RETURN(0); > + } > + > + inline const User_table& user_table() const > + { > + return m_user_table; > + } I personally would just make user_table/etc members public, instead of creating accessors like that. > + > + inline const Db_table& db_table() const > + { > + return m_db_table; > + } > + > + > + inline const Tables_priv_table& tables_priv_table() const > + { > + return m_tables_priv_table; > + } > + > + inline const Columns_priv_table& columns_priv_table() const > + { > + return m_columns_priv_table; > + } > + > + inline const Host_table& host_table() const > + { > + return m_host_table; > + } > + > + inline const Procs_priv_table& procs_priv_table() const > + { > + return m_procs_priv_table; > + } > + > + inline const Proxies_priv_table& proxies_priv_table() const > + { > + return m_proxies_priv_table; > + } > + > + inline const Roles_mapping_table& roles_mapping_table() const > + { > + return m_roles_mapping_table; > + } > + > + private: > + User_table m_user_table; > + Db_table m_db_table; > + Tables_priv_table m_tables_priv_table; > + Columns_priv_table m_columns_priv_table; > + Host_table m_host_table; > + Procs_priv_table m_procs_priv_table; > + Proxies_priv_table m_proxies_priv_table; > + Roles_mapping_table m_roles_mapping_table; > + > + /* Bitmap of which tables we want to open. */ > + const int which_tables; you don't need it if you walk the list in open_and_lock() > + /* Type of lock we want to acquire for the tables we are oppening. */ > + const enum thr_lock_type lock_type; not needed here, it's stored in every TABLE_LIST anyway > + /* The grant tables are set-up in a linked list. We keep the head of it. */ > + Grant_table_base *first_table_in_list; > + /** > + Chain two grant tables' TABLE_LIST members. > + */ > + static void link_tables(Grant_table_base *from, Grant_table_base *to) > + { > + DBUG_ASSERT(from); > + if (to) > + from->tl.next_local= from->tl.next_global= &to->tl; > + else > + from->tl.next_local= from->tl.next_global= NULL; > + } > +}; Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] Server crashes in inline_mysql_mutex_lock
by Michael Widenius 10 Feb '17

10 Feb '17

Hi! > Thread 1 (Thread 0x7fa0f7549700 (LWP 21346)): > #0 0x00007fa0f9511101 in __pthread_kill (threadid=<optimized out>, > signo=11) at ../nptl/sysdeps/unix/sysv/linux/pthread_kill.c:61 > #1 0x00000000013f4d28 in my_write_core (sig=11) at > /data/src/bb-10.0-monty/mysys/stacktrace.c:478 > #2 0x00000000009f6fc3 in handle_fatal_signal (sig=11) at > /data/src/bb-10.0-monty/sql/signal_handler.cc:285 > #3 <signal handler called> > #4 0x000000000093123c in inline_mysql_mutex_lock (that=0x0, > src_file=0x14df108 "/data/src/bb-10.0-monty/sql/rpl_parallel.cc", > src_line=1278) at > /data/src/bb-10.0-monty/include/mysql/psi/mysql_thread.h:662 > #5 0x0000000000936842 in handle_rpl_parallel_thread (arg=0x7fa0df2d3ee8) at > /data/src/bb-10.0-monty/sql/rpl_parallel.cc:1278 > #6 0x00007fa0f950c0a4 in start_thread (arg=0x7fa0f7549700) at > pthread_create.c:309 > #7 0x00007fa0f76c487d in clone () at > ../sysdeps/unix/sysv/linux/x86_64/clone.S:111 The problem is flush tables with read locks combined with a stop slave of the only replication thread that forces a resize of thread pool. This can get the replication thread into a wrong state where it tries to delete itself before waiting for the flush tables, which the thread pool doesn't like. Fixed by adding a check that we are not in flush tables with read lock when we delete ourselves from the pool. Regards, Monty

1 0

Re: [Maria-developers] [Commits] fb49b98: MDEV-10436 non-deterministic vcol does not force rbr
by Sergei Golubchik 10 Feb '17

10 Feb '17

Hi, Sachin! On Jan 24, sachin wrote: > revision-id: fb49b98a85dc28846c1fdd893ff9791803873aad (mariadb-10.2.3-91-gfb49b98) > parent(s): 0d107a85b3dd6969e66cc9cb4bd29e1cc92a7d18 > author: Sachin Setiya > committer: Sachin Setiya > timestamp: 2017-01-24 17:58:23 +0530 > message: > > MDEV-10436 non-deterministic vcol does not force rbr > diff --git a/sql/sql_class.cc b/sql/sql_class.cc > index f78382d..50d767d 100644 > --- a/sql/sql_class.cc > +++ b/sql/sql_class.cc > @@ -5824,6 +5826,16 @@ int THD::decide_logging_format(TABLE_LIST *tables) > table->table->file->ht) > multi_access_engine= TRUE; > > + /* > + Loop through each table field if it is virtual field check if it is deterministic > + or not. > + */ > + for (field_ptr= table->table->field; *field_ptr; field_ptr++) > + { > + field= *field_ptr; > + if (field->vcol_info && field->vcol_info->flags & VCOL_NOT_STRICTLY_DETERMINISTIC) > + lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_NON_DETER_VCOL); > + } > prev_access_table= table->table; > } 1. don't iterate table->table->field, use table->table->vfield - it only contains generated columns. 2. your check is too strong, in INSERT t1 VALUES (...) it does not matter if some of virtual columns are not strictly deterministic, so BINLOG_STMT_UNSAFE_NON_DETER_VCOL should not be raised. It only should be used when such a table is selected from (or in a WHERE condition of an UPDATE or DELETE). And only when such a column is used in a statement. There is nothing non-deterministing in INSERT t1 SELECT 1 FROM vcol_table 3. It might be not possible to do a per-column check, if read_set bitmaps are not ready yet. In that case, ok, do it per table, but then the result for the table is always the same, I mean where a table has not strictly deterministic columns is a property of a table. So, don't calculate it every time here, do it once and store in the TABLE_SHARE structure. Much like non_determinstic_insert is. But I'm removing non_determinstic_insert now (it's always false, dead code) so add another flag. Or - better - add a 'uint vcol_flags' member which is OR of vcol_info->flag for all fields. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 1

[Maria-developers] Doublewrite block number in XtraDB
by Hank Lyu 10 Feb '17

10 Feb '17

Hello: In XtraDB, if we adjust the size of doublewrite buffer and relative variable (i.e. srv_doublewrite_batch_size) , in theory, we can get better throughput when buffer pool flush to disk. I wonder that why doublewrite buffer size is 2 block and each flush page number is 120 (decided by srv_doublewrite_batch_size), instead doublewrite size is 8 and each flush page number is 500 or more? Is it worry about that when doing flush will occupy too much resource or other reason? Best Regard Hank Lyu

3 4

Re: [Maria-developers] [Commits] 15fde78: MDEV-10731: Wrong NULL match results in "Subquery returns more than 1 row" (error code 1242)
by Sergey Petrunia 09 Feb '17

09 Feb '17

Hi Varun, On Thu, Feb 09, 2017 at 10:37:17PM +0530, Varun wrote: > revision-id: 15fde78ce812c8db91340936941b986fb28ce1ad (mariadb-10.1.20-82-g15fde78) > parent(s): 99b2de92c6214ddd73eba35c935f490eedf87a26 > author: Varun Gupta > committer: Varun Gupta > timestamp: 2017-02-09 22:36:03 +0530 > message: > > MDEV-10731: Wrong NULL match results in "Subquery returns more than 1 row" (error code 1242) > > Post review fixes As far as I understand it's not Post-review fixes, it's a complete patch. Please use an appropriate comment. > > --- > mysql-test/r/update_innodb.result | 26 ++++++++++++++++++++++++++ > mysql-test/t/update_innodb.test | 26 ++++++++++++++++++++++++++ > sql/sql_select.cc | 19 +++++++++++-------- > 3 files changed, 63 insertions(+), 8 deletions(-) > > diff --git a/mysql-test/r/update_innodb.result b/mysql-test/r/update_innodb.result > index 88c86c5..9ba6699 100644 > --- a/mysql-test/r/update_innodb.result > +++ b/mysql-test/r/update_innodb.result > @@ -29,3 +29,29 @@ CREATE ALGORITHM=UNDEFINED VIEW `v1` AS select `t4`.`c1` AS `c1`,`t4`.`c2` AS `c > UPDATE t1 a JOIN t2 b ON a.c1 = b.c1 JOIN v1 vw ON b.c2 = vw.c1 JOIN t3 del ON vw.c2 = del.c2 SET a.c2 = ( SELECT max(t.c1) FROM t3 t, v1 i WHERE del.c2 = t.c2 AND vw.c3 = i.c3 AND t.c3 = 4 ) WHERE a.c2 IS NULL OR a.c2 < '2011-05-01'; > drop view v1; > drop table t1,t2,t3,t4; > +# > +# MDEV-10232 Scalar result of subquery changes after adding an outer select stmt > +# > +CREATE TABLE t1 ( > +a_id INT(20) UNSIGNED NOT NULL AUTO_INCREMENT, > +b_id INT(20) UNSIGNED NULL DEFAULT NULL, > +c_id VARCHAR(255) NULL DEFAULT NULL, > +PRIMARY KEY (a_id))COLLATE = 'utf8_general_ci' ENGINE = InnoDB; > +CREATE TABLE t2 ( > +b_id INT(20) UNSIGNED NOT NULL AUTO_INCREMENT, > +c_id VARCHAR(255) NULL DEFAULT NULL, > +PRIMARY KEY (b_id), > +INDEX idx_c_id (c_id))COLLATE = 'utf8_general_ci' ENGINE = InnoDB; > +INSERT INTO t1 (b_id, c_id) VALUES (NULL, NULL); > +INSERT INTO t2 (c_id) VALUES (NULL); > +INSERT INTO t2 (c_id) VALUES (NULL); > +SELECT * FROM T1; > +a_id b_id c_id > +1 NULL NULL > +SELECT t2.b_id FROM t1,t2 WHERE t2.c_id = t1.c_id; > +b_id > +UPDATE t1 SET b_id = (SELECT t2.b_id FROM t2 t2 WHERE t2.c_id = t1.c_id); > +SELECT * FROM T1; > +a_id b_id c_id > +1 NULL NULL > +drop table t1,t2; > diff --git a/mysql-test/t/update_innodb.test b/mysql-test/t/update_innodb.test > index 67c356c..d4fb8b7 100644 > --- a/mysql-test/t/update_innodb.test > +++ b/mysql-test/t/update_innodb.test > @@ -37,3 +37,29 @@ UPDATE t1 a JOIN t2 b ON a.c1 = b.c1 JOIN v1 vw ON b.c2 = vw.c1 JOIN t3 del ON v > > drop view v1; > drop table t1,t2,t3,t4; > + > +--echo # > +--echo # MDEV-10232 Scalar result of subquery changes after adding an outer select stmt > +--echo # > + > +CREATE TABLE t1 ( > + a_id INT(20) UNSIGNED NOT NULL AUTO_INCREMENT, > + b_id INT(20) UNSIGNED NULL DEFAULT NULL, > + c_id VARCHAR(255) NULL DEFAULT NULL, > + PRIMARY KEY (a_id))COLLATE = 'utf8_general_ci' ENGINE = InnoDB; > + > +CREATE TABLE t2 ( > + b_id INT(20) UNSIGNED NOT NULL AUTO_INCREMENT, > + c_id VARCHAR(255) NULL DEFAULT NULL, > + PRIMARY KEY (b_id), > + INDEX idx_c_id (c_id))COLLATE = 'utf8_general_ci' ENGINE = InnoDB; > + > +INSERT INTO t1 (b_id, c_id) VALUES (NULL, NULL); > +INSERT INTO t2 (c_id) VALUES (NULL); > +INSERT INTO t2 (c_id) VALUES (NULL); > + > +SELECT * FROM T1; > +SELECT t2.b_id FROM t1,t2 WHERE t2.c_id = t1.c_id; > +UPDATE t1 SET b_id = (SELECT t2.b_id FROM t2 t2 WHERE t2.c_id = t1.c_id); > +SELECT * FROM T1; > +drop table t1,t2; > diff --git a/sql/sql_select.cc b/sql/sql_select.cc > index e37c1b8..bc9a80f 100644 > --- a/sql/sql_select.cc > +++ b/sql/sql_select.cc > @@ -9367,8 +9367,6 @@ static void add_not_null_conds(JOIN *join) > UPDATE t1 SET t1.f2=(SELECT MAX(t2.f4) FROM t2 WHERE t2.f3=t1.f1); > not_null_item is the t1.f1, but it's referred_tab is 0. > */ > - if (!referred_tab) > - continue; > if (!(notnull= new (join->thd->mem_root) > Item_func_isnotnull(join->thd, item))) > DBUG_VOID_RETURN; > @@ -9380,16 +9378,21 @@ static void add_not_null_conds(JOIN *join) > */ > if (notnull->fix_fields(join->thd, &notnull)) > DBUG_VOID_RETURN; > - DBUG_EXECUTE("where",print_where(notnull, > - referred_tab->table->alias.c_ptr(), > - QT_ORDINARY);); > + > + if(referred_tab) > + DBUG_EXECUTE("where",print_where(notnull, > + referred_tab->table->alias.c_ptr(), > + QT_ORDINARY);); > + else > + DBUG_EXECUTE("where",print_where(notnull, > + "outer_ref_cond",QT_ORDINARY);); Please take the re-factoring further and have one DBUG_EXECUTE call with referred_tab? referred_tab->table->alias.c_ptr(): "outer_ref_cond" expression as the parameter. > if (!tab->first_inner) > - { > - COND *new_cond= referred_tab->join == join ? > + { Does the above break indentation or it has a tab character? > + COND *new_cond= (referred_tab && referred_tab->join == join) ? > referred_tab->select_cond : > join->outer_ref_cond; > add_cond_and_fix(join->thd, &new_cond, notnull); > - if (referred_tab->join == join) > + if (referred_tab && referred_tab->join == join) > referred_tab->set_select_cond(new_cond, __LINE__); > else > join->outer_ref_cond= new_cond; OK to push after the above is addressed. BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

[Maria-developers] bb-10.2-compatibility
by jerome brauge 09 Feb '17

09 Feb '17

Hello, I come back with a question on functions. On Oracle, there is only one difference between a function and a stored procedure : the return code. On Mariadb, stored functions are more limited : - Recursive stored functions are not allowed - Explicit or implicit commit is not allowed in stored function - in out parameter are not allowed (MDEV-10654) - no dynamic sql (execute immediate) Have you planned to remove these limits? If not, we'll have to find the best workarounds to use stored procedures (and find a solution to mimic a return code) Best regards, Jérôme.

2 3

Re: [Maria-developers] [Commits] cb918231a25: MDEV-11582 InnoDB: Failing assertion: !((field)->vcol_info && !(field)->stored_in_db())
by Alexander Barkov 09 Feb '17

09 Feb '17

Hello Sergei, The patch looks fine. I suggest one improvement. Currently SERIAL can have various redundant, ignored, non-applicable attributes, e.g.: CREATE TABLE t1 (a SERIAL NULL); CREATE TABLE t1 (a SERIAL AUTO_INCREMENT); CREATE TABLE t1 (a SERIAL SERIAL DEFAULT VALUE); CREATE OR REPLACE TABLE t1 (a SERIAL COLLATE `binary`); Can we split the "attribute:" rule to disallow these for SERIAL? Thanks! On 02/09/2017 03:06 PM, serg(a)mariadb.org wrote: > revision-id: cb918231a25ee792749d7e64ebc97e19c0faaf09 (mariadb-10.2.3-192-gcb918231a25) > parent(s): d1cc58f55ad516453aed42b65d71810e1392f745 > author: Sergei Golubchik > committer: Sergei Golubchik > timestamp: 2017-02-09 12:06:15 +0100 > message: > > MDEV-11582 InnoDB: Failing assertion: !((field)->vcol_info && !(field)->stored_in_db()) > > change the parser not to allow SERIAL as a normal data type. > make a special rule for it, where it could be used for define > fields, but not generated fields, not return type of a stored function, etc. > > --- > mysql-test/r/parser.result | 12 ++++++++++++ > mysql-test/t/parser.test | 11 +++++++++++ > sql/sql_yacc.yy | 22 ++++++++++++++-------- > 3 files changed, 37 insertions(+), 8 deletions(-) > > diff --git a/mysql-test/r/parser.result b/mysql-test/r/parser.result > index 7a0a8667350..1292b0bd2e5 100644 > --- a/mysql-test/r/parser.result > +++ b/mysql-test/r/parser.result > @@ -1289,3 +1289,15 @@ SELECT 1; > ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'UNION > SELECT 1' at line 2 > DROP TABLE t1; > +create table t1 (i int, vc serial as (i)); > +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'as (i))' at line 1 > +create function fs() returns serial return 1; > +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'serial return 1' at line 1 > +create table t1 ( id serial ); > +show create table t1; > +Table Create Table > +t1 CREATE TABLE `t1` ( > + `id` bigint(20) unsigned NOT NULL AUTO_INCREMENT, > + UNIQUE KEY `id` (`id`) > +) ENGINE=MyISAM DEFAULT CHARSET=latin1 > +drop table t1; > diff --git a/mysql-test/t/parser.test b/mysql-test/t/parser.test > index 1fa7df7fc3d..dc538db6073 100644 > --- a/mysql-test/t/parser.test > +++ b/mysql-test/t/parser.test > @@ -1321,3 +1321,14 @@ UNION > SELECT 1; > > DROP TABLE t1; > + > +# > +# MDEV-11582 InnoDB: Failing assertion: !((field)->vcol_info && !(field)->stored_in_db()) > +# > +--error ER_PARSE_ERROR > +create table t1 (i int, vc serial as (i)); > +--error ER_PARSE_ERROR > +create function fs() returns serial return 1; > +create table t1 ( id serial ); > +show create table t1; > +drop table t1; > diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy > index 12423e71f1f..db22708db43 100644 > --- a/sql/sql_yacc.yy > +++ b/sql/sql_yacc.yy > @@ -6145,8 +6145,7 @@ field_spec: > lex->init_last_field(f, $1.str, NULL); > $<create_field>$= f; > } > - field_type { Lex->set_last_field_type($3); } > - field_def > + field_type_or_serial > { > LEX *lex=Lex; > $$= $<create_field>2; > @@ -6164,6 +6163,19 @@ field_spec: > } > ; > > +field_type_or_serial: > + field_type { Lex->set_last_field_type($1); } field_def > + | SERIAL_SYM > + { > + Lex_field_type_st type; > + type.set(MYSQL_TYPE_LONGLONG); > + Lex->set_last_field_type(type); > + Lex->last_field->flags|= AUTO_INCREMENT_FLAG | NOT_NULL_FLAG > + | UNSIGNED_FLAG | UNIQUE_KEY_FLAG; > + } > + opt_attribute > + ; > + > field_def: > opt_attribute > | opt_generated_always AS virtual_column_func > @@ -6441,12 +6453,6 @@ field_type: > { $$.set(MYSQL_TYPE_SET); } > | LONG_SYM opt_binary > { $$.set(MYSQL_TYPE_MEDIUM_BLOB); } > - | SERIAL_SYM > - { > - $$.set(MYSQL_TYPE_LONGLONG); > - Lex->last_field->flags|= (AUTO_INCREMENT_FLAG | NOT_NULL_FLAG | UNSIGNED_FLAG | > - UNIQUE_KEY_FLAG); > - } > ; > > spatial_type: > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits >

1 0

[Maria-developers] Archive Storage Engine
by Varun Gupta 08 Feb '17

08 Feb '17

Hi, Here is the issue https://jira.mariadb.org/browse/MDEV-11645 on which I need suggestions The observations made are: - in 10.1, varchar endspace was bzero-ed archive depended on it to not pack garbage - in 10.2, varchar endspace is not bzero-ed which breaks the archive SE. Was this change intended? If yes, then we should update ha_archive to fill the garbage parts of the buffer with zeroes that will cause it to be (slightly?) slower but there's no other way. BR, Varun

3 4

Re: [Maria-developers] [Commits] e84a5064674: After review fixes for MDEV-11759.
by Marko Mäkelä 08 Feb '17

08 Feb '17

On Tue, Feb 7, 2017 at 8:12 PM, jan <jan.lindstrom(a)mariadb.com> wrote: > # Write file to make mysql-test-run.pl expect the "crash", but don't > # start it until it's told to > -# We give 30 seconds to do a clean shutdown because we do not want > -# to redo apply the pages of t1.ibd at the time of recovery. > -# We want SQL to initiate the first access to t1.ibd. > # Wait until disconnected. Remove all these messages. > diff --git a/storage/innobase/fil/fil0crypt.cc b/storage/innobase/fil/fil0crypt.cc > index fed1cf94d84..4299600709a 100644 > --- a/storage/innobase/fil/fil0crypt.cc > +++ b/storage/innobase/fil/fil0crypt.cc > @@ -982,7 +982,16 @@ fil_space_verify_crypt_checksum( > /* Declare empty pages non-corrupted */ > if (checksum == 0 > && *reinterpret_cast<const ib_uint64_t*>(page + FIL_PAGE_LSN) == 0) { > - return (true); > + /* make sure that the page is really empty */ > + for (ulint i = 0; i < UNIV_PAGE_SIZE; i++) { > + if (page[i] != 0) { > + ib_logf(IB_LOG_LEVEL_INFO, > + "Checksum fields zero but page is not empty."); > + > + return(false); > + } > + } > + return(true); > } Please use buf_page_is_zeroes() instead of duplicating the code. Do not spam the error log if the checksum happens to be 0. I guess we could also have FIL_PAGE_LSN at 0 when importing a file. > @@ -1016,16 +1025,28 @@ fil_space_verify_crypt_checksum( > bool encrypted = (checksum == cchecksum1 || checksum == cchecksum2 > || checksum == BUF_NO_CHECKSUM_MAGIC); > > - /* Old InnoDB versions did not initialize > - FIL_PAGE_FILE_FLUSH_LSN field so there could be garbage > - and above checksum check could produce false positive. > - Thus we also check does the traditional stored > - checksum fields match the calculated one. Both of these > - could naturally produce false positive but then > - we just decrypt the page and after that corrupted > - pages very probable stay corrupted and valid > - pages very probable stay valid. > + /* MySQL 5.6 and MariaDB 10.0 and 10.1 will write an LSN to the > + first page of each system tablespace file at > + FIL_PAGE_FILE_FLUSH_LSN offset. On other pages and in other files, > + the field might have been uninitialized until MySQL 5.5. In MySQL 5.7 > + (and MariaDB Server 10.2.2) WL#7990 stopped writing the field for other > + than page 0 of the system tablespace. I think that we should conduct some research to make the comments more useful and reliable. Looking at the MySQL 3.23.49 source code, I see that we indeed did not initialize FIL_PAGE_FILE_FLUSH_LSN (or FIL_PAGE_ARCH_LOG_NO for that matter) in the system tablespace. The latter field was repurposed as SPACE_ID later. This means that the system tablespace (which was the only option before MySQL 4.1) can contain garbage in the FIL_PAGE_FILE_FLUSH_LSN field, except if we can infer from some other attributes of the system tablespace that it was created by MySQL 5.5 or later. The next interesting MySQL version to check is 4.1.2 where innodb_file_per_table was introduced. The oldest version in the 4.1 series that I was able to find is 4.1.21. It indeed does not make any effort to initialize FIL_PAGE_FILE_FLUSH_LSN on other pages than the first one. Because the buffer pool can be used for general-purpose memory allocations (BUF_BLOCK_MEMORY), it is possible that the FLUSH_LSN field will be written as nonzero garbage, even if we were able to assume that the initially allocated memory was zero-initialized by the operating system. So, the above comment is correct. > + Starting from MariaDB 10.1 the fiedl has been repurposed for > + encryption key_version. Correct the typo "fiedl". > + > + Starting with MySQL 5.7 (and MariaDB Server 10.2), the > + field has been repurposed for SPATIAL INDEX pages. Refer to the field name FIL_RTREE_SPLIT_SEQ_NUM in the comment, so that it can be found easily by grep. > + > + Note that FIL_PAGE_FILE_FLUSH_LSN is not included in the InnoDB page > + checksum. > + > + Thus,FIL_PAGE_FILE_FLUSH_LSN could contain uninitialized value, 0, > + correct key_version or spatial index information, thus we also > + verify page using traditional checksum check to detect if page > + would be valid but not encrypted. > */ Well, 10.1 cannot handle spatial index information. I would simply say: "Thus, FIL_PAGE_FILE_FLUSH_LSN could contain any value. While the field would usually be 0 for pages that are not encrypted, we cannot assume that a nonzero value means that the page is encrypted. Therefore we must validate the page both as encrypted and unencrypted when FIL_PAGE_FILE_FLUSH_LSN does not contain 0." OK to push with these changes. Note: I should still go through the whole logic, to be able to say if we really fixed the issues. I currently believe that more work is needed, but that should be tracked as a separate issue. Marko -- DON’T MISS M|17 April 11 - 12, 2017 The Conrad Hotel New York City https://m17.mariadb.com/ Marko Mäkelä, Lead Developer InnoDB MariaDB Corporation

1 0

Re: [Maria-developers] [Commits] 3a8f101: MDEV-11379 - AliSQL: [Feature] Issue#8: SELECT FOR UPDATE WAIT
by Michael Widenius 08 Feb '17

08 Feb '17

Hi! >>>>> "Sergey" == Sergey Vojtovich <svoj(a)mariadb.org> writes: Sergey> revision-id: 3a8f101a173b987b314ba5fcc5c74d80ab56802b (mariadb-10.2.2-160-g3a8f101) Sergey> parent(s): c846ebe9df9adc90a5c25be0dce816c5d4302794 Sergey> committer: Sergey Vojtovich Sergey> timestamp: 2017-02-07 13:27:42 +0400 Sergey> message: Sergey> MDEV-11379 - AliSQL: [Feature] Issue#8: SELECT FOR UPDATE WAIT Sergey> Extended syntax so that it is now possible to set lock_wait_timeout for the Sergey> following statements: Sergey> SELECT ... FOR UPDATE [WAIT n|NOWAIT] Sergey> SELECT ... LOCK IN SHARED MODE [WAIT n|NOWAIT] Sergey> LOCK TABLE ... [WAIT n|NOWAIT] Sergey> This is amended AliSQL patch. We prefer Oracle syntax for [WAIT n|NOWAIT] Sergey> instead of original [WAIT [n]|NO_WAIT]. Agree it's better to use a syntax that someone else is using. Sergey> diff --git a/include/mysql/plugin.h b/include/mysql/plugin.h Sergey> index b3c71c65..b0a5849 100644 Sergey> --- a/include/mysql/plugin.h Sergey> +++ b/include/mysql/plugin.h Sergey> @@ -633,6 +633,7 @@ void thd_storage_lock_wait(MYSQL_THD thd, long long value); Sergey> int thd_tx_isolation(const MYSQL_THD thd); Sergey> int thd_tx_is_read_only(const MYSQL_THD thd); Sergey> int thd_rpl_is_parallel(const MYSQL_THD thd); Sergey> +unsigned long thd_lex_lock_wait_timeout(const MYSQL_THD thd); Sergey> /** Sergey> Create a temporary file. <cut> Sergey> +++ b/sql/sql_base.cc Sergey> @@ -2723,13 +2723,18 @@ Open_table_context::Open_table_context(THD *thd, uint flags) Sergey> :m_thd(thd), Sergey> m_failed_table(NULL), Sergey> m_start_of_statement_svp(thd->mdl_context.mdl_savepoint()), Sergey> - m_timeout(flags & MYSQL_LOCK_IGNORE_TIMEOUT ? Sergey> - LONG_TIMEOUT : thd->variables.lock_wait_timeout), Sergey> m_flags(flags), Sergey> m_action(OT_NO_ACTION), Sergey> m_has_locks(thd->mdl_context.has_locks()), Sergey> m_has_protection_against_grl(FALSE) Sergey> -{} Sergey> +{ Sergey> + if (flags & MYSQL_LOCK_IGNORE_TIMEOUT) Sergey> + m_timeout= LONG_TIMEOUT; Sergey> + else if (thd->lex->lock_wait_timeout == ULONG_MAX) Sergey> + m_timeout= thd->variables.lock_wait_timeout; Sergey> + else Sergey> + m_timeout= thd->lex->lock_wait_timeout; Sergey> +} Instead of adding thd->lex->lock_wait_timeout I suggest that we instead use the existing facititliy of SET STATEMENT innodb_lock_wait_timeout=xxx SELECT This has the following benefits: - We only need one innodb timeout variable, not two - Probably simpler code. Very few changes needed in InnoDB - One will get exactly same behaviour independent of the syntax that is used This would change the task to be implemented the following wat: - If WAIT|NOWAIT is used, change in sql_yacc.yy to use the SET STATEMENT facility to set innodb_lock_wait_timeout and lock_wait_timeout - Add support for nowait (timeout == 0 would mean nowait) I agree that having the new syntax is good as it's used by other databases and simpler than having to set two variables. Regards, Monty

1 0