October 2014 - developers - lists.mariadb.org

[Maria-developers] Syntax for parallel replication
by Kristian Nielsen 18 Dec '14

18 Dec '14

Hi Serg, Can you help me with suggestions/comments for the following proposal for how to do configuration for MDEV-6676, speculative parallel replication? I am not too happy about how things are in 10.0. There, there is a single option --slave-parallel-threads=N. If N>0, then parallel replication is enabled, else not. One problem is that this makes it not configurable per multi-source master connection. Another problem is that there are two possible mechanisms for parallelisation, group-… [View More]commit based and domain_id based; currently, one can enable none of them or both, but not just one of them. MDEV-6676 will introduce at least one other mechanism, which seems to make it essential to make a better way to configure this. Now, ideally, there would not be any configuration at all. The server would just run things in parallel when possible and to the degree desirable. However, I think there are some reasons that we need to allow the user this configurability: - Parallel replication is still a somewhat experimental feature, so it seems too risky to enable it by default. Also, it doesn't really seem possible for the server to automatically set the best number of threads to use, with current implementation (or possibly any implementation). - When replicating with non-transactional updates, or in non-gtid mode, slave state is not crash safe. This is true in non-parallel replication also, but in parallel replication, the problem seems amplified, as there may be multiple transactions in progress at the time of a crash, complicating possible manual recovery. This also suggests that parallel replication must be configurable. - When using domain-based parallel replication, the user is responsible for ensuring that independent domains are non-conflicting and can be replicated out-of-order wrt. each other. So if replication domains are used, but this property is not guaranteed, then domain-based parallel replication need to be configurable, or parallel replication cannot be used at all. - The new speculative replication feature in MDEV-6676 is not always guaranteed to be a win - in some workloads, where there are many conflicts between successive transactions, excessive rollback could cause it to be less efficient than not using it. Again, this suggests it needs to be configurable. So given this, I came up with the following idea for syntax: CHANGE MASTER TO PARALLEL_MODE=(domain,groupcommit,transactional,waiting) Each of the four keywords in the parenthesis is optional. "domain" enables domain-based parallelisation, where each replication domain is treated independently. "groupcommit" enables the non-speculative mode, where only transactions that group-committed together on the master are applied in parallel on the slave. "transactional" enables the speculative mode, where all transactional DML is optimistically tried in parallel, and then in case of conflict a rollback and retry is done. "groupcommit" and "transactional" are mutually exclusive, at most one of them can be specified. The default would be (domain,groupcommit) to be back-wards compatible with 10.0. If slave_parallel_thread=0, then no parallel apply will happen even if PARALLEL_MODE is non-empty. If slave_parallel_thread>0 but PARALLEL_MODE is empty (PARALLEL_MODE=()), then again no parallel apply will be done. The "waiting" option is not essential to add, we could remove it, I put it in because there were already a number of options so it seemed to cause no harm. The idea is that on the master, we detect if transaction T2 had to do a row lock wait on transaction T1. If so, it seems likely that a similar conflict could occur on the slave, so we will not run T2 in parallel with T1; instead we will let T2 wait for T1 to commit before T2 is started. The "waiting" option could be enabled by a user to disable this check, enabling even more aggressive parallelisation. I am not sure if it is worth it to have this configurable though, comments welcome. I have not checked how hard it will be to implement the new syntax. We do not have any similar multi-option CHANGE MASTER elements, as far as I know, but it is similar to ENUM system variables. And we already have a IGNORE_SERVER_IDS syntax with comma-separated list within parenthesis. So hopefully not too hard to do, and somewhat consistent with existing syntax. So what do you think? Is it a reasonable syntax? Any comments, or suggestions for better way to do it? Thanks, - Kristian. [View Less]

5 23

