- developers - lists.mariadb.org

Re: [Maria-developers] [Commits] Rev 3565: MDEV-492: fixed incorrect error check. in file:///home/bell/maria/bzr/work-maria-5.3-MDEV-492/
by Sergei Golubchik 29 Aug '12

29 Aug '12

Hi, Oleksandr! On Aug 29, Oleksandr Byelkin wrote: > >> === modified file 'sql/sql_update.cc' > >> --- a/sql/sql_update.cc 2012-04-26 17:21:37 +0000 > >> +++ b/sql/sql_update.cc 2012-08-27 14:29:26 +0000 > >> @@ -872,7 +872,7 @@ int mysql_update(THD *thd, > >> id= thd->arg_of_last_insert_id_function ? > >> thd->first_successful_insert_id_in_prev_stmt : 0; > >> > >> - if (error < 0) > >> + if (error < 0 && (!thd->is_error())) > >> { > >> char buff[MYSQL_ERRMSG_SIZE]; > >> my_snprintf(buff, sizeof(buff), ER(ER_UPDATE_INFO), (ulong) found, (ulong) updated, > > Nope, this is not good. > > The bug happens because the error from remove_eq_conds() is ignored. > > You need to catch and process this error as soon as possible. > > While in your patch you let the update work all the way till the end. > > > When "update works all the way" it catched inside update loops, but was > that if update never make any update it ignores errors. > > So actually it should be my fix or both fixes, because errors > (especially such unexpected as "out of resources" could happened almost > anywhere, so issuing OK to the client without checking error state is a > mistake. I know. Before sending a review I actually tried to get a crash by generating an error in different places of the UPDATE. E.g. in the loop: INSERT t1 VALUES (1,2); UPDATE .... or in fill_record(): UPDATE t1 SET a=COLUMN_CREATE(...); or with double my_error(): ... WHERE COLUMN_CREATE(...) OR COLUMN_CREATE(...); all these cases were correctly taken care of, no crash, no ok packet. So, I think, it's enough to fix only that early failure that you have found. Regards, Sergei

1 0

Re: [Maria-developers] [Commits] Rev 3565: MDEV-492: fixed incorrect error check. in file:///home/bell/maria/bzr/work-maria-5.3-MDEV-492/
by Sergei Golubchik 28 Aug '12

28 Aug '12

Hi, Sanja! On Aug 27, sanja(a)askmonty.org wrote: > message: > MDEV-492: fixed incorrect error check. > === modified file 'mysql-test/t/errors.test' > --- a/mysql-test/t/errors.test 2012-03-12 07:56:56 +0000 > +++ b/mysql-test/t/errors.test 2012-08-27 14:29:26 +0000 > @@ -85,3 +85,12 @@ INSERT INTO t2 VALUES (1,0) ON DUPLICATE > INSERT INTO t2(a,b) VALUES (1,0) ON DUPLICATE KEY UPDATE > b=(SELECT VALUES(a)+2 FROM t1); > DROP TABLE t1, t2; > + > +--echo # > +--echo # MDEV-492: incorrect error check before sending OK in mysql_update > +--echo # > +CREATE TABLE t1 (a CHAR(3), b BLOB); > +--error ER_DYN_COL_DATA > +UPDATE t1 SET a = 'new' > +WHERE COLUMN_CREATE( 1, 'v', 1, 'w' ) IS NULL; > +drop table t1; > > === modified file 'sql/sql_update.cc' > --- a/sql/sql_update.cc 2012-04-26 17:21:37 +0000 > +++ b/sql/sql_update.cc 2012-08-27 14:29:26 +0000 > @@ -872,7 +872,7 @@ int mysql_update(THD *thd, > id= thd->arg_of_last_insert_id_function ? > thd->first_successful_insert_id_in_prev_stmt : 0; > > - if (error < 0) > + if (error < 0 && (!thd->is_error())) > { > char buff[MYSQL_ERRMSG_SIZE]; > my_snprintf(buff, sizeof(buff), ER(ER_UPDATE_INFO), (ulong) found, (ulong) updated, Nope, this is not good. The bug happens because the error from remove_eq_conds() is ignored. You need to catch and process this error as soon as possible. While in your patch you let the update work all the way till the end. This patch is better, it aborts much earlier: === modified file 'sql/sql_update.cc' --- sql/sql_update.cc 2012-08-09 15:22:00 +0000 +++ sql/sql_update.cc 2012-08-28 19:05:33 +0000 @@ -406,7 +406,7 @@ int mysql_update(THD *thd, table->file->info(HA_STATUS_VARIABLE | HA_STATUS_NO_LOCK); select= make_select(table, 0, 0, conds, 0, &error); - if (error || !limit || + if (error || !limit || thd->is_error() || (select && select->check_quick(thd, safe_update, limit))) { delete select; Regards, Sergei

1 0

Re: [Maria-developers] [Commits] Rev 3353: Performed re-factoring and re-structuring of the code for mwl#248: in file:///home/igor/maria/maria-5.5-mwl248-changes/
by Sergei Golubchik 28 Aug '12

28 Aug '12

Hi, Davi! On Jul 31, Davi Arnaut wrote: > Also, I would like to point out that my main argument a few e-mails > ago is that you cannot memset/bzero a non-POD type in C++ without > occurring in undefined/implementation-defined behavior, which was > ignored. This significantly lowers my confidence in MariaDB being a > high quality code base (save for some developers who tend to get it). :) After your first email I've checked that code. Perhaps I've missed something, but it did look like all bzero-ed structures in the igor's patch were of POD types. Regards, Sergei

2 3

[Maria-developers] MariaDB 5.3.8 Release Prep
by Daniel Bartholomew 28 Aug '12

28 Aug '12

All, The MariaDB 5.3.8 release is almost ready. The files have been uploaded and should be mirrored soon. I expect to send out the release announcement at around 10am U.S. Eastern time tomorrow (28 Aug 2012). As usual, I've created preliminary Release Notes and Changelog pages: http://kb.askmonty.org/en/mariadb-538-release-notes/ http://kb.askmonty.org/en/mariadb-538-changelog/ Please let me know ASAP if there are any particularly noteworthy items in this release which I should mention in the Release Notes. Thanks. -- Daniel Bartholomew Google+ - http://gplus.to/dbart Twitter - http://twitter.com/daniel_bart MariaDB: An Enhanced Drop-in Replacement for MySQL Website - http://mariadb.org Twitter - http://twitter.com/mariadb Google+ - http://google.com/+mariadb Facebook - http://fb.com/MariaDB.dbms Knowledgebase - http://kb.askmonty.org Monty Program - http://montyprogram.com

1 0

[Maria-developers] table_share->field vs table->field
by Markus Pilman 22 Aug '12

22 Aug '12

Hi all, I ran into a strange problem: I am working on a storage engine for MariaDB. To access field information I usually do as written in the documentation: for (Field** field = table->field; *field; ++field) { /* some code here */ } This works fine in most cases (for writing - read handler::write_row - it seems to work always) but sometimes (I think always in the index_next method) accessing **(table->field) results in a segmentation fault (but *(table->field) is not null, but something like 0x5e6f4c6d4578244d). Strange enough, this error does not happen always. In the debugger I saw, that table_share->field points to the correct location there so I replaces table->field in the above loop with table_share->field. But this also seems to not work - it now crashes on other queries (also on updates, but after restarting MariaDB it also crashes on a full table scan). So my question: how can I access the fields in a reliable manner? I need to do that to set/get the BLOB/TEXT fields. We have a method in our handler called set_blobs that resets the pointer in the record to the blobs, which looks like the following: void ha_kvstore::set_blobs(uchar *buf, kvid key, btree::buffer &raw_data) { auto current_read_pos = raw_data.const_ptr() + table_share->reclength; for (Field** field = table->field; *field; ++field) { if (!((*field)->flags & BLOB_FLAG)) continue; (*field)->table = table; Field_blob* blob = *((Field_blob**)field); uint32 packlength = blob->pack_length_no_ptr(); bmove((*field)->ptr + packlength, &current_read_pos, sizeof(char*)); current_read_pos += blob->get_length(); } } The storage engine always crashes in this code. What are we doing wrong? Any help is appreciated! Best Markus

2 2

[Maria-developers] Introduction / Os X Build Host
by Pablito 21 Aug '12

21 Aug '12

Hello I'm Pablito Vicente, I'm from Argentina, have been using Mysql for 12 years now as a web developer ( PHP mostly ). During the weekend I was trying to download a package for Mac and I notice brew was the only choice, that I used and worked but I would like to know if I can help generating builds for the latest versions and I will try to also investigate how to create packages so people can download a DMG image for easier installation. I will follow the instructions on: https://kb.askmonty.org/en/buildbot-setup-buildbot-setup-for-macosx/ to set it up but I was wondering if this will work when the computers are turned on only for this task. I don't usually leave computers on unless that's required. I could offer builds for Mountain Lion and Snow Leopard, though I think they're probably the same. well let me know if this can be of any help. I might send some emails for some help as I've been away from sys admin tasks for years. warm regards, Pablito

1 0

Re: [Maria-developers] Fwd: [Commits] Rev 3498: Fix bug mdev-447: Wrong output from the EXPLAIN command of the test case for lp bug #714999 in file:///home/tsk/mprog/src/5.5-md447/
by Sergei Petrunia 21 Aug '12

21 Aug '12

Hi Timour Ok to push. On Mon, Aug 20, 2012 at 02:47:35PM +0300, Timour Katchaounov wrote: > Sergey, > > Could you please review this simple patch. > It is a backport of several lines from your code. > > Timour > > ------------------------------------------------------------ > revno: 3498 > revision-id: timour(a)askmonty.org-20120820114533-hznbt0z7ikdxoawq > parent: knielsen(a)knielsen-hq.org-20120817123528-eggusbbjukhxshvs > committer: timour(a)askmonty.org > branch nick: 5.5-md447 > timestamp: Mon 2012-08-20 14:45:33 +0300 > message: > Fix bug mdev-447: Wrong output from the EXPLAIN command of the test case for lp bug #714999 > > The fix backports from MWL#182: Explain running statements the logic that > saves the original JOIN_TAB array of a query plan after optimization. This > array is later used during EXPLAIN to iterate over the original JOIN plan > nodes in the cases when this plan could be changed by early subquery > execution during the optimization phase of the outer query. > > > === modified file 'mysql-test/r/subselect.result' > --- a/mysql-test/r/subselect.result 2012-07-25 16:41:48 +0000 > +++ b/mysql-test/r/subselect.result 2012-08-20 11:45:33 +0000 > @@ -4196,7 +4196,7 @@ INSERT INTO t1 VALUES (1,1),(2,1); > EXPLAIN SELECT 1 FROM t1 WHERE a = (SELECT COUNT(*) FROM t1 GROUP BY b); > id select_type table type possible_keys key key_len ref rows Extra > 1 PRIMARY t1 ref a a 5 const 1 Using where; Using index > -2 SUBQUERY internal_tmp_table ALL group_key NULL NULL NULL 1 Using temporary; Using filesort > +2 SUBQUERY t1 ALL NULL NULL NULL NULL 2 Using temporary; Using filesort > DROP TABLE t1; > CREATE TABLE t1 (id int NOT NULL, st CHAR(2), INDEX idx(id)); > INSERT INTO t1 VALUES > > === modified file 'mysql-test/r/subselect_innodb.result' > --- a/mysql-test/r/subselect_innodb.result 2012-06-06 19:26:40 +0000 > +++ b/mysql-test/r/subselect_innodb.result 2012-08-20 11:45:33 +0000 > @@ -333,7 +333,7 @@ WHERE (SELECT DISTINCT b FROM t3) > 0); > id select_type table type possible_keys key key_len ref rows Extra > 1 PRIMARY t1 const PRIMARY PRIMARY 4 const 1 Using where; Using index > 2 SUBQUERY NULL NULL NULL NULL NULL NULL NULL Impossible WHERE > -3 SUBQUERY internal_tmp_table ALL group_key NULL NULL NULL 0 Using temporary > +3 SUBQUERY t3 ALL NULL NULL NULL NULL 1 Using temporary > SELECT * > FROM t1 > WHERE t1.a = ( > @@ -386,7 +386,7 @@ select 1 from t1 where 1 like (select 1 > id select_type table type possible_keys key key_len ref rows Extra > 1 PRIMARY t1 ALL NULL NULL NULL NULL 1 > 2 SUBQUERY t1 ALL NULL NULL NULL NULL 1 > -3 SUBQUERY internal_tmp_table ALL group_key NULL NULL NULL 1 Using temporary; Using filesort > +3 SUBQUERY t1 ALL NULL NULL NULL NULL 1 Using temporary; Using filesort > select 1 from t1 where 1 like (select 1 from t1 where 1 <=> (select 1 from t1 group by a1)); > 1 > 1 > > === modified file 'mysql-test/r/subselect_mat_cost_bugs.result' > --- a/mysql-test/r/subselect_mat_cost_bugs.result 2012-07-25 16:41:48 +0000 > +++ b/mysql-test/r/subselect_mat_cost_bugs.result 2012-08-20 11:45:33 +0000 > @@ -148,7 +148,7 @@ FROM t2 GROUP BY f1 > id select_type table type possible_keys key key_len ref rows Extra > 1 PRIMARY NULL NULL NULL NULL NULL NULL NULL Impossible WHERE noticed after reading const tables > 2 SUBQUERY t1 system NULL NULL NULL NULL 1 > -3 SUBQUERY internal_tmp_table ALL group_key NULL NULL NULL 1 Using temporary; Using filesort > +3 SUBQUERY t2 ALL NULL NULL NULL NULL 2 Using temporary; Using filesort > drop table t1, t2, t3; > # > # LP BUG#715034 Item_sum_distinct::clear(): Assertion `tree != 0' failed > > === modified file 'mysql-test/r/subselect_no_mat.result' > --- a/mysql-test/r/subselect_no_mat.result 2012-07-25 16:41:48 +0000 > +++ b/mysql-test/r/subselect_no_mat.result 2012-08-20 11:45:33 +0000 > @@ -4200,7 +4200,7 @@ INSERT INTO t1 VALUES (1,1),(2,1); > EXPLAIN SELECT 1 FROM t1 WHERE a = (SELECT COUNT(*) FROM t1 GROUP BY b); > id select_type table type possible_keys key key_len ref rows Extra > 1 PRIMARY t1 ref a a 5 const 1 Using where; Using index > -2 SUBQUERY internal_tmp_table ALL group_key NULL NULL NULL 1 Using temporary; Using filesort > +2 SUBQUERY t1 ALL NULL NULL NULL NULL 2 Using temporary; Using filesort > DROP TABLE t1; > CREATE TABLE t1 (id int NOT NULL, st CHAR(2), INDEX idx(id)); > INSERT INTO t1 VALUES > > === modified file 'mysql-test/r/subselect_no_opts.result' > --- a/mysql-test/r/subselect_no_opts.result 2012-07-25 16:41:48 +0000 > +++ b/mysql-test/r/subselect_no_opts.result 2012-08-20 11:45:33 +0000 > @@ -4196,7 +4196,7 @@ INSERT INTO t1 VALUES (1,1),(2,1); > EXPLAIN SELECT 1 FROM t1 WHERE a = (SELECT COUNT(*) FROM t1 GROUP BY b); > id select_type table type possible_keys key key_len ref rows Extra > 1 PRIMARY t1 ref a a 5 const 1 Using where; Using index > -2 SUBQUERY internal_tmp_table ALL group_key NULL NULL NULL 1 Using temporary; Using filesort > +2 SUBQUERY t1 ALL NULL NULL NULL NULL 2 Using temporary; Using filesort > DROP TABLE t1; > CREATE TABLE t1 (id int NOT NULL, st CHAR(2), INDEX idx(id)); > INSERT INTO t1 VALUES > > === modified file 'mysql-test/r/subselect_no_scache.result' > --- a/mysql-test/r/subselect_no_scache.result 2012-07-25 16:41:48 +0000 > +++ b/mysql-test/r/subselect_no_scache.result 2012-08-20 11:45:33 +0000 > @@ -4202,7 +4202,7 @@ INSERT INTO t1 VALUES (1,1),(2,1); > EXPLAIN SELECT 1 FROM t1 WHERE a = (SELECT COUNT(*) FROM t1 GROUP BY b); > id select_type table type possible_keys key key_len ref rows Extra > 1 PRIMARY t1 ref a a 5 const 1 Using where; Using index > -2 SUBQUERY internal_tmp_table ALL group_key NULL NULL NULL 1 Using temporary; Using filesort > +2 SUBQUERY t1 ALL NULL NULL NULL NULL 2 Using temporary; Using filesort > DROP TABLE t1; > CREATE TABLE t1 (id int NOT NULL, st CHAR(2), INDEX idx(id)); > INSERT INTO t1 VALUES > > === modified file 'mysql-test/r/subselect_no_semijoin.result' > --- a/mysql-test/r/subselect_no_semijoin.result 2012-07-25 16:41:48 +0000 > +++ b/mysql-test/r/subselect_no_semijoin.result 2012-08-20 11:45:33 +0000 > @@ -4196,7 +4196,7 @@ INSERT INTO t1 VALUES (1,1),(2,1); > EXPLAIN SELECT 1 FROM t1 WHERE a = (SELECT COUNT(*) FROM t1 GROUP BY b); > id select_type table type possible_keys key key_len ref rows Extra > 1 PRIMARY t1 ref a a 5 const 1 Using where; Using index > -2 SUBQUERY internal_tmp_table ALL group_key NULL NULL NULL 1 Using temporary; Using filesort > +2 SUBQUERY t1 ALL NULL NULL NULL NULL 2 Using temporary; Using filesort > DROP TABLE t1; > CREATE TABLE t1 (id int NOT NULL, st CHAR(2), INDEX idx(id)); > INSERT INTO t1 VALUES > > === modified file 'sql/sql_select.cc' > --- a/sql/sql_select.cc 2012-08-15 06:34:18 +0000 > +++ b/sql/sql_select.cc 2012-08-20 11:45:33 +0000 > @@ -271,8 +271,11 @@ Item_equal *find_item_equal(COND_EQUAL * > bool *inherited_fl); > JOIN_TAB *first_depth_first_tab(JOIN* join); > JOIN_TAB *next_depth_first_tab(JOIN* join, JOIN_TAB* tab); > -JOIN_TAB *first_breadth_first_tab(JOIN *join); > -JOIN_TAB *next_breadth_first_tab(JOIN *join, JOIN_TAB *tab); > + > +enum enum_exec_or_opt {WALK_OPTIMIZATION_TABS , WALK_EXECUTION_TABS}; > +JOIN_TAB *first_breadth_first_tab(JOIN *join, enum enum_exec_or_opt tabs_kind); > +JOIN_TAB *next_breadth_first_tab(JOIN *join, enum enum_exec_or_opt tabs_kind, > + JOIN_TAB *tab); > > /** > This handles SELECT with and without UNION. > @@ -6649,12 +6652,12 @@ double JOIN::get_examined_rows() > { > ha_rows examined_rows; > double prev_fanout= 1; > - JOIN_TAB *tab= first_breadth_first_tab(this); > + JOIN_TAB *tab= first_breadth_first_tab(this, WALK_OPTIMIZATION_TABS); > JOIN_TAB *prev_tab= tab; > > examined_rows= tab->get_examined_rows(); > > - while ((tab= next_breadth_first_tab(this, tab))) > + while ((tab= next_breadth_first_tab(this, WALK_OPTIMIZATION_TABS, tab))) > { > prev_fanout *= prev_tab->records_read; > examined_rows+= (ha_rows) (tab->get_examined_rows() * prev_fanout); > @@ -7269,23 +7272,30 @@ prev_record_reads(POSITION *positions, u > Enumerate join tabs in breadth-first fashion, including const tables. > */ > > -JOIN_TAB *first_breadth_first_tab(JOIN *join) > +JOIN_TAB *first_breadth_first_tab(JOIN *join, enum enum_exec_or_opt tabs_kind) > { > - return join->join_tab; /* There's always one (i.e. first) table */ > + /* There's always one (i.e. first) table */ > + return (tabs_kind == WALK_EXECUTION_TABS)? join->join_tab: > + join->table_access_tabs; > } > > > -JOIN_TAB *next_breadth_first_tab(JOIN *join, JOIN_TAB *tab) > +JOIN_TAB *next_breadth_first_tab(JOIN *join, enum enum_exec_or_opt tabs_kind, > + JOIN_TAB *tab) > { > + JOIN_TAB* const first_top_tab= first_breadth_first_tab(join, tabs_kind); > + const uint n_top_tabs_count= (tabs_kind == WALK_EXECUTION_TABS)? > + join->top_join_tab_count: > + join->top_table_access_tabs_count; > if (!tab->bush_root_tab) > { > /* We're at top level. Get the next top-level tab */ > tab++; > - if (tab < join->join_tab + join->top_join_tab_count) > + if (tab < first_top_tab + n_top_tabs_count) > return tab; > > /* No more top-level tabs. Switch to enumerating SJM nest children */ > - tab= join->join_tab; > + tab= first_top_tab; > } > else > { > @@ -7309,7 +7319,7 @@ JOIN_TAB *next_breadth_first_tab(JOIN *j > Ok, "tab" points to a top-level table, and we need to find the next SJM > nest and enter it. > */ > - for (; tab < join->join_tab + join->top_join_tab_count; tab++) > + for (; tab < first_top_tab + n_top_tabs_count; tab++) > { > if (tab->bush_children) > return tab->bush_children->start; > @@ -7333,7 +7343,7 @@ JOIN_TAB *first_top_level_tab(JOIN *join > > JOIN_TAB *next_top_level_tab(JOIN *join, JOIN_TAB *tab) > { > - tab= next_breadth_first_tab(join, tab); > + tab= next_breadth_first_tab(join, WALK_EXECUTION_TABS, tab); > if (tab && tab->bush_root_tab) > tab= NULL; > return tab; > @@ -7633,6 +7643,12 @@ get_best_combination(JOIN *join) > > join->top_join_tab_count= join->join_tab_ranges.head()->end - > join->join_tab_ranges.head()->start; > + /* > + Save pointers to select join tabs for SHOW EXPLAIN > + */ > + join->table_access_tabs= join->join_tab; > + join->top_table_access_tabs_count= join->top_join_tab_count; > + > update_depend_map(join); > DBUG_RETURN(0); > } > @@ -21389,8 +21405,8 @@ static void select_describe(JOIN *join, > bool printing_materialize_nest= FALSE; > uint select_id= join->select_lex->select_number; > > - for (JOIN_TAB *tab= first_breadth_first_tab(join); tab; > - tab= next_breadth_first_tab(join, tab)) > + for (JOIN_TAB *tab= first_breadth_first_tab(join, WALK_OPTIMIZATION_TABS); tab; > + tab= next_breadth_first_tab(join, WALK_OPTIMIZATION_TABS, tab)) > { > if (tab->bush_root_tab) > { > @@ -21473,16 +21489,8 @@ static void select_describe(JOIN *join, > else > { > TABLE_LIST *real_table= table->pos_in_table_list; > - /* > - Internal temporary tables have no corresponding table reference > - object. Such a table may appear in EXPLAIN when a subquery that needs > - a temporary table has been executed, and JOIN::exec replaced the > - original JOIN with a plan to access the data in the temp table > - (made by JOIN::make_simple_join). > - */ > - const char *tab_name= real_table ? real_table->alias : > - "internal_tmp_table"; > - item_list.push_back(new Item_string(tab_name, strlen(tab_name), cs)); > + item_list.push_back(new Item_string(real_table->alias, > + strlen(real_table->alias), cs)); > } > /* "partitions" column */ > if (join->thd->lex->describe & DESCRIBE_PARTITIONS) > > === modified file 'sql/sql_select.h' > --- a/sql/sql_select.h 2012-06-06 19:26:40 +0000 > +++ b/sql/sql_select.h 2012-08-20 11:45:33 +0000 > @@ -897,6 +897,19 @@ class JOIN :public Sql_alloc > > public: > JOIN_TAB *join_tab, **best_ref; > + > + /* > + For "Using temporary+Using filesort" queries, JOIN::join_tab can point to > + either: > + 1. array of join tabs describing how to run the select, or > + 2. array of single join tab describing read from the temporary table. > + > + SHOW EXPLAIN code needs to read/show #1. This is why two next members are > + there for saving it. > + */ > + JOIN_TAB *table_access_tabs; > + uint top_table_access_tabs_count; > + > JOIN_TAB **map2table; ///< mapping between table indexes and JOIN_TABs > JOIN_TAB *join_tab_save; ///< saved join_tab for subquery reexecution > > > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits -- BR Sergei -- Sergei Petrunia, Software Developer Monty Program AB, http://askmonty.org Blog: http://s.petrunia.net/blog

1 0

Re: [Maria-developers] [Commits] Rev 3353: Performed re-factoring and re-structuring of the code for mwl#248: in file:///home/igor/maria/maria-5.5-mwl248-changes/
by Kristian Nielsen 13 Aug '12

13 Aug '12

Davi Arnaut <davi(a)twitter.com> writes: > On Wed, Aug 1, 2012 at 12:46 AM, Kristian Nielsen > <knielsen(a)knielsen-hq.org> wrote: >> Davi Arnaut <davi(a)twitter.com> writes: >> >>> On Thu, Jul 26, 2012 at 5:50 PM, Igor Babaev <igor(a)askmonty.org> wrote: >>>> + bzero(column_stats, sizeof(Column_statistics) * cnt); >>> >>> That's not allowed for non-POD types. >> >> Can you elaborate on exactly when bzero() (or memset() or similar) is allowed >> or not allowed? Ok, so I got time to research this a bit more. The class in question is like this, the original has more members but this is the essential structure: class Column_statistics { private: static const uint Scale_factor_nulls_ratio= 100000; public: Field *min_value; private: ulong nulls_ratio; public: void set_all_nulls() { ... } }; The question is whether it is ok to allocate this with alloc_root() or similar, and initialise it with bzero(pointer, sizeof(Column_statistics)). (The issue of bzero vs. memset is a separate one, not discussed in this mail). Davi points out that this is not a POD. This is because a POD must be a C++ aggregate, and aggregates can not have private members. Unfortunately, it seems the question of whether bzero is allowed for a non-POD, is more difficult to answer. If we look to the standards, the answer is no. It is not well-defined to bzero() any data except integers. This is what the standard says. However, this is unhelpful for the case at hand, as bzero to initialise a simple struct is a well-established practice in systems programming and commonly accepted to be valid. It seems generally accepted that bzero() of a POD is acceptable. Since the only thing that makes Column_statistics not a POD is the presence of private: members, the question becomes: Should bzero initialisation of classes with private members be disallowed? I could not find any definitive reference to support such disallowance. I am not sure why private members would be disallowed in aggretate, and hence POD. My guess is because aggregates may be initialised with an initialiser list, which makes no sense for private member. Or maybe compilers could re-arrance private members to be placed after public members. Or compilers could optimise based on knowledge that no external code can modify a private member (so it does not need to be spilled over a call to pthread_mutex_lock() or similar). But none of this would give a problem with bzero-initialisation after alloc. So I do not know of a language standard/compiler argument to disallow bzero-initialisation of non-POD. ---- But I would still strongly advice to be very careful with such code, and I think Davi's original concern is similar to mine. The problem is that Column_statistics already looks a lot like a nicely encapsulated proper C++ class. It has private members, accessors, it even defines some constants. One could easily imagine that in the future, there will be the temptation to extend it even more so, like add a virtual function - and then we suddenly have a big problem. bzero() is most certainly _NOT_ ok for such class. So I would personally stay well away from this grey area, and rather - _either_ use only a simple struct with no member functions or private or anything like that - and use bzero/memcpy and such. - _or_ do a full class, but then only initialise it with proper call of contructors (whether explicitly or implicitly defined). That makes it much clearer whether a given struct class is one or the other. But I suppose the optimiser code already heavily uses a different strategy. At the least, the declaration of class Column_statistics should have a comment saying that it is initialised with bzero(), so it can not use features not compatible with this. The nice thing about Davi's suggested rule "no bzero of non-POD" is that it is simple, easy to remember, and easy to state. Which is the primary point of coding style guidelines in the first place: make the rules clear, so there is less wasted time discussing what they are. Hope this helps, - Kristian.

1 1

Re: [Maria-developers] [Commits] Rev 3353: Performed re-factoring and re-structuring of the code for mwl#248: in file:///home/igor/maria/maria-5.5-mwl248-changes/
by Michael Widenius 13 Aug '12

13 Aug '12

Hi! >>>>> "Davi" == Davi Arnaut <davi(a)twitter.com> writes: <cut> Davi> It's not well defined, that's one of the reasons why it was Davi> deprecated. It does not even exist anymore in some systems, Davi> POSIX.1-2008 removed the specification of bzero(). It's well defined in the sence that it works everywhere (at least for MySQL code). >> Better to always use the same code to reset memory than doing it many >> different ways. In MySQL code bzero() has been the offical and >> recommended way to reset memory. Davi> Not anymore, Oracle MySQL has addressed this issue. In MariaDB code that is still not the case. Personally, I find bzero() easier to read, instead of memset() as there is no extra parameter and it's also easier grep for it. I assume you agree that for all particular purposes, there is no reason to not use bzero() in MariaDB code? Regards, Monty

2 1

[Maria-developers] Thoughts on Global Transaction ID, parallel slave, and multi-source replication
by Kristian Nielsen 10 Aug '12

10 Aug '12

This is a writeup of some thoughts I have on getting Global Transaction ID in MariaDB, as well as some related thoughts on parallel slave and multi-source replication. MySQL 5.6 has global transaction ID now, and I looked into it in some details. Unfortunately, I _really_ did not like what I saw. There are two main problems, in my opinion: 1. The basic design is bad, leading to an excessively complicated implementation. 2. The implementation looks very incomplete/immature, to the point that I am amazed that anyone would push code like this into a main tree. Point 2 means that I really do not want to maintain this piece of **** in MariaDB, though we can hope that things will improve in subsequent releases. Point 1 leads me to suggest that instead of merging the MySQL 5.6 implementation, we could do our own implementation, using a design that is both much simpler, and at the same time more flexible that what is in MySQL 5.6. I will elaborate further on these points in the rest of the mail. ---- With respect to point 2, let me quote this little gem from the MySQL 5.6 code: /* Apparently this code is not being called. We need to investigate if this is a bug or this code is not necessary. /Alfranio */ DBUG_ASSERT(0); /NOTREACHED/ Now, certainly, there are dark corners of the MySQL replication code, and adding a comment/assert like this could be justified if a developer encounters it and finds it may be dead code. But this is _new_ code, added in a new 5.6 release! So the authors of the new feature, by their own statement, have no idea what the new code they are pushing into the main tree is actually doing. That is just insane. I have no idea what the replication team at MySQL is up to these days pulling stunts like this, I have to confess I am disappointed. I found many similar examples in the 5.6 code. I wrote up a more detailed analysis in TODO-171, where I elaborate more on the problems I see with the MySQL 5.6 Global Transaction ID code: https://mariadb.atlassian.net/browse/TODO-171 When the authors do not understand the code themselves, it really does not seem attractive for us to try to support and bugfix it. I also found a lot of what I think are left-over, unsused parts of an even more complicated design that was apparently abandoned (or postponed?), but left in the code, which does not make supporting this any easier. ---- To explain point 1, I first need to mention the MySQL 5.6 feature of parallel slave apply, as I believe that a major factor in the design of MySQL Global Transaction ID has been the need to interoperate with that feature. The MySQL parallel replication slave feature works so that transactions on tables in different databases can be run in parallel with each other (incidentally, there is some indication that the MySQL replication team considers this *the* meaning of parallel slave, which is sad, as it is just one piece of the general problem of replication slave scalability - it helps in some workloads, but it is useless for others). So with parallel slave, transactions on the master can be executed in a different order on the slave. This means that the crucial notion of a slaves current position in the replication stream becomes more complicated - it is not just a single point in the master binlog, rather every slave replication thread can be at a different point in the master binlog. MySQL 5.6 solves this by introducing "Gtid sets". A Gtid is a MySQL 5.6 global transaction ID - a unique, 24-byte identifier for every transaction. The state of a replication slave at any given moment is then the set Gtids of all transactions ever executed on that slave server This notion of having a set of all transactions ever executed really complicates the design. There is a need to represent such a set as basically intervals of "holes" in the Gtid sequence, to avoid the memory representation of such sets growing without bound over time. This in turns introduces the requirement that there can _never_ be holes in the sequence of Gtids generated on a master - if there ever is introduced a hole, slaves will wait around for ever waiting for a transaction containing the missing Gtid. I think recovering from errors with such a concept will be forever a nightmare. I think this basic fundament of the design is the main source of my dislike for the MySQL global transaction ID. It sounds nice on a first look, but once you start to think deeper on the problem it just does not seem to work well. ---- So I think I have a better idea of how to make Global Transaction ID work well with parallel slave based on executing transactions in different databases out-of-order. The MySQL Gtid is defined as a pair of server-id and sequence number. Every master server has a single server-id (despite no less than 128 bits being allocated for this - anyone planning a replication hierarchy with 2**128 servers?). The master generates a single stream of events in the binlog, though streams from other servers may be interleaved with it in case of multi-master. It is then the task of a slave to try to split up the single master stream in to multiple streams that can be executed in parallel. I think this is the root of the problem. The slave is the wrong place to do this splitting! It is on the master that we know how to interpret transactions (ie. that transactions in different databases are / are not independent), so doing this on the slave is harder. And two slaves can in general split differently, which makes it harder for a slave deeper in the replication hierarchy to switch to a new master (that master being itself a slave of a higher-up master), leading to the need for a complex set-of-all-executed-transactions concept. Suppose we instead make the decision on the _master_ on how to split up the replication stream for parallel apply. Instead of a single server-id for the master, we can have multiple - let us call them "Source ID". On the _master_, we will decide that two transactions in different databases are independent, and assign them different Source ID. The slave then receives multiple replication streams, and is free to execute each stream in parallel - or not, at the discretion of the DBA. Transactions _within_ a single stream are always executed in-order, so the slave has no need to remember any sets of all transactions ever executed - all it needs to remember is the sequence number within each stream it has seen - ie. last sequence number for each Source ID. This is a simple and obvious generalisation of the old concept of current binlog position. Downstream slaves see the exact same binlog sequences, each with transactions in the same order (though sequences may be interleaved differently) - so changing masters is simpler, all servers in the hierarchy have the same view. So to implement the same functionality as MySQL 5.6 parallel replication, all we need is on the master to generate Source ID as a hash of the database name (just as MySQL 5.6 assigns transactions to slave threads based on such hash of the database name). And implement a facility on the slave such that transactions with different Source ID can be executed in parallel by multiple slave threads. And now, since we decide on the master, we can provide much more flexibility for the application. We can have _some_ databases db1,db2 be executed in parallel on the slave - while other databases db3,db4 are considered dependent, and not modified out-of-order on the slave. Or the application can declare that two transactions are independent within the same database or even within the same table; for example log record inserts into a table can often be allowed to happen in different order on slaves. Or the DBA can assign a special Source ID to a long-runnign ALTER TABLE, and such ALTER TABLE can then execute independently on slaves to not stall the normal load. All we need is to provide a simple facility (eg. session variable) that applications/users can use to specify Source ID. And then the next step is multi-source replication, which is the much-request feature that one slave can replicate from two or more different masters at the same time. The different masters will just supply different Source IDs, so apart from the need to maintain multiple TCP connections, one to each master, from the rest of the code's point of view multi-source replication is no different, it is just more independent replication streams. So my suggestion is: rather than struggle to try and merge and support the MySQL 5.6 Global Transaction ID and parallel slave, let us instead implement something like the above idea, with multiple Source IDs on the master, and parallel execution of distinct replication streams on the slave. I believe this will be no more work, and the end result will be both simpler and more flexible. Thanks, - Kristian.

4 9