- developers - lists.mariadb.org

[Maria-developers] better execution plan possible with INNER JOIN of mroonga (or whatever storage engine) ?
by Kazuhiko Shiozaki 03 Dec '14

03 Dec '14

Hello ! I am investigating a slow query and have a question about execution plan with mroonga (possibly any storage engine?). (with MariaDB 10.0.15.) Here are tables : CREATE TABLE catalog ( uid bigint(20) unsigned NOT NULL, path varchar(255) COLLATE utf8_unicode_ci NOT NULL DEFAULT '', PRIMARY KEY (uid) ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci; CREATE TABLE catalog_full_text ( uid bigint(20) unsigned NOT NULL, title varchar(255) COLLATE utf8_unicode_ci DEFAULT '', PRIMARY KEY (uid), FULLTEXT KEY title (title) COMMENT 'parser "TokenBigramSplitSymbolAlphaDigit"' ) ENGINE=Mroonga DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci; CREATE TABLE category ( uid bigint(20) unsigned NOT NULL, category_uid bigint(20) unsigned NOT NULL DEFAULT '0', base_category_uid bigint(20) unsigned NOT NULL DEFAULT '0', PRIMARY KEY (uid,category_uid,base_category_uid) ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci; Here is the query : SELECT DISTINCT catalog.path, catalog.uid FROM ( ( catalog AS catalog INNER JOIN ( category AS category INNER JOIN catalog_full_text AS source_title_full_text ON category.base_category_uid = 18482 AND source_title_full_text.uid = category.category_uid ) ON category.uid = catalog.uid ) INNER JOIN catalog_full_text AS title_full_text ON title_full_text.uid = catalog.uid ) WHERE MATCH (source_title_full_text.title) AGAINST ('bar' IN BOOLEAN MODE) AND MATCH (title_full_text.title) AGAINST ('foo' IN BOOLEAN MODE) Here is the output of EXPLAIN : *************************** 1. row *************************** id: 1 select_type: SIMPLE table: source_title_full_text type: fulltext possible_keys: PRIMARY,title key: title key_len: 0 ref: rows: 1 Extra: Using where with pushed condition; Using temporary *************************** 2. row *************************** id: 1 select_type: SIMPLE table: title_full_text type: fulltext possible_keys: PRIMARY,title key: title key_len: 0 ref: rows: 1 Extra: Using where with pushed condition *************************** 3. row *************************** id: 1 select_type: SIMPLE table: catalog type: eq_ref possible_keys: PRIMARY key: PRIMARY key_len: 8 ref: test.title_full_text.uid rows: 1 Extra: *************************** 4. row *************************** id: 1 select_type: SIMPLE table: category type: eq_ref possible_keys: PRIMARY key: PRIMARY key_len: 24 ref: test.title_full_text.uid,test.source_title_full_text.uid,const rows: 1 Extra: Using index; Distinct As you see, we have two full-text queries at the beginning and both have 'rows = 1'. In a slow query case, 25k records mathces with the first full-text query and 1.6k records matches with the second full-text query. When I manually modify the order of INNER JOIN so that the 'lighter' full-text query appears first, EXPLAIN output is different as well and the performance is much improved. Accoding to Kohei, a Mroonga developer, we always have 'rows = 1' for mroonga full-text query because MariaDB does not request such value to mroonga (JOIN::get_examined_rows() in sql/sql_select.cc), and it is possible to modify mroonga to return an estimation of rows if MariaDB has such an API. So here are my questions : * can MariaDB request an 'estimation of rows' to mroonga (or whatever storage engine) ? * can such information help to decide a better execution plan with INNER JOIN like above ? Thanks in advance ! -- Kazuhiko Shiozaki, Nexedi SA Senior Consultant Nexedi: Consulting and Development of Free / Open Source Software http://www.nexedi.com/ ERP5: Full Featured High End Open Source ERP http://www.erp5.com/

3 3

[Maria-developers] Google's patches to MariaDB are available in the opensource repo
by Pavel Ivanov 03 Dec '14

03 Dec '14

Hi all, Effective immediately the full opensource tree of changes made to MariaDB by Google to make it work for our internal applications is available at https://code.google.com/p/google-mysql/. Note: I didn't update the main page on the site to reflect the changes yet, but the sources are there and the master branch points to the tip of MariaDB tree. Note #2: this tree is not an exact copy of our internal tree, and it's not intended to reflect verbatim our internal development process. It's just a set of changes split roughly one feature per commit with a grouping of several commits related to the same feature set into branches. That means that whenever we find a bug in some of our commits we'll squash the fix in the appropriate place rebuilding the whole git tree on top of that. As a consequence of that links to specific commits will not last forever, though links to specific branches likely will. Note #3: unfortunately code.google.com doesn't have a good visualization of branches in the git tree, so it's not easy to see which commits belong to which branch in our structure of branches stacked on top of each other. So the easiest way to see that is to clone the tree on your local disk and run gitk on it (or other tool that you find more convenient). Besides raising a general awareness of existence of our opensource tree, I'd like to draw attention of MariaDB developers especially to the following branches. I think many commits from there can be considered for inclusion into MariaDB proper. 16-encryption https://code.google.com/p/google-mysql/source/list?name=mariadb-10.0.12%2F1… Encryption of different subsystems writing to disk (not all subsystems included yet, but will be soon). 09-rpl https://code.google.com/p/google-mysql/source/list?name=mariadb-10.0.12%2F0… Replication-related fixes and improvements. 01-base https://code.google.com/p/google-mysql/source/list?name=mariadb-10.0.12%2F0… Mostly basic compilation and testing fixes. 03-privileges https://code.google.com/p/google-mysql/source/list?name=mariadb-10.0.12%2F0… Fixes for some quirks in privilege subsystem (aka sql_acl.cc). If you want to discuss any changes that you see in the tree feel free to reply to this email or start a separate email thread. Thank you, Pavel

2 1

[Maria-developers] Some patches for the C client library
by Daniel Schepler 02 Dec '14

02 Dec '14

While I was creating Debian packages for the "MariaDB Client Library for C" I came across several minor issues that I corrected. I thought I'd post the patches here in case you might be interested in integrating them; I'd have no problem with contributing them under the MCA if that's necessary (i.e. if they're even big enough to require such a thing). Each one has a fairly complete description of its purpose at the start. I've also noticed that mariadb_config --libs prints out something like ... -l-lz if WITH_EXTERNAL_ZLIB=ON. However, for the Debian packages I instead patched mariadb_config to remove the indirect dependencies altogether, as we generally do, so I didn't write a fix for the -l-lz issue. Also, it seems that if I try to build with WITH_SQLITE=ON then I get a build error, so I haven't enabled that in the Debian packages. -- Daniel Schepler

2 2

[Maria-developers] crash, reporting bugs
by Roberto Spadim 02 Dec '14

02 Dec '14

hi guys, i have a consistent crash (mdev-7182) but this email is not about it... when i got the crash, i got this info: ... To report this bug, see http://kb.askmonty.org/en/reporting-bugs ... should we update this url to the current one? https://mariadb.com/kb/en/mariadb/development/debugging-mariadb/reporting-b… thanks -- Roberto Spadim SPAEmpresarial Eng. Automação e Controle

3 3

Re: [Maria-developers] [Commits] 020e569: MDEV-4412 : SLOW QUERY LOG - add affected rows (UPDATE / DELETE) in slow query log
by Sergei Golubchik 30 Nov '14

30 Nov '14

Hi, Nirbhay! On Nov 29, nirbhay(a)mariadb.com wrote: > revision-id: 020e569dc7f638b6add8a043b1c9ff308f688ffa > parent(s): 7b55b67de5fdfe91283357fe6c7ccc3f9e355925 > committer: Nirbhay Choubey > branch nick: b4412 > timestamp: 2014-11-29 19:05:32 -0500 > message: > > MDEV-4412 : SLOW QUERY LOG - add affected rows (UPDATE / DELETE) in slow query log > > Added Rows_affected to slow query log & mysql.slow_log table. > > diff --git a/scripts/mysql_system_tables_fix.sql b/scripts/mysql_system_tables_fix.sql > index b9bb59c..aeaf919 100644 > --- a/scripts/mysql_system_tables_fix.sql > +++ b/scripts/mysql_system_tables_fix.sql > @@ -249,7 +249,8 @@ SET GLOBAL general_log = @old_log_state; > SET @old_log_state = @@global.slow_query_log; > SET GLOBAL slow_query_log = 'OFF'; > ALTER TABLE slow_log > - ADD COLUMN thread_id BIGINT(21) UNSIGNED NOT NULL AFTER sql_text; > + ADD COLUMN thread_id BIGINT(21) UNSIGNED NOT NULL AFTER sql_text, > + ADD COLUMN rows_affected INTEGER NOT NULL AFTER thread_id; > ALTER TABLE slow_log > MODIFY start_time TIMESTAMP(6) NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, > MODIFY user_host MEDIUMTEXT NOT NULL, I am not sure it'll work. It would work if you need to upgrade a table that has neither thread_id nor rows_affected column to the latest version. So that ALTER would've been an acceptable solution if both thread_id and rows_affected were added in the same MariaDB version. But they were not and there are quite a few MariaDB releases where slow_log table has thread_id and doesn't have rows_affected. I suspect in that case ALTER will fail. But if you'd like you can try a small test case and see whether such an ALTER will add a second column even if the first one already exists. Better add rows_affected in a separate ALTER TABLE statement. Regards, Sergei

1 0

[Maria-developers] Patch: CREATE [[NO] FORCE] VIEW Options
by James Briggs 30 Nov '14

30 Nov '14

Hi List. Below is my patch that implements the CREATE [[NO] FORCE] VIEW options against MariaDB 10.1.0. Notes: - It's a fairly solid patch, but I think the best thing is to commit it to alpha and let it bake for a while. - it passes t/view.test: # ./mysql-test-run.pl view Logging: ./mysql-test-run.pl view vardir: /usr/local/mariadb-10.1.0/mysql-test/var MariaDB Version 10.1.0-MariaDB-debug TEST RESULT TIME (ms) or COMMENT -------------------------------------------------------------------------- main.view [ pass ] 1896 -------------------------------------------------------------------------- The servers were restarted 0 times Spent 1.896 of 7 seconds executing testcases Completed: All 1 tests were successful. - I wrote tests/view.pl which does 8,000 tests. It passes. (I will include it in a separate email.) - Please take a look at the style, and the 411 comments. Thanks, James Briggs. -- Available for MySQL/NoSQL DBA/Programming in San Jose area or remote. $ cat create_force_view.patch --- ../mariadb-10.1.0/sql/sql_view.h 2014-06-27 04:50:36.000000000 -0700 +++ sql/sql_view.h 2014-09-02 02:35:42.000000000 -0700 @@ -29,10 +29,10 @@ /* Function declarations */ bool create_view_precheck(THD *thd, TABLE_LIST *tables, TABLE_LIST *view, - enum_view_create_mode mode); + enum_view_create_mode mode, enum_view_create_force force); bool mysql_create_view(THD *thd, TABLE_LIST *view, - enum_view_create_mode mode); + enum_view_create_mode mode, enum_view_create_force force); bool mysql_make_view(THD *thd, File_parser *parser, TABLE_LIST *table, uint flags); --- ../mariadb-10.1.0/sql/sql_lex.h 2014-06-27 04:50:33.000000000 -0700 +++ sql/sql_lex.h 2014-09-02 01:21:10.000000000 -0700 @@ -170,6 +170,12 @@ VIEW_CREATE_OR_REPLACE // check only that there are not such table }; +enum enum_view_create_force +{ + VIEW_CREATE_NO_FORCE, // default - check that there are not such VIEW/table + VIEW_CREATE_FORCE, // check that there are not such VIEW/table, then ignore table object dependencies +}; + enum enum_drop_mode { DROP_DEFAULT, // mode is not specified @@ -2442,6 +2448,7 @@ }; enum enum_var_type option_type; enum enum_view_create_mode create_view_mode; + enum enum_view_create_force create_view_force; enum enum_drop_mode drop_mode; uint profile_query_id; --- ../mariadb-10.1.0/sql/sql_parse.cc 2014-06-27 04:50:34.000000000 -0700 +++ sql/sql_parse.cc 2014-09-02 02:34:31.000000000 -0700 @@ -4943,7 +4943,7 @@ Note: SQLCOM_CREATE_VIEW also handles 'ALTER VIEW' commands as specified through the thd->lex->create_view_mode flag. */ - res= mysql_create_view(thd, first_table, thd->lex->create_view_mode); + res= mysql_create_view(thd, first_table, thd->lex->create_view_mode, thd->lex->create_view_force); break; } case SQLCOM_DROP_VIEW: --- ../mariadb-10.1.0/sql/sql_yacc.yy 2014-06-27 04:50:37.000000000 -0700 +++ sql/sql_yacc.yy 2014-09-05 17:19:29.000000000 -0700 @@ -1851,7 +1851,7 @@ statement sp_suid sp_c_chistics sp_a_chistics sp_chistic sp_c_chistic xa opt_field_or_var_spec fields_or_vars opt_load_data_set_spec - view_algorithm view_or_trigger_or_sp_or_event + view_algorithm view_or_trigger_or_sp_or_event view_force_option definer_tail no_definer_tail view_suid view_tail view_list_opt view_list view_select view_check_option trigger_tail sp_tail sf_tail udf_tail event_tail @@ -2446,6 +2446,7 @@ VIEW_CREATE_OR_REPLACE); Lex->create_view_algorithm= DTYPE_ALGORITHM_UNDEFINED; Lex->create_view_suid= TRUE; + Lex->create_view_force= VIEW_CREATE_NO_FORCE; /* initialize just in case */ } view_or_trigger_or_sp_or_event { @@ -15887,6 +15888,15 @@ | event_tail ; +view_force_option: + /* empty */ /* 411 - is there a cleaner way of initializing here? */ + { Lex->create_view_force = VIEW_CREATE_NO_FORCE; } + | NO_SYM FORCE_SYM + { Lex->create_view_force = VIEW_CREATE_NO_FORCE; } + | FORCE_SYM + { Lex->create_view_force = VIEW_CREATE_FORCE; } + ; + /************************************************************************** DEFINER clause support. @@ -15944,7 +15954,7 @@ ; view_tail: - view_suid VIEW_SYM table_ident + view_suid view_force_option VIEW_SYM table_ident { LEX *lex= thd->lex; lex->sql_command= SQLCOM_CREATE_VIEW; --- ../mariadb-10.1.0/sql/sql_view.cc 2014-06-27 04:50:36.000000000 -0700 +++ sql/sql_view.cc 2014-09-05 19:33:58.000000000 -0700 @@ -248,7 +248,7 @@ */ bool create_view_precheck(THD *thd, TABLE_LIST *tables, TABLE_LIST *view, - enum_view_create_mode mode) + enum_view_create_mode mode, enum_view_create_force force) { LEX *lex= thd->lex; /* first table in list is target VIEW name => cut off it */ @@ -259,7 +259,7 @@ DBUG_ENTER("create_view_precheck"); /* - Privilege check for view creation: + Privilege check for view creation with default (NO FORCE): - user has CREATE VIEW privilege on view table - user has DROP privilege in case of ALTER VIEW or CREATE OR REPLACE VIEW @@ -272,6 +272,7 @@ checked that we have not more privileges on correspondent column of view table (i.e. user will not get some privileges by view creation) */ + if ((check_access(thd, CREATE_VIEW_ACL, view->db, &view->grant.privilege, &view->grant.m_internal, @@ -285,6 +286,11 @@ check_grant(thd, DROP_ACL, view, FALSE, 1, FALSE)))) goto err; + if (force) { + res = false; + DBUG_RETURN(res || thd->is_error()); + } + for (sl= select_lex; sl; sl= sl->next_select()) { for (tbl= sl->get_table_list(); tbl; tbl= tbl->next_local) @@ -369,7 +375,7 @@ #else bool create_view_precheck(THD *thd, TABLE_LIST *tables, TABLE_LIST *view, - enum_view_create_mode mode) + enum_view_create_mode mode, enum_view_create_force force) { return FALSE; } @@ -391,7 +397,7 @@ */ bool mysql_create_view(THD *thd, TABLE_LIST *views, - enum_view_create_mode mode) + enum_view_create_mode mode, enum_view_create_force force) { LEX *lex= thd->lex; bool link_to_local; @@ -425,14 +431,13 @@ goto err; } - if ((res= create_view_precheck(thd, tables, view, mode))) + if (res= create_view_precheck(thd, tables, view, mode, force)) goto err; lex->link_first_table_back(view, link_to_local); view->open_type= OT_BASE_ONLY; - if (open_temporary_tables(thd, lex->query_tables) || - open_and_lock_tables(thd, lex->query_tables, TRUE, 0)) + if (open_temporary_tables(thd, lex->query_tables) || (!force && open_and_lock_tables(thd, lex->query_tables, TRUE, 0))) { view= lex->unlink_first_table(&link_to_local); res= TRUE; @@ -513,6 +518,7 @@ } } +if (!force) { /* prepare select to resolve all fields */ lex->context_analysis_only|= CONTEXT_ANALYSIS_ONLY_VIEW; if (unit->prepare(thd, 0, 0)) @@ -612,6 +618,7 @@ } } #endif +} res= mysql_register_view(thd, view, mode); @@ -621,7 +628,7 @@ meta-data changes after ALTER VIEW. */ - if (!res) + // if (!res) + if (!res && !force) /* 411 - solves segfault problems with CREATE FORCE VIEW option sometimes */ tdc_remove_table(thd, TDC_RT_REMOVE_ALL, view->db, view->table_name, false); if (mysql_bin_log.is_open()) @@ -908,6 +915,8 @@ fn_format(path_buff, file.str, dir.str, "", MY_UNPACK_FILENAME); path.length= strlen(path_buff); if (ha_table_exists(thd, view->db, view->table_name, NULL)) { if (mode == VIEW_CREATE_NEW) --- ../mariadb-10.1.0/mysql-test/t/view.test 2014-06-27 04:50:30.000000000 -0700 +++ mysql-test/t/view.test 2014-09-06 00:23:32.000000000 -0700 @@ -5263,4 +5263,17 @@ --echo # ----------------------------------------------------------------- --echo # -- End of 10.0 tests. --echo # ----------------------------------------------------------------- + +create no force view v1 as select 1; +drop view if exists v1; + +create force view v1 as select 1; +drop view if exists v1; + +create force view v1 as select * from missing_base_table; +drop view if exists v1; + +--echo # ----------------------------------------------------------------- +--echo # -- End of 10.1 tests. +--echo # ----------------------------------------------------------------- SET optimizer_switch=@save_optimizer_switch;

2 2

Re: [Maria-developers] MDEV-4412 : SLOW QUERY LOG - add affected rows (UPDATE / DELETE) in slow query log
by Sergei Golubchik 30 Nov '14

30 Nov '14

Hi, Nirbhay! On Nov 28, nirbhay(a)mariadb.com wrote: > revision-id: 7a5559421f4f3dda44d04bf946e79661d09b4dc2 > parent(s): 7b55b67de5fdfe91283357fe6c7ccc3f9e355925 > committer: Nirbhay Choubey > branch nick: b4412 > timestamp: 2014-11-28 20:53:02 -0500 > message: > > MDEV-4412 : SLOW QUERY LOG - add affected rows (UPDATE / DELETE) in slow query log > > Added Rows_affected to slow query log & mysql.slow_log table. Just a couple of comments: > diff --git a/scripts/mysql_system_tables_fix.sql b/scripts/mysql_system_tables_fix.sql > index b9bb59c..4973dc9 100644 > --- a/scripts/mysql_system_tables_fix.sql > +++ b/scripts/mysql_system_tables_fix.sql > @@ -262,7 +262,8 @@ ALTER TABLE slow_log > MODIFY insert_id INTEGER NOT NULL, > MODIFY server_id INTEGER UNSIGNED NOT NULL, > MODIFY sql_text MEDIUMTEXT NOT NULL, > - MODIFY thread_id BIGINT(21) UNSIGNED NOT NULL; > + MODIFY thread_id BIGINT(21) UNSIGNED NOT NULL, > + MODIFY rows_affected INTEGER NOT NULL; Eh? That makes no sense. This file is for mysql_upgrade. Your ALTER TABLE would help if there was some MariaDB version where rows_affected column would've been nullable and you wanted to change it to NOT NULL. But there was no such MariaDB version. There was MariaDB version without rows_affected column at all. So, instead of this MODIFY rows_affected, you need to put another ALTER TABLE statement that adds rows_affected INTEGER NOT NULL column. > SET GLOBAL slow_query_log = @old_log_state; > > ALTER TABLE plugin > diff --git a/sql/log.cc b/sql/log.cc > index ff04105..24e5283 100644 > --- a/sql/log.cc > +++ b/sql/log.cc > @@ -2918,12 +2924,16 @@ bool MYSQL_QUERY_LOG::write(THD *thd, time_t current_time, > sprintf(lock_time_buff, "%.6f", ulonglong2double(lock_utime)/1000000.0); > if (my_b_printf(&log_file, > "# Thread_id: %lu Schema: %s QC_hit: %s\n" \ > - "# Query_time: %s Lock_time: %s Rows_sent: %lu Rows_examined: %lu\n", > + "# Query_time: %s Lock_time: %s Rows_sent: %lu Rows_examined: %lu\n" \ > + "# Rows_affected: %lu\n", I'd rather have Rows_affected on the same line with Rows_sent and Rows_examined > (ulong) thd->thread_id, (thd->db ? thd->db : ""), > ((thd->query_plan_flags & QPLAN_QC) ? "Yes" : "No"), > query_time_buff, lock_time_buff, > (ulong) thd->get_sent_row_count(), > - (ulong) thd->get_examined_row_count()) == (size_t) -1) > + (ulong) thd->get_examined_row_count(), > + thd->get_stmt_da()->is_ok() ? > + (ulong) thd->get_stmt_da()->affected_rows() : > + 0) == (size_t) -1) > tmp_errno= errno; > if ((thd->variables.log_slow_verbosity & LOG_SLOW_VERBOSITY_QUERY_PLAN) && > (thd->query_plan_flags & Regards, Sergei

2 1

Re: [Maria-developers] [Commits] Rev 4484: MDEV-4255 MIXED replication: Unique key updates are not marked unsafe, replication fails in lp:~maria-captains/maria/10.0
by Kristian Nielsen 29 Nov '14

29 Nov '14

Sergei Golubchik <serg(a)mariadb.org> writes: > Kristian Nielsen, could you please review this patch? I guess my main question here is, did you think about why you want to) fix this 1) in a post-GA release, 2) at all? One problem with adding new statements as unsafe for statement-based binlogging is that it introduces new warnings in existing applications. This is especially annoying on a slave with binlogging enabled, as every unsafe statement now causes a warning in the slave error log. Thus, it can happen that a security update for a user leaves them with an error log that constantly fills up on a busy slave. It is my understanding that statement-based binlogging (as opposed to mixed or row) is still quite popular, and there have been complaints about these warnings before. Another point is that this patch seems to fix only a corner case of a bigger problem? This particular patch is about non-transactional updates. But updating a unique key can break replication just as well on transactional tables: CREATE TABLE t1 (a INT PRIMARY KEY); INSERT INTO t1 VALUES (2), (3), (4); UPDATE t1 SET a=a-1; Since we do not have deferred constraints, this UPDATE will fail or succeed depending on the order in which rows are updated. So a statement that succeeds andgets binlogged on the master can break on the slave if the rows are updated in a different order. So any reason that you fix one part of UPDATE of unique key but not the other? (I suppose the same problem can also happen with foreign key constraints, though it might take some creativity to come up with an example where an UPDATE will conflict or not with foreign keys depending on row update order). The bug has fix version 5.5, your patch seems to be against 10.0, where do you intend to push it? I would probably have chosen to fix in 10.1, if at all, but I'm not necessarily against 5.5 or 10.0, I'm mostly just asking what the reason is for whichever version was chosen? > I don't particularly like that it explicitly invokes > thd->decide_logging_format() . But there's no way around it, first time > logging format is decided in lock_tables(), basically when tables are just > opened. At that point we don't know what fields will be updated yet. I am not really familiar with how binlogging is decided on the master, have not worked with that code so far. So unfortunately I am not able to say much about whether this is a problem or not. At least the call is still before the actual update of rows starts, so I suppose it should be ok... > At lp:~maria-captains/maria/10.0 > > ------------------------------------------------------------ > revno: 4484 > revision-id: sergii(a)pisem.net-20141116234900-747qdhm9ti9xh070 > parent: sergii(a)pisem.net-20141113161325-mdz5ox7e86iyazss > fixes bug: https://mariadb.atlassian.net/browse/MDEV-4255 > committer: Sergei Golubchik <sergii(a)pisem.net> > branch nick: 10.0 > timestamp: Mon 2014-11-17 00:49:00 +0100 > message: > MDEV-4255 MIXED replication: Unique key updates are not marked unsafe, replication fails I think this needs a better description, especially since this can cause new warnings in slave error logs. Here is a suggestion, but feel free to use something else if you prefer: "MDEV-4255 MIXED replication: Unique key updates are not marked unsafe, replication fails Statement-based replication of UPDATE statements that modify (parts of) unique keys is not safe. If the order of row updates differ between master and slave, the slave can end up modifying a different set of rows before a unique key violation error than what was updated on the master. If the table is non-transactional, then the statement will be partially executed on the master and on the slave, but with different result, causing replication to diverge. This patch marks as unsafe for statement-based replication any UPDATE that modifies a unique key on a non-transactinal table. In mixed-mode replication, this will cause the query to be logged in row mode. In statement-mode replication, this will produce a warning about binlogging of unsafe statement." > === modified file 'sql/share/errmsg-utf8.txt' > --- a/sql/share/errmsg-utf8.txt 2014-11-13 16:13:25 +0000 > +++ b/sql/share/errmsg-utf8.txt 2014-11-16 23:49:00 +0000 > @@ -7111,3 +7111,5 @@ ER_SLAVE_SKIP_NOT_IN_GTID > eng "When using GTID, @@sql_slave_skip_counter can not be used. Instead, setting @@gtid_slave_pos explicitly can be used to skip to after a given GTID position." > ER_TABLE_DEFINITION_TOO_BIG > eng "The definition for table %`s is too big" > +ER_BINLOG_UNSAFE_UPDATE_UNIQUE_NONTRANS > + eng "The statement is unsafe because it updates a UNIQUE key colums in a non-transactional table. This is unsafe because in the case of a conflict a non-predictable part of the table will stay updated." (Adding a new error code in a GA release requires some care, especially if fix-version:5.5 is intended, but I'm sure you're better aware of this than most). > === modified file 'sql/sql_lex.h' > --- a/sql/sql_lex.h 2014-08-07 16:06:56 +0000 > +++ b/sql/sql_lex.h 2014-11-16 23:49:00 +0000 > @@ -1319,6 +1319,12 @@ class Query_tables_list > */ > BINLOG_STMT_UNSAFE_AUTOINC_NOT_FIRST, > > + /** > + INSERT into auto-inc field which is not the first part of composed > + primary key. > + */ > + BINLOG_STMT_UPDATE_UNIQUE_NONTRANS, > + Copy-paste error. Here, you copied the comment of the earlier entry, you surely intended to re-write the comment to describe the newly added entry. Hope this helps, - Kristian.

3 7

Re: [Maria-developers] [Commits] 953dd8d: MDEV-6997 MDEV-6996 MDEV-7002 MDEV-7006 MDEV-7009 MDEV-7012 MDEV-7015 MDEV-6977
by Sergei Golubchik 29 Nov '14

29 Nov '14

Hi, sanja! On Nov 05, sanja(a)mariadb.com wrote: > revision-id: 953dd8d87fea224e26d11452eb458c79ba098f4a > parent(s): d6981391eb8f904135dfdedb5b90a7592b35e648 > committer: Oleksandr Byelkin > branch nick: work-maria-10.1-sysvar4 > timestamp: 2014-11-05 11:05:01 +0100 > message: > > MDEV-6997 MDEV-6996 MDEV-7002 MDEV-7006 MDEV-7009 MDEV-7012 MDEV-7015 MDEV-6977 I'd prefer a slightly more verbose commit comment. And don't forget - first line is the subject. For example: Use NO_SET_STMT flag for more variables Fixes MDEV-6997 MDEV-6996 MDEV-7002 MDEV-7006 MDEV-7009 MDEV-7012 MDEV-7015 MDEV-6977 I won't comment on individual variables here, as you're simply disabling them all. See my comments in Jira. Regards, Sergei

1 0

[Maria-developers] MDEV-7004 review
by Sergei Golubchik 28 Nov '14

28 Nov '14

Hi, Sergey I didn't review MDEV-7200 (InnoDB) and patches that are already in 10.1. Almost everything else was fine, see few comments below. > ------------------------------------------------------------ > revno: 4513 > committer: Sergey Vojtovich <svoj(a)mariadb.org> > branch nick: 10.0-power > timestamp: Fri 2014-11-07 22:03:18 +0400 > message: > Multi-rwlock lock for table definition cache hash. > added: > modified: > diff: > === modified file 'sql/table_cache.cc' > --- sql/table_cache.cc 2014-10-21 12:20:39 +0000 > +++ sql/table_cache.cc 2014-11-07 18:03:18 +0000 > @@ -53,6 +53,77 @@ > #include "table.h" > #include "sql_base.h" > > + > +/** > + Table definition hash lock. > + > + Lock consists of multiple rw-locks (8 by default). Lock is write locked > + when all instances are write locked. Lock is read locked when any instance > + is read locked. > + > + This class was implemented to workaround scalability bottleneck of table > + definition hash rw-lock. > + > + To be replaced with lock-free hash when we understand how to solve the > + following problems: > + - lock-free hash iterator (can be workarounded by introducing global > + all_tables list); I don't particularly like that solution. Could you try to switch to lock-free hash now? I'll try to do the iterator asap > + - lf_hash_search() may fail because of OOM. It is rather complex to handle > + it properly. > +*/ > + > +class TDC_lock > +{ > + uint m_instances; > + bool m_write_locked; > + char pad[128]; > + struct st_locks > + { > + mysql_rwlock_t lock; > + char pad[128]; In these cases I usually do: pad[128 - sizeof(mysql_rwlock_t)]; > + } *locks; > +public: > + int init(PSI_rwlock_key key, uint instances) > + { > + m_instances= instances; > + m_write_locked= false; > + locks= (st_locks *) my_malloc(sizeof(struct st_locks) * m_instances, MYF(MY_WME)); > + if (!locks) > + return 1; > + for (uint i= 0; i < m_instances; i++) > + mysql_rwlock_init(key, &locks[i].lock); > + return 0; > + } > + void destroy(void) > + { > + for (uint i= 0; i < m_instances; i++) > + mysql_rwlock_destroy(&locks[i].lock); > + my_free(locks); > + } > + void wrlock(void) > + { > + for (uint i= 0; i < m_instances; i++) > + mysql_rwlock_wrlock(&locks[i].lock); > + m_write_locked= true; > + } > + void rdlock(THD *thd= 0) > + { > + mysql_rwlock_rdlock(&locks[thd ? thd->thread_id % m_instances : 0].lock); > + } > + void unlock(THD *thd= 0) > + { > + if (m_write_locked) > + { > + m_write_locked= false; > + for (uint i= 0; i < m_instances; i++) > + mysql_rwlock_unlock(&locks[i].lock); > + } > + else > + mysql_rwlock_unlock(&locks[thd ? thd->thread_id % m_instances : 0].lock); > + } > +}; > + > + > /** Configuration. */ > ulong tdc_size; /**< Table definition cache threshold for LRU eviction. */ > ulong tc_size; /**< Table cache threshold for LRU eviction. */ > ------------------------------------------------------------ > revno: 4512 > committer: Sergey Vojtovich <svoj(a)mariadb.org> > branch nick: 10.0-power > timestamp: Fri 2014-10-31 15:44:36 +0400 > message: > Preallocate locks on THD mem_root to avoid expensive malloc. > modified: > diff: > === modified file 'sql/lock.cc' > --- sql/lock.cc 2014-09-30 17:31:14 +0000 > +++ sql/lock.cc 2014-10-31 11:44:36 +0000 > @@ -700,7 +699,7 @@ MYSQL_LOCK *get_lock_data(THD *thd, TABL > TABLE **to, **table_buf; > DBUG_ENTER("get_lock_data"); > > - DBUG_ASSERT((flags == GET_LOCK_UNLOCK) || (flags == GET_LOCK_STORE_LOCKS)); > + DBUG_ASSERT((flags & GET_LOCK_UNLOCK) || (flags & GET_LOCK_STORE_LOCKS)); Not the same, old assert also verified that these flags aren't set both. New assert doesn't. But I don't think it matters much. > DBUG_PRINT("info", ("count %d", count)); > > for (i=lock_count=table_count=0 ; i < count ; i++) > ------------------------------------------------------------ > revno: 4508 > committer: Sergey Vojtovich <svoj(a)mariadb.org> > branch nick: 10.0 > timestamp: Tue 2014-10-21 16:20:39 +0400 > message: > modified: > diff: > === modified file 'sql/handler.cc' > --- sql/handler.cc 2014-11-13 10:01:31 +0000 > +++ sql/handler.cc 2014-10-21 12:20:39 +0000 > @@ -410,13 +410,16 @@ static int hton_ext_based_table_discover > static void update_discovery_counters(handlerton *hton, int val) > { > if (hton->discover_table_existence == full_discover_for_existence) > - my_atomic_add32(&need_full_discover_for_existence, val); > + my_atomic_add32_explicit(&need_full_discover_for_existence, val, > + MY_MEMORY_ORDER_RELAXED); > > if (hton->discover_table_names) > - my_atomic_add32(&engines_with_discover_table_names, val); > + my_atomic_add32_explicit(&engines_with_discover_table_names, val, > + MY_MEMORY_ORDER_RELAXED); > > if (hton->discover_table) > - my_atomic_add32(&engines_with_discover, val); > + my_atomic_add32_explicit(&engines_with_discover, val, > + MY_MEMORY_ORDER_RELAXED); I wouldn't do that. Technically there's a dependency between these counters and loaded engines, so the order, kind of, matters. I don't think there is any practical risk of bugs here, but, on the other hand, these counters are only updated when an engine is loaded or unloaded, so there's no need to bother with the relaxed memory ordering here. > } > > int ha_finalize_handlerton(st_plugin_int *plugin) > > === modified file 'sql/mysqld.cc' > --- sql/mysqld.cc 2014-11-18 21:25:47 +0000 > +++ sql/mysqld.cc 2014-10-21 12:20:39 +0000 > @@ -3884,7 +3884,7 @@ static void my_malloc_size_cb_func(long > } > // workaround for gcc 4.2.4-1ubuntu4 -fPIE (from DEB_BUILD_HARDENING=1) > int64 volatile * volatile ptr=&global_status_var.memory_used; > - my_atomic_add64(ptr, size); > + my_atomic_add64_explicit(ptr, size, MY_MEMORY_ORDER_RELAXED); good! > } > } > > > === modified file 'sql/mysqld.h' > --- sql/mysqld.h 2014-11-13 08:56:28 +0000 > +++ sql/mysqld.h 2014-10-21 12:20:39 +0000 > @@ -629,7 +629,7 @@ inline __attribute__((warn_unused_result > { > query_id_t id; > my_atomic_rwlock_wrlock(&global_query_id_lock); > - id= my_atomic_add64(&global_query_id, 1); > + id= my_atomic_add64_explicit(&global_query_id, 1, MY_MEMORY_ORDER_RELAXED); I believe this is fine > my_atomic_rwlock_wrunlock(&global_query_id_lock); > return (id); > } > === modified file 'sql/table_cache.cc' > --- sql/table_cache.cc 2014-06-10 18:20:33 +0000 > +++ sql/table_cache.cc 2014-10-21 12:20:39 +0000 > @@ -139,7 +139,7 @@ uint tc_records(void) > { > uint count; > my_atomic_rwlock_rdlock(&LOCK_tdc_atomics); > - count= my_atomic_load32(&tc_count); > + count= my_atomic_load32_explicit(&tc_count, MY_MEMORY_ORDER_RELAXED); I'm not sure about this file. You should know better whether it's safe to use relaxed memory ordering here. > my_atomic_rwlock_rdunlock(&LOCK_tdc_atomics); > return count; > } Regards, Sergei

2 5