[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 ----------------------------… [View More]---------------------------------------------- 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; [View Less]

2 2

Re: [Maria-developers] [Commits] Rev 3879: MDEV-6593 : domain_id based replication filters in lp:~maria-captains/maria/maria-10.0-galera
by Kristian Nielsen 24 Nov '14

24 Nov '14

Nirbhay Choubey <nirbhay(a)skysql.com> writes: > ------------------------------------------------------------ > revno: 3879 > revision-id: nirbhay(a)skysql.com-20140912181622-s33e7r0vtgpju95q > parent: nirbhay(a)skysql.com-20140814224304-rtkeal7p9lk06li1 > committer: Nirbhay Choubey <nirbhay(a)skysql.com> > branch nick: maria-10.0-galera_6593 > timestamp: Fri 2014-09-12 14:16:22 -0400 > message: > MDEV-6593 : domain_id based replication filters > … [View More]> Added support for IGNORE_DOMAIN_IDS and DO_DOMAIN_IDS. Here is my review. Some general comments, followed by some detailed comments in-line in the patch: 1. Did you prepare any documentation? You could either add it to the knowledgebase directly, or write up a text file and ask Daniel to add it in relevant places. 2. Did you run the full test suite? I would have expected at least some tests in the multi_source suite to need modification. The chance touches enough things that you should test it in a feature tree in Buildbot to ensure that it is green before pushing it to a main tree. 3. I think some extra test cases are needed: - You should test (and document) what happens when the same domain_id is in both the DO and the IGNORE list. Is it actually ever sensible to have both DO_DOMAIN_IDS and IGNORE_DOMAIN_IDS ? If not, I think it should be an error to try. - You should also add a test using non-GTID mode. - You should also add a test where parallel replication is used (--slave-parallel-threads > 0). - Add a test for the following scenario when using GTID mode: Some domain id is ignored, some events are received from the master and ignored. Then the slave is stopped, and the ignore rule is removed. What happens when the slave is restarted and reconnects? I think the previously ignored domain id events will now be re-fetched and replicated. I guess that's ok, but it should be documented. Also test what happens when binlog files on the master have been rotated and purged, so that the events are no longer available on the master (an error should occur). - Add a similar test for when not using GTID mode, the last event replicated by the slave is ignored, the slave is stopped, the ignore rule removed and the slave restarted; will the last event now be re-fetched or not (I think not). - Add a test that checks that seconds_behind_master is properly updated to zero even when the last event is ignored due to DO_DOMAIN_IDS/IGNORE_DOMAIN_IDS. It can perhaps be a bit tricky to test, but I think you can use this existing test as a basis: mysql-test/suite/rpl/t/rpl_parallel2.test - It would also be good to check that the code works when used on an old master (which does not send GTID events). You can do this by creating a binlog file with a 5.5 server and storing it in mysql-test/std_data/, and then installing it in a master; there should be several other test cases that already do this. 4. Also see detailed comments for some possible problems with the implementation. The most serious is probably to ensure that events are not skipped after the end of the group, we need a couple of tests for this, see below. Also, there is a small memory leak, see below. > +DO_DOMAIN_IDS (before) : 1 > +IGNORE_DOMAIN_IDS (before) : 2 > +CHANGE MASTER TO DO_DOMAIN_IDS=(4,4,5,1,7,7,7,1,1,2,6,8,1,4,5,5,9,3), MASTER_USE_GTID=current_pos; So if a domain_id is specified multiple times, the redundant ones will be ignored. It would seem preferable to give an error... but as far as I can see the behaviour is the same for IGNORE_SERVER_IDS, so better to be consistent as you did in the code. Just be sure to add this to the documentation. > === added file 'mysql-test/suite/rpl/t/rpl_domain_id_filter.test' > --- a/mysql-test/suite/rpl/t/rpl_domain_id_filter.test 1970-01-01 00:00:00 +0000 > +++ b/mysql-test/suite/rpl/t/rpl_domain_id_filter.test 2014-09-12 18:16:22 +0000 > +SET @gtid_domain_id_saved = @@session.gtid_domain_id; This is not needed, nor do you use it anywhere, you can remove it. > +##### Case 3: When both DO_DOMAIN_IDS and IGNORE_DOMAIN_IDS are non-empty > + > +source include/stop_slave.inc; > +let $do_domain_ids_before= query_get_value(SHOW SLAVE STATUS, Replicate_Do_Domain_Ids, 1); > +let $ignore_domain_ids_before= query_get_value(SHOW SLAVE STATUS, Replicate_Ignore_Domain_Ids, 1); > +--echo DO_DOMAIN_IDS (before) : $do_domain_ids_before > +--echo IGNORE_DOMAIN_IDS (before) : $ignore_domain_ids_before > + > +# Replicate events belonging to "domain_id 1". (DO_DOMAIN_IDS should win!) > +CHANGE MASTER TO DO_DOMAIN_IDS=(1), IGNORE_DOMAIN_IDS=(2), MASTER_USE_GTID=current_pos; Did you intend here to test overlapping ids in DO_DOMAIN_IDS and IGNORE_DOMAIN_IDS (due to the comment "DO_DOMAIN_IDS should win") ? But if I understand correctly, IGNORE_DOMAIN_IDS will be ignored if DO_DOMAIN_IDS is non-empty. So I think it should be an error to try to set both - do you agree? > +--source include/rpl_end.inc Please add additional testcases, as suggested at the start of review. > === modified file 'sql/rpl_mi.cc' > --- a/sql/rpl_mi.cc 2014-02-19 10:05:15 +0000 > +++ b/sql/rpl_mi.cc 2014-09-12 18:16:22 +0000 > @@ -513,6 +513,25 @@ > else > mi->using_gtid= Master_info::USE_GTID_NO; > } > + > + // DO_DOMAIN_IDS > + if (0 == strncmp(buf, STRING_WITH_LEN("do_domain_ids=")) && > + mi->domain_id_filter.init_ids(buf + sizeof("do_domain_ids"), > + Domain_id_filter::DO_DOMAIN_IDS)) > + { > + sql_print_error("Failed to initialize master info do_domain_ids"); > + goto errwithmsg; > + } > + > + // IGNORE_DOMAIN_IDS > + if (0 == strncmp(buf, STRING_WITH_LEN("ignore_domain_ids=")) && > + mi->domain_id_filter.init_ids(buf + sizeof("ignore_domain_ids"), > + Domain_id_filter::IGNORE_DOMAIN_IDS)) > + { > + sql_print_error("Failed to initialize master info " > + "ignore_domain_ids"); > + goto errwithmsg; > + } So here you use the same style as adjacent code (that I wrote). However, I have since learned that the style (0 == strncmp(...)) is not used in the server. So please change this, as well as my existing code, to use either strncmp(...) == 0 or !strncmp(), as you prefer. Another thing from my old code is the sizeof("do_domain_ids"), which highly confused me for a bit before I realised that while the '=' is not included in the sizeof(), the terminating '\0' _is_ included. So since even I myself is confused by this code, better clarify it ;-) Perhaps it would be clearer if changed to (sizeof("do_domain_ids=")-1) (both in my old code and in your new code). (Normally it's good to follow the style of existing code as you did, I just noticed this in the review and realised that my own "existing code" would benefit from these small improvements). > @@ -634,6 +653,16 @@ > } > } > > + char* do_domain_ids_buf= > + mi->domain_id_filter.as_string(Domain_id_filter::DO_DOMAIN_IDS); > + if (do_domain_ids_buf == NULL) > + DBUG_RETURN(1); > + > + char* ignore_domain_ids_buf= > + mi->domain_id_filter.as_string(Domain_id_filter::IGNORE_DOMAIN_IDS); > + if (ignore_domain_ids_buf == NULL) > + DBUG_RETURN(1); You have a memory leak here, do_domain_ids_buf will not be freed if ignore_domain_ids_buf fails due to out-of-memory. > @@ -1348,4 +1382,218 @@ > DBUG_RETURN(result); > } > > +/* ctor */ > +Domain_id_filter::Domain_id_filter() : m_filter(false) > +{ > + for (int i= DO_DOMAIN_IDS; i <= IGNORE_DOMAIN_IDS; i ++) > + { > + my_init_dynamic_array(&m_domain_ids[i], sizeof(uint32), 16, 16, MYF(0)); > + } > +} > + > +/* dtor */ > +Domain_id_filter::~Domain_id_filter() I don't think comments like /* ctor */ and /* dtor */ are useful, better just omit them. > +/** > + Update the domain id list of the given type with elements from the new list. > + > + @param new_ids [IN] new list of ids > + @param type [IN] domain id list type to update > + @param changed [IN] has the domain id list changed in the > + last CHANGE MASTER command? > + > + @retval void > +*/ > +void Domain_id_filter::update_ids(DYNAMIC_ARRAY *new_ids, enum_list_type type, > + bool changed) > +{ > + DYNAMIC_ARRAY *ids= &m_domain_ids[type]; > + > + if (changed == true) > + reset_dynamic(ids); > + > + /* bsearch requires an ordered list. */ > + sort_dynamic(new_ids, change_master_domain_id_cmp); > + > + for (uint i= 0; i < new_ids->elements; i++) > + { > + uint32 domain_id; > + get_dynamic(new_ids, (void *) &domain_id, i); > + > + if (bsearch((const uint32 *) &domain_id, ids->buffer, ids->elements, > + sizeof(uint32), change_master_domain_id_cmp) == NULL) > + { > + insert_dynamic(ids, (uint32*) &domain_id); > + } > + } > + return; > +} Ok, so I understand that this code is modelled after the existing code for IGNORED_SERVER_IDS. But I found it rather confusing, because it looks like there is a bug: if changed==false, then the code does not ensure that the resulting array will still be sorted. I guess the bug is not possible to trigger, because the list of stuff to add will only be non-empty if changed==true? But that is rather confusing. Please simplify the logic; maybe you only need to call this function when changed==true, so the parameter can be omitted and the array always reset? Also, consider if you can rewrite the existing code for IGNORE_SERVER_IDS so that the same common code can be used as for IGNORE_DOMAIN_IDS and DO_DOMAIN_IDS, that would simplify the code and reduce risk of bugs or inconsistencies. > +/** > + Initialize the given domain id list with the numbers from the specified > + buffer. > + > + @param buf [IN] buffer > + @param type [IN] domain id list type > + > + @retval false Success > + true Error > +*/ > +bool Domain_id_filter::init_ids(char *buf, enum_list_type type) > +{ > + char *token, *last; > + uint num_items; > + DYNAMIC_ARRAY *ids= &m_domain_ids[type]; > + > + token= strtok_r(buf, " ", &last); > + if (token == NULL) > + { > + return true; /* error */ > + } > + > + num_items= atoi(token); > + for (uint i= 0; i < num_items; i++) > + { > + token= strtok_r(NULL, " ", &last); > + if (token == NULL) > + { > + return true; /* error */ > + } > + else > + { > + uint32 val= atoi(token); > + insert_dynamic(ids, (uchar *) &val); > + } > + } > + return false; > +} Again, did you consider using the same code for this and for IGNORE_SERVER_IDS? (I think you did, because you added a comment to init_dynarray_intvar_from_file ;-). Maybe consider it again; it is fine to rewrite/remove the existing code for IGNORE_SERVER_IDS and replace it with your new code. Again, it would reduce risk for bugs or inconsistencies. > + /* getter */ > + bool get_filter() { return m_filter; } > + > + /* setter */ > + void set_filter(uint32 domain_id); These are not really getter and setters, I think, at least I find those names confusing. Maybe something like do_filter(uint32 domain_id) and is_group_filtered() ? And expand the comments, so that they explain that these functions respectively check a GTID for needing to be filtered, and return whether the current group is being filtered. Also, personally I dislike setter/getter, and would prefer just to access the m_filter as a public member. But I suppose that's style and up to you. > + /* > + Initialize the given domain id list with the numbers from the specified > + buffer. > + > + @param buf [IN] buffer > + @param type [IN] domain id list type > + > + @retval false Success > + true Error > + */ > + bool init_ids(char *buf, enum_list_type type); Clarify in the comment that the list is initialized from a string of space-separated numbers, the first of which is the number of following entries. > @@ -182,6 +279,7 @@ > bool flush_relay_log_cache, > bool need_lock_relay_log); > int change_master_server_id_cmp(ulong *id1, ulong *id2); > +int change_master_domain_id_cmp(uint32 *id1, uint32 *id2); I think change_master_domain_id_cmp() is only used in rpl_mi.cc, so should be static there, and not refered here in rpl_mi.h > === modified file 'sql/slave.cc' > --- a/sql/slave.cc 2014-08-12 03:55:41 +0000 > +++ b/sql/slave.cc 2014-09-12 18:16:22 +0000 > @@ -1259,6 +1259,7 @@ > going to ignore (to not log them in the relay log). > Items being read are supposed to be decimal output of values of a > type shorter or equal of @c long and separated by the single space. > + It also used to restore DO_DOMAIN_IDS & IGNORE_DOMAIN_IDS lists. This comment suggests you wanted to share code, then later changed your mind. If you end up keeping the code separate, you still need to delete this comment. (And there is a typo: "It also" -> "It is also", but if you remove it anyway...). > @@ -5601,6 +5610,10 @@ > mi->last_queued_gtid= event_gtid; > mi->last_queued_gtid_standalone= > (gtid_flag & Gtid_log_event::FL_STANDALONE) != 0; > + > + /* Should filter all the subsequent events in the current GTID group? */ > + mi->domain_id_filter.set_filter(event_gtid.domain_id); So, here we will set m_filter to true, if we want to skip the following group. But I don't see any code that will set it to false at the end of the group? That seems a problem, as we could then end up wrongly skipping all kinds of other events, such as Gtid_list, Incident, ... Also note that it is possible for a GTID group to not be terminated by COMMIT/XID. This can happen at least if the master crashes in the middle of writing a group; the slave will receive only the first part of it, followed by a master restart Format_description event. That case needs to be handled, and you should add a test case for it. Also it is possible for eg. a master downgrade to introduce following event groups that do not start with GTID - seems these would also be wrongly skipped? So we also need a test case for this... Hope this helps, - Kristian. [View Less]

2 8

Re: [Maria-developers] [Commits] MDEV-60 Support for Spatial Reference systems for the GIS data.
by Sergei Golubchik 11 Nov '14

11 Nov '14

Hi, Alexey! First, please, please, *do not* send patches from Thunderbird. It took me a lot of time to fix it, the patch was badly garbled. May be broken formatting everywhere in the patch was also caused by this. Then, I didn't see any tests in the patch, not even syntax tests, but also not create field, store, read tests. Please add them. Now, the review: > diff --git a/sql/field.cc b/sql/field.cc > --- a/sql/field.cc > +++ b/sql/field.cc > @@ -7808,6 +7808,35 @@ uint … [View More]Field_blob::is_equal(Create_field *new_field) > > #ifdef HAVE_SPATIAL > > +uint gis_field_options_image(uchar *buff, List<Create_field> &create_fields) > +{ > + uint image_size= 0; > + uint n_field= 0; > + List_iterator<Create_field> it(create_fields); > + Create_field *field; > + while ((field= it++)) > + { > + if (field->sql_type != MYSQL_TYPE_GEOMETRY) > + continue; > + if (buff) > + int2store(buff + image_size, ((uint16) n_field)); > + image_size+= 2; > + if (buff) > + int2store(buff + image_size, ((uint16) field->length)); > + image_size+= 2; > + if (buff) > + int2store(buff + image_size, ((uint16) field->decimals)); > + image_size+= 2; > + if (buff) > + int4store(buff + image_size, ((uint32) field->srid)); > + image_size+= 4; > + n_field++; > + } > + > + return image_size; > +} Hmm... So, you basically, put an array of properties in frm, under EXTRA2_GIS Two comments: 1. Why n_field, length and decimals? I thought you only add srid, other properties are already stored elsewhere, aren't they? 2. Really think about upgrades downgrades. The "extra" section in the frm doesn't handle them well, I know how these problems happen. Think, what happens when you add more properties here? What if you remove some? How new server will parse old frms, how old server will read new frms? > + > + > void Field_geom::sql_type(String &res) const > { > CHARSET_INFO *cs= &my_charset_latin1; > @@ -9181,7 +9210,7 @@ bool Create_field::init(THD *thd, char *fld_name, enum_field_types fld_type, > Item *fld_on_update_value, LEX_STRING *fld_comment, > char *fld_change, List<String> *fld_interval_list, > CHARSET_INFO *fld_charset, uint fld_geom_type, > - Virtual_column_info *fld_vcol_info, > + char *fld_srid, Virtual_column_info *fld_vcol_info, fix the formatting, please > engine_option_value *create_opt, bool check_exists) > { > uint sign_len, allowed_type_modifier= 0; > @@ -9232,6 +9261,7 @@ bool Create_field::init(THD *thd, char *fld_name, enum_field_types fld_type, > pack_length= key_length= 0; > charset= fld_charset; > geom_type= (Field::geometry_type) fld_geom_type; > + srid= fld_srid ? (uint)atoi(fld_srid) : 0; and here > interval_list.empty(); > > comment= *fld_comment; > @@ -9635,7 +9665,7 @@ Field *make_field(TABLE_SHARE *share, uchar *ptr, uint32 field_length, > uint pack_flag, > enum_field_types field_type, > CHARSET_INFO *field_charset, > - Field::geometry_type geom_type, > + Field::geometry_type geom_type, uint srid, and here, and everywhere, I won't comment on that anymore > Field::utype unireg_check, > TYPELIB *interval, > const char *field_name) > diff --git a/sql/sql_lex.h b/sql/sql_lex.h > --- a/sql/sql_lex.h > +++ b/sql/sql_lex.h > @@ -2358,6 +2358,7 @@ struct LEX: public Query_tables_list > char *backup_dir; /* For RESTORE/BACKUP */ > char* to_log; /* For PURGE MASTER LOGS TO */ > char* x509_subject,*x509_issuer,*ssl_cipher; > + char *srid; really? it can only be used for field declarations, I don't think there's a need to put it in LEX see below. > String *wild; /* Wildcard in SHOW {something} LIKE 'wild'*/ > sql_exchange *exchange; > select_result *result; > diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy > --- a/sql/sql_yacc.yy > +++ b/sql/sql_yacc.yy > @@ -6359,6 +6360,16 @@ precision: > } > ; > > +srid_option: > + /* empty */ > + { Lex->srid= (char *)0; } > + | > + SRID_SYM EQ NUM > + { > + Lex->srid=$3.str; > + } return the value of srid here, don't store it in lex. Like $$ = $3.str; or even $$ = atoi($3.str); > + ; > + > field_options: > /* empty */ {} > | field_opt_list {} > diff --git a/sql/table.cc b/sql/table.cc > --- a/sql/table.cc > +++ b/sql/table.cc > @@ -1451,6 +1464,20 @@ int TABLE_SHARE::init_from_binary_frm_image(THD *thd, bool write, > #ifdef HAVE_SPATIAL > geom_type= (Field::geometry_type) strpos[14]; > charset= &my_charset_bin; > + if (gis_options && gis_options_len >= 10) > + { > + gis_length= uint2korr(gis_options+2); > + gis_decimals= uint2korr(gis_options+4); > + srid= uint4korr(gis_options+6); > + gis_options_len-= 10; > + gis_options+= 10; If you have the function to generate this data in field.cc, I'd put a small function to read it in field.cc too. Just to have all reading/writing code in the same place. > + } > + else > + { > + srid= 0; > + gis_length= 0; > + gis_decimals= 0; > + } > #else > goto err; > #endif > diff --git a/sql/unireg.h b/sql/unireg.h > --- a/sql/unireg.h > +++ b/sql/unireg.h > @@ -190,6 +190,7 @@ enum extra2_frm_value_type { > #define EXTRA2_ENGINE_IMPORTANT 128 > > EXTRA2_ENGINE_TABLEOPTS=128, > + EXTRA2_GIS=129, better make it 2, not 129 (below EXTRA2_ENGINE_IMPORTANT threshold), because you de facto ignore it anyway when HAVE_SPATIAL is not defined. > }; > > int rea_create_table(THD *thd, LEX_CUSTRING *frm, > diff --git a/unittest/mysys/CMakeLists.txt b/unittest/mysys/CMakeLists.txt > --- a/unittest/mysys/CMakeLists.txt > +++ b/unittest/mysys/CMakeLists.txt > @@ -17,7 +17,7 @@ MY_ADD_TESTS(bitmap base64 my_vsnprintf my_atomic my_rdtsc lf my_malloc > LINK_LIBRARIES mysys) > > MY_ADD_TESTS(ma_dyncol > - LINK_LIBRARIES mysqlclient) > + LINK_LIBRARIES mysqlclient stdc++) Why? > > IF(WIN32) > MY_ADD_TESTS(my_delete LINK_LIBRARIES mysys) > Regards, Sergei [View Less]

2 2

[Maria-developers] Cross-compiling MariaDB galera cluster
by Sylvain Raybaud 30 Oct '14

30 Oct '14

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Dear MariaDB community, Recently I've been trying to cross-compile MariaDB and I've been running in some problems. I'd like to have your opinion on them. 1) Determining stack direction 2) Building Percona XtraDB 3) Building jemalloc Let me first tell that I'm trying to build MariaDB galera version 10.0.12. I'm compiling for Arm but may be compiling for something else in the unpredictable future. 1) Determining stack direction I started by … [View More]passing the following options to cmake (via the buildroot system): - -DCMAKE_CROSSCOMPILING=1 -DWITH_WSREP=1 -DWITH_INNODB_DISALLOW_WRITES=1 The first problem I ran into was cmake complaining: STACK_DIRECTION is not defined. Please specify -DSTACK_DIRECTION=1 or -DSTACK_DIRECTION=-1 when calling cmake. It seems that STACK_DIRECTION is not automatically determined when cross-compiling. I may determine the correct value for a given target. But if I need a portable solution (and I need one in order to create a buildroot package so that I can build for many different target without too much hassle) I'm not sure what my options are. It seems that the toolchain is aware of this value since stack direction (or more precisely STACK_GROWS_DOWNWARD) is set in ./output/host/opt/ext-toolchain/lib/gcc/arm-none-linux-gnueabi/4.8.1/plugin/include/config/arm/arm.h Maybe there is a way to use this somehow? What do you think? 2) Building Percona XtraDB For the sake of trying I explicitly set -DSTACK_DIRECTION=1 just to see what happens. Building goes on until it reaches the xtradb module. Then it fails with: CMake Error at storage/xtradb/CMakeLists.txt:434 (MESSAGE): Percona XtraDB is not supported on this platform (Here I'll just copy-paste something I wrote on maria-discuss list a few days ago) This error is triggered in storage/xtradb/CMakeLists.txt when XTRADB_OK is not set; it seems to me that XTRADB_OK is set in either one of the following cases: a) CMAKE_CROSSCOMPILING is NOT set and some tests are successful (thus setting HAVE_IB_GCC_ATOMIC_BUILTINS) or b) We're compiling for MSVC or CMAKE_SYSTEM_NAME is "SunOS" and some other conditions are met. Since I'm not compiling for MSVC or SunOS and I'm crosscompiling indeed, XTRADB_OK is not set and the error is triggered. Here I'm stuck. I've got the feeling that as such one cannot cross-compile xtradb because XTRADB_OK will never be set; or maybe with proper options passed to cmake the tests would be made elsewhere and HAVE_IB_GCC_ATOMIC_BUILTINS would be set beforehand? Or maybe I should patch this CMakeLists.txt? Or maybe I should just compile without xtradb altogether, if that's possible? Finally I just passed -DXTRADB_OK just to see what would happen, again (obviously there is little chance that it would result in anything usable). And reached problem #3: 3) Building jemalloc This one fails at configure time with: checking whether we are cross compiling... configure: error: in `/home/sylvain/DEPOTS/buildroot/output/build/mariadb-galera-10.0.12/extra/jemalloc/build': configure: error: cannot run C compiled programs. If you meant to cross compile, use `--host'. Somehow I was expecting that having passed -DCMAKE_CROSSCOMPILING=1 and the host being set by buildroot it would propagate down to the modules... Am I missing something? At this point I'm out of dirty hacks, even to "see what happens" ;) Cheers, - -- Sylvain Raybaud www.green-communications.fr -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJT4ke/AAoJEEkkwl4JtJ9yJW0QALBz3wj0VsfMtUBUabP13q74 ++y1AiCLZ0js+UGCqxB9PVz4yvhhnMAz8QnPpH37XJtDm+VaJBQTEZL65CHF3nlr dHe46/q/wswlbdhNsQu6JmMe3JoOe2TFbjDenQMiUvW9mOilt86ioveHVLGYbpnq 5LYMrPlYXUBpDcxEYXXbTpIHsAoHUrOE1Gj5AsxwbUvQuHDDr9M/Ws01iIXLbzT8 0pImhgBPYUwqaj9tJnPkls+9LzoOb2m269n9TCbmWk0ixakpHjeNMn5lnRDhOwNh AKkHrGdMUomSBca4G0kQoeNxwC2rCjO8+bRxjh7v6GTdME/sgsgn3yOv2EznldUr tFaO/JfraHSUgmiRyVo41U3uTESxZKRJYOV9IqUhl4HiS85D5oa/bD6l84rhx95J 52/hUoApoStr8tSk0tzXk1/UWJEQ3fUZ7XoXqgWq+c1uMgbgZftAAqAw7Xvmc88t tf69FqBzw5dpf8Kg0I7iAggBpBLIWGJwnlVvcmLY4UUE1cDFfZstMBSXaS155LQF VClK+ytE2Ef8oAB9/nlMXgg4b3WXlD0PRQDRTNIwCAzdvsuu+S0q64xLtw/JYnLl KPcLWfwXy+kRhT+vtUWtqMjjkSQ3e6rJDybK11vgYIAWm/Xfk5s8i0g7Bgmnf3wp JGYUAR285gY0XMqL6LmU =vBo4 -----END PGP SIGNATURE----- [View Less]

2 4

[Maria-developers] Valgrind failure in Buildbot
by Kristian Nielsen 30 Oct '14

30 Oct '14

Hi Sergey, We briefly discussed the new valgrind error in buildbot after your push on IRC: https://buildbot.askmonty.org/buildbot/builders/work-amd64-valgrind/builds/… ==13161== Conditional jump or move depends on uninitialised value(s) ==13161== at 0x4C280AA: memcpy (mc_replace_strmem.c:115) ==13161== by 0x723F6B: Field_varstring::get_key_image(unsigned char*, unsigned int, Field::imagetype) (field.cc:6932) ==13161== by 0x818D5B: key_copy(unsigned char*, unsigned char*, st_key*, … [View More]unsigned int, bool) (key.cc:146) ==13161== by 0x6DD6FE: Mrr_ordered_index_reader::interrupt_read() (multi_range_read.cc:448) ==13161== by 0x6DE605: Mrr_ordered_rndpos_reader::refill_from_index_reader() (multi_range_read.cc:688) ==13161== by 0x6DE7D2: Mrr_ordered_rndpos_reader::refill_buffer(bool) (multi_range_read.cc:618) ==13161== by 0x6DFB27: DsMrr_impl::dsmrr_init(handler*, st_range_seq_if*, void*, unsigned int, unsigned int, st_handler_buffer*) (multi_range_read.cc:966) ==13161== by 0x91AAE9: ha_innobase::multi_range_read_init(st_range_seq_if*, void*, unsigned int, unsigned int, st_handler_buffer*) (ha_innodb.cc:13961) ==13161== by 0x6DA60A: JOIN_TAB_SCAN_MRR::open() (sql_join_cache.cc:3867) ==13161== by 0x6D7363: JOIN_CACHE::join_matching_records(bool) (sql_join_cache.cc:2249) ==13161== by 0x6D5647: JOIN_CACHE::join_records(bool) (sql_join_cache.cc:2086) ==13161== by 0x5F8259: sub_select_cache(JOIN*, st_join_table*, bool) (sql_select.cc:16717) ==13161== by 0x6D71BA: JOIN_CACHE::generate_full_extensions(unsigned char*) (sql_join_cache.cc:2397) ==13161== by 0x6D74E6: JOIN_CACHE::join_matching_records(bool) (sql_join_cache.cc:2289) ==13161== by 0x6D5647: JOIN_CACHE::join_records(bool) (sql_join_cache.cc:2086) ==13161== by 0x5F8219: sub_select_cache(JOIN*, st_join_table*, bool) (sql_select.cc:16697) I took a quick look. The problem seems to be that the code is copying out the fields of a key from table->record[0] (key_copy()), however, the first length byte of the value is uninitialised. I think it's the field col_varchar_key in the table, testcase is appended below. I checked a bit back up the callstack. The field col_varchar_key is reported as uninitialised already at the start of Mrr_ordered_rndpos_reader::refill_from_index_reader(), but I'm not really familiar with when and how table->record[0] is supposed to be initialised during multi-range read. As you said on IRC, your recent patch is related to interrupt_read(), so probably best if you take a look and see if you can figure out something. Incidentally, I noticed something that looks wrong in key_copy(to_key, from_record, key_info, key_length, with_zerofill) The comments say that the key is copied from `from_record', however actually the function only works if the from_record passed in is table->record[0] (or more precisely, the same record as is installed in key_info->key_part->field). Because it checks the null bit in from_record, but copies the data from table->record[0]. However, this is not the problem in this particular failure (as in this case from_record == table->record[0]), just struck me as something that could easily cause surprises later... Below is a reduced test case that triggers the valgrind error. It did not trigger for me with a debug build, but _did_ trigger (in 5.5) with this build command: EXTRA_FLAGS="-O3 -fno-omit-frame-pointer -Wno-uninitialized" AM_EXTRA_MAKEFLAGS="VERBOSE=1" BUILD/compile-amd64-valgrind-max - Kristian. ----------------------------------------------------------------------- -- source include/have_xtradb.inc set optimizer_switch='mrr=on,mrr_sort_keys=on,index_condition_pushdown=on'; CREATE TABLE t1 ( pk int(11) NOT NULL AUTO_INCREMENT, col_int_key int(11) NOT NULL, col_varchar_key varchar(1) NOT NULL, col_varchar_nokey varchar(1) NOT NULL, PRIMARY KEY (pk), KEY col_int_key (col_int_key), KEY col_varchar_key (col_varchar_key,col_int_key) ) ENGINE=InnoDB; INSERT INTO t1 VALUES (10,8,'v','v'), (11,8,'f','f'), (12,5,'v','v'), (13,8,'s','s'), (14,8,'a','a'), (15,6,'p','p'), (16,7,'z','z'), (17,2,'a','a'), (18,5,'h','h'), (19,7,'h','h'), (20,2,'v','v'), (21,9,'v','v'), (22,142,'b','b'), (23,3,'y','y'), (24,0,'v','v'), (25,3,'m','m'), (26,5,'z','z'), (27,9,'n','n'), (28,1,'d','d'), (29,107,'a','a'); CREATE TABLE t2 ( pk int(11) NOT NULL AUTO_INCREMENT, col_int_key int(11) NOT NULL, col_varchar_key varchar(1) NOT NULL, col_varchar_nokey varchar(1) NOT NULL, PRIMARY KEY (pk), KEY col_int_key (col_int_key), KEY col_varchar_key (col_varchar_key,col_int_key) ) ENGINE=InnoDB; INSERT INTO t2 VALUES (10,3,'i','i'), (11,186,'x','x'), (12,1,'g','g'), (13,8,'q','q'), (14,226,'m','m'), (15,133,'p','p'), (16,6,'e','e'), (17,3,'t','t'), (18,8,'j','j'), (19,5,'h','h'), (20,7,'w','w'); set join_cache_level=6; set join_buffer_size=1536; SELECT count(*), sum(table1.col_int_key*table2.pk) FROM t2 AS table1, t1 AS table2, t2 AS table3 WHERE table3.col_varchar_nokey = table2.col_varchar_key AND table3.pk > table2.col_varchar_nokey ; ----------------------------------------------------------------------- [View Less]

1 0

Re: [Maria-developers] Rev 4323: Set proper alternative names on RHEL7: they should match Fedora names
by Sergei Golubchik 30 Oct '14

30 Oct '14

Hi, Sergey! I cannot verify whether rhel7 alternative names should match Fedora names or they should be something different. Assuming they should match, your patch is ok. On Oct 09, Sergey Vojtovich wrote: > ------------------------------------------------------------ > revno: 4323 > revision-id: svoj(a)mariadb.org-20141009112748-eq3xwoyobgnz0cxj > parent: svoj(a)mariadb.org-20141008141031-xv2x7vrq269202vf > committer: Sergey Vojtovich <svoj(a)mariadb.org> > branch … [View More]

2 2

[Maria-developers] Fwd: [debian-mysql] Backporting the mysql_no_login plugin
by Colin Charles 30 Oct '14

30 Oct '14

This seems to get a +1 for backporting by at least Honza (RH), so I am wondering if we do this in the 5.5 branch too, that is shipping in many distributions. Begin forwarded message: > From: "Norvald H. Ryeng" <norvald.ryeng(a)oracle.com> > Subject: [debian-mysql] Backporting the mysql_no_login plugin > Date: 24 October 2014 15:49:34 GMT+8 > To: "pkg-mysql-maint(a)lists.alioth.debian.org" <pkg-mysql-maint(a)lists.alioth.debian.org>, "Honza Horak" <hhorak(a)redhat.… [View More]com>, "Roman Drahtmueller" <draht(a)suse.de> > > Hi package maintainers, > > We have a new plugin in MySQL 5.7 that makes it possible to have > accounts that can't log in: > > CREATE USER foo@localhost IDENTIFIED WITH 'mysql_no_login'; > > The mysql_no_login plugin simply denies all login attempts. This is > useful for users that are created, e.g., to serve as proxy users, or > as owners of stored programs/functions, views or events. > > This new plugin doesn't fix known security defects in the server, but > does provide new and better means to harden security. Best practices > for security include application of least-required privileges, and in > some cases, that means no client connections for privileged > accounts. This new plugin provides means to implement such > restrictions in a standard way. > > Because of the security benefits, we'd like to discuss backporting it > to 5.6. Like you, we don't like big changes to GA releases, but this > time we think it has a good use case, it's safe and has a very low > risk of regressions: > > - Since this is a plugin, it doesn't touch server code > - All new code is in a plugin that must be enabled explicitly by the > DBA > - The code itself is very simple. It's only one line of "real" code > (unconditionally return authentication failure), plus necessary > plugin plumbing to fill out the plugin API. > > If we backport this to 5.6, there are multiple ways to avoid it: > > - Apply a patch from us to remove the plugin > - Don't build it > - Build it, but don't ship it > - Build and ship it, but don't use it (in any case, the DBA has to > enable it and alter the user accounts to use it) > > So what do you think about backporting this? The only thing you'll > notice is one more file in the plugins directory. > > Regards, > > Norvald > > _______________________________________________ > pkg-mysql-maint mailing list > pkg-mysql-maint(a)lists.alioth.debian.org > http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-mysql-maint -- Colin Charles, Chief Evangelist, MariaDB Corporation blog: http://bytebot.net/blog/| t: +6-012-204-3201 | Skype: colincharles [View Less]

2 1

Re: [Maria-developers] [Commits] b28e6bf: MDEV-6945 MDEV-6948 MDEV-6956 MDEV-6957 MDEV-6953 MDEV-6954 MDEV-6955
by Sergei Golubchik 29 Oct '14

29 Oct '14

Hi, Sanja! On Oct 29, sanja(a)mariadb.com wrote: > --- > mysql-test/r/set_statement.result | 14 ++++++++++++++ > mysql-test/r/set_statement_profiling.result | 4 ++++ > mysql-test/t/set_statement.test | 18 ++++++++++++++++++ > mysql-test/t/set_statement_profiling.test | 9 +++++++++ Did you put set_statement_debug.test in a separate commit? That's good. > sql/set_var.h | 4 +++- > sql/share/errmsg-utf8.txt … [View More] | 2 ++ > sql/sql_parse.cc | 5 +++++ > sql/sys_vars.cc | 20 +++++++++++--------- > sql/sys_vars.h | 1 + > 9 files changed, 67 insertions(+), 10 deletions(-) > > diff --git a/sql/share/errmsg-utf8.txt b/sql/share/errmsg-utf8.txt > index bb42807..1633169 100644 > --- a/sql/share/errmsg-utf8.txt > +++ b/sql/share/errmsg-utf8.txt > @@ -7113,3 +7113,5 @@ ER_STATEMENT_TIMEOUT 70100 > eng "Query execution was interrupted (max_statement_time exceeded)" > ER_SUBQUERIES_NOT_SUPPORTED 42000 > eng "%s does not support subqueries or stored functions." > +ER_SET_STATEMENT_NOT_SUPPORTED 42000 > + eng "Variable '%s' is not supported in SET STATEMENT." Use the text eng "The system variable %.200s cannot be set in SET STATEMENT." > diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc > index 6252e89..fd77e5f 100644 > --- a/sql/sys_vars.cc > +++ b/sql/sys_vars.cc > @@ -997,7 +997,7 @@ static bool check_master_connection(sys_var *self, THD *thd, set_var *var) > static Sys_var_session_lexstring Sys_default_master_connection( > "default_master_connection", > "Master connection to use for all slave variables and slave commands", > - SESSION_ONLY(default_master_connection), > + NO_SET_STMT SESSION_ONLY(default_master_connection), Why should this be prohibited? I'd rather have it supported. Use case: setting many connection variables for a single master, like in SET STATEMENT default_master_connection=foobar SET max_relay_log_size=10000000, sql_slave_skip_counter=5; Admittedly, this use case, while valid, isn't a strong argument at the moment - we don't have many per-master variables yet. When we'll have more, this will become more important. So, if fixing default_master_connection is not easy - keep it NO_SET_STMT, it can be enabled later. Why, exactly, does it fail? > NO_CMD_LINE, IN_SYSTEM_CHARSET, > DEFAULT(""), MAX_CONNECTION_NAME, ON_CHECK(check_master_connection)); > #endif > @@ -3527,12 +3528,12 @@ static Sys_var_bit Sys_unique_checks( > #ifdef ENABLED_PROFILING > static Sys_var_bit Sys_profiling( > "profiling", "profiling", > - SESSION_VAR(option_bits), NO_CMD_LINE, OPTION_PROFILING, > + NO_SET_STMT SESSION_VAR(option_bits), NO_CMD_LINE, OPTION_PROFILING, Why should this be prohibited? I'd rather have it supported. Use case: one usually needs to profile a single statement, not everything. > DEFAULT(FALSE)); > > static Sys_var_ulong Sys_profiling_history_size( > "profiling_history_size", "Limit of query profiling memory", > - SESSION_VAR(profiling_history_size), CMD_LINE(REQUIRED_ARG), > + NO_SET_STMT SESSION_VAR(profiling_history_size), CMD_LINE(REQUIRED_ARG), > VALID_RANGE(0, 100), DEFAULT(15), BLOCK_SIZE(1)); > #endif > > @@ -3566,7 +3567,8 @@ static bool check_skip_replication(sys_var *self, THD *thd, set_var *var) > > static Sys_var_bit Sys_skip_replication( > "skip_replication", "skip_replication", > - SESSION_ONLY(option_bits), NO_CMD_LINE, OPTION_SKIP_REPLICATION, > + NO_SET_STMT SESSION_ONLY(option_bits), > + NO_CMD_LINE, OPTION_SKIP_REPLICATION, Why should this be prohibited? I'd rather have it supported. Use case: skip replication of a single statement. A generic version of FLUSH NO_WRITE_TO_BINLOG TABLE REPAIR NO_WRITE_TO_BINLOG TABLE ANALYZE NO_WRITE_TO_BINLOG TABLE OPTIMIZE NO_WRITE_TO_BINLOG TABLE and all ALTER TABLE ... PARTITION variants: ALTER TABLE ... REBUILD PARTITION NO_WRITE_TO_BINLOG ALTER TABLE ... OPTIMIZE PARTITION NO_WRITE_TO_BINLOG ALTER TABLE ... ANALYZE PARTITION NO_WRITE_TO_BINLOG ALTER TABLE ... REPAIR PARTITION NO_WRITE_TO_BINLOG ALTER TABLE ... COALESCE PARTITION NO_WRITE_TO_BINLOG ALTER TABLE ... ADD PARTITION NO_WRITE_TO_BINLOG ALTER TABLE ... REORGANIZE PARTITION NO_WRITE_TO_BINLOG > DEFAULT(FALSE), NO_MUTEX_GUARD, NOT_IN_BINLOG, > ON_CHECK(check_skip_replication)); > > @@ -3688,7 +3690,7 @@ static ulonglong read_rand_seed(THD *thd) > static Sys_var_session_special Sys_rand_seed1( > "rand_seed1", "Sets the internal state of the RAND() " > "generator for replication purposes", > - sys_var::ONLY_SESSION, NO_CMD_LINE, > + NO_SET_STMT sys_var::ONLY_SESSION, NO_CMD_LINE, > VALID_RANGE(0, ULONG_MAX), BLOCK_SIZE(1), > NO_MUTEX_GUARD, IN_BINLOG, ON_CHECK(0), > ON_UPDATE(update_rand_seed1), ON_READ(read_rand_seed)); > @@ -3706,7 +3708,7 @@ static bool update_rand_seed2(THD *thd, set_var *var) > static Sys_var_session_special Sys_rand_seed2( > "rand_seed2", "Sets the internal state of the RAND() " > "generator for replication purposes", > - sys_var::ONLY_SESSION, NO_CMD_LINE, > + NO_SET_STMT sys_var::ONLY_SESSION, NO_CMD_LINE, > VALID_RANGE(0, ULONG_MAX), BLOCK_SIZE(1), > NO_MUTEX_GUARD, IN_BINLOG, ON_CHECK(0), > ON_UPDATE(update_rand_seed2), ON_READ(read_rand_seed)); I'd rather fix rand_seed1/rand_seed2 variables not to be special anymore. Like, move them to SV structure, remove special behavior and so on. Regards, Sergei [View Less]

1 0

Re: [Maria-developers] [Commits] b18ddd2: Added 'mariadb-galera-server' metapackage to install all
by Sergei Golubchik 28 Oct '14

28 Oct '14

Hi, Nirbhay! On Oct 23, nirbhay(a)skysql.com wrote: > > Added 'mariadb-galera-server' metapackage to install all > necessary Galera-related dependencies. > > --- > debian/dist/Debian/control | 13 ++++++++++--- > debian/dist/Ubuntu/control | 13 ++++++++++--- > 2 files changed, 20 insertions(+), 6 deletions(-) > > diff --git a/debian/dist/Debian/control b/debian/dist/Debian/control > index e68f027..bc48dbc 100644 > --- a/debian/dist/Debian/control > +++… [View More]

1 0