September 2013 - developers - lists.mariadb.org

[Maria-developers] Extendet FederatedX storage engine to support updating Sphinx RT indexes
by Markus Lidel 12 Jun '14

12 Jun '14

Hello, the Sphinx Search engine has RT indexes, which could be updated with a subset of SQL commands, refered to as SphinxQL: http://sphinxsearch.com/docs/2.1.1/sphinxql-reference.html Because the FederatedX storage engine uses some commands, which Sphinx does not know, here is a patch, wich extends the FederatedX storage engine to support this subset of commands. Now its possible to update Sphinx RT indexes directly from MariaDB. To create a connection to the RT index in the Sphinx default RT-index: index rt { type = rt path = @CONFDIR@/data/rt rt_field = title rt_field = content rt_attr_string = title rt_attr_string = content rt_attr_uint = gid } you just have to call: CREATE TABLE `rt` ( `id` BIGINT UNSIGNED NOT NULL, `title` TEXT, `content` TEXT, `gid` INT UNSIGNED ) ENGINE=FEDERATED CONNECTION='sphinxql://root@localhost:9306/rt'; It would be great, if the attached patch could be integrated into MariaDB. If you have any suggestions, please feel free to contact me. Best regards, Markus Lidel

4 8

Re: [Maria-developers] MDEV-4956 - Reduce usage of LOCK_open: TABLE_SHARE::tdc.used_tables
by Sergei Golubchik 03 Dec '13

03 Dec '13

Hi, Sergey! On Aug 27, Sergey Vojtovich wrote: > At lp:maria/10.0 > > ------------------------------------------------------------ > revno: 3807 > revision-id: svoj(a)mariadb.org-20130827121233-xh1uyhgfwbhedqyf > parent: jplindst(a)mariadb.org-20130823060357-pww92qxla7o8iir7 > committer: Sergey Vojtovich <svoj(a)mariadb.org> > branch nick: 10.0 > timestamp: Tue 2013-08-27 16:12:33 +0400 > message: > MDEV-4956 - Reduce usage of LOCK_open: TABLE_SHARE::tdc.used_tables > > - tc_acquire_table and tc_release_table do not access > TABLE_SHARE::tdc.used_tables anymore > - in tc_acquire_table(): release LOCK_tdc after we relase LOCK_open > (saves a few CPU cycles in critical section) > - in tc_release_table(): if we reached table cache threshold, evict > to-be-released table without moving it to unused_tables. unused_tables > must be empty at this point (added assertion). I don't understand what you're doing here, could you elaborate? E.g. explain in a changeset comment what you've done, why you introduced a new list for tables (all_share), what is its semantics, etc. Regards, Sergei

3 12

[Maria-developers] Wrong SQL thread position reporting after IO thread restart
by Pavel Ivanov 11 Oct '13

11 Oct '13

Krisitan, I found what I think is a bug in IO and SQL thread accounting. How to reproduce: 1) Set up two servers S1 and S2. S1 is a master, S2 is slave with master_using_gtid = current_pos. 2) Execute some transactions on the master, e.g. create database d; create table d.t (n int); insert into d.t values (1); 3) Both servers are at 0-1-3 now, SHOW SLAVE STATUS on S2 shows Read_Master_Log_Pos equals to Exec_Master_Log_Pos. 4) Execute STOP SLAVE IO_THREAD on S2. 5) S2 reports in the logs: "Slave I/O thread exiting, ... GTID position 0-1-2". So IO thread didn't realize that it received full transaction for 0-1-3 even though it didn't receive next GTID event. 6) Execute START SLAVE IO_THREAD on S2. 7) At this point SHOW SLAVE STATUS on S2 shows Read_Master_Log_Pos the same as in step 3, but Exec_Master_Log_Pos is now less than in step 3 as if SQL thread didn't catch up with IO thread yet. But despite both threads running and no more transactions executed on master Exec_Master_Log_Pos doesn't change and doesn't become equal to Read_Master_Log_Pos. This apparently happens because IO thread restarts from one transaction behind, adds to relay log Rotate event that master sends with the position of that transaction, but then doesn't add any events for the transaction because it knows they already were added into relay log. I think both problems are bugs. And although after fixing the first it would be really hard (if possible) to reproduce second, I'd think the reporting of SQL thread's position still should be fixed. Thank you, Pavel

2 2

[Maria-developers] Questions re MDEV-4736 and MDEV-4739 (was Re: Spider's installation sql file)
by Sergey Vojtovich 07 Oct '13

07 Oct '13

Hi Kentoku, I'm adding MariaDB developers to CC. On Thu, Sep 19, 2013 at 01:19:13AM +0900, kentoku wrote: > Hi Sergey, > > > But what kind of errors are possible in your case? Other storage engines > doesn't > seem to suffer from this API violation. > > Spider support bulk updating and deleting for avoiding network roundtrip > between data node. Some times, last bulk updating is executed in rnd_end() > function. So rnd_end() has possibility getting errors from data node. I'm afraid fixing rnd_end() callers in the server may stall for a long time. Is it accaptable for spider to use bulk updates and deletes API instead (see handler.h: start_bulk_update/start_bulk_delete). > By the way, about MDEV-4739. I get the following stack trace. > Program received signal SIGSEGV, Segmentation fault. > 0x00000000005eabf6 in thd_wait_begin (thd=0x29da060, > wait_type=10) > at /ssd1/mariadb-10.0.4/sql/sql_class.cc:4277 > 4277 MYSQL_CALLBACK(thd->scheduler, thd_wait_begin, (thd, wait_type)); > (gdb) print thd > $1 = (THD *) 0x29da060 > (gdb) bt > #0 0x00000000005eabf6 in thd_wait_begin ( > thd=0x29da060, wait_type=10) > at /ssd1/mariadb-10.0.4/sql/sql_class.cc:4277 > #1 0x000000000072a114 in scheduler_wait_sync_begin () > at /ssd1/mariadb-10.0.4/sql/scheduler.cc:59 > #2 0x0000000000d6dc20 in my_sync (fd=23, my_flags=0) > at /ssd1/mariadb-10.0.4/mysys/my_sync.c:76 > #3 0x0000000000d6b54f in my_msync (fd=23, > addr=0x7ffff7ff4000, len=4096, flags=4) > at /ssd1/mariadb-10.0.4/mysys/my_mmap.c:27 > #4 0x00000000008bea03 in TC_LOG_MMAP::open ( > this=0x16e6a00, opt_name=0xe19c87 "tc.log") > at /ssd1/mariadb-10.0.4/sql/log.cc:7735 > #5 0x00000000005751cb in init_server_components () > at /ssd1/mariadb-10.0.4/sql/mysqld.cc:4797 > #6 0x0000000000575a07 in mysqld_main (argc=30, > argv=0x1f204d0) > at /ssd1/mariadb-10.0.4/sql/mysqld.cc:5208 > #7 0x000000000056d884 in main (argc=11, > argv=0x7fffffffe3a8) > at /ssd1/mariadb-10.0.4/sql/main.cc:25 > (gdb) print thd->scheduler > $2 = (scheduler_functions *) 0x8f8f8f8f8f8f8f8f > (gdb) print thd_wait_begin > $3 = {void (THD *, > int)} 0x5eaba4 <thd_wait_begin(THD*, int)> > (gdb) print wait_type > $4 = 10 > > It is looks that "thd->scheduler" is not initialized. What do you think? > Must storage engine set it? Nope, it looks like a bug in thread pool. MDEV-4739 has different trace, how did you get this one? Just executed given test? Regards, Sergey

3 16

[Maria-developers] Replicating same server_id problem
by Pavel Ivanov 01 Oct '13

01 Oct '13

Kristian, Currently MariaDB (as well as MySQL of all previous versions) has a very big problem related to replicating same server_id. There is --replicate-same-server-id flag which as I understand (when set to 0) controls two things: 1) It doesn't allow slave to connect to a master with the same server_id. 2) Slave ignores all binlog events in the replication stream that have the same server_id as slave. And this flag cannot be set to 1 when --log-slave-updates is used. And that is a big problem. Consider the following scenario: let's say we have two servers S1 (master) and S2 (slave). Let's say at some moment in time they are completely in sync and you bring down S2 to take cold backup (you can even include binlogs in it). Then you bring it back up, S1 is still master. Now you execute some transactions, then you do a failover, make S2 master and execute some more transactions. Then you bring down S1, restore it from the backup taken earlier and connect to replicate from S2 again. At this point S1 will have to reply some transaction that have the same server_id as S1 has, but S1 doesn't have those transactions yet, so it shouldn't skip them, but it will. What do you think about how this should be fixed? As I understand you explicitly wanted to support replication cycles, so you still want the skipping of transactions with the same server_id to exist. But the situation above is a valid production use case. Maybe in GTID world it can be solved better? E.g. if transaction has the same server_id, but the GTID wasn't applied yet then it shouldn't be skipped? Thank you, Pavel

4 8

[Maria-developers] regexp review
by Oleksandr Byelkin 30 Sep '13

30 Sep '13

Hi! Hi! Everything is OK. But there are some small issues: --- mysql-test/include/ctype_utf8mb4.inc 2010-03-05 08:17:19 +0000 +++ mysql-test/include/ctype_utf8mb4.inc 2013-09-26 14:02:17 +0000 @@ -234,15 +234,15 @@ set names utf8mb4; set names utf8mb4; # This should return TRUE -select 'вася' rlike '[[:<:]]вася[[:>:]]'; -select 'вася ' rlike '[[:<:]]вася[[:>:]]'; -select ' вася' rlike '[[:<:]]вася[[:>:]]'; -select ' вася ' rlike '[[:<:]]вася[[:>:]]'; +select 'вася' rlike '\\bвася\\b'; +select 'вася ' rlike '\\bвася\\b'; +select ' вася' rlike '\\bвася\\b'; +select ' вася ' rlike '\\bвася\\b'; Is above unsupported pattern? === modified file 'sql/mysqld.cc' --- sql/mysqld.cc 2013-09-18 11:07:31 +0000 +++ sql/mysqld.cc 2013-09-26 14:02:17 +0000 @@ -1898,7 +1898,7 @@ void clean_up(bool print_message) delete global_rpl_filter; end_ssl(); vio_end(); - my_regex_end(); + //my_regex_end(); #if defined(ENABLED_DEBUG_SYNC) /* End the debug sync facility. See debug_sync.cc. */ debug_sync_end(); @@ -3904,10 +3904,10 @@ static int init_common_variables() return 1; item_init(); #ifndef EMBEDDED_LIBRARY - my_regex_init(&my_charset_latin1, check_enough_stack_size); + //my_regex_init(&my_charset_latin1, check_enough_stack_size); my_string_stack_guard= check_enough_stack_size; #else - my_regex_init(&my_charset_latin1, NULL); + //my_regex_init(&my_charset_latin1, NULL); #endif /* Process a comma-separated character set list and choose Remove it please (I think it was just forgotten).

2 1

[Maria-developers] Empty directory in mariadb's datadir is not handled correctly
by Honza Horak 30 Sep '13

30 Sep '13

Hi guys, quite tricky problem came up after a directory has been created by something else than mysqld in the datadir. Suppose we use datadir=/var/lib/mysql. When we create any file in that directory by hand (or some program does it, which is not just imagination, it really happens), e.g. /var/lib/mysql/.local, MariaDB (Oracle's MySQL will behave the same probably) server thinks this directory is a database, which is wrong. Then we get some weird messages and outputs: # mysqlshow +--------------------+ | Databases | +--------------------+ | information_schema | | #mysql50#.local | | mysql | | performance_schema | | test | +--------------------+ # mysqlcheck -A mysqlcheck: Got error: 1102: Incorrect database name '#mysql50#.local' when selecting the database I believe this is not how the things should work. I'm not sure, but I'd expect there is still some information about existing databases in the information_schema table. So IMHO the server/tool should always fetch such info in addition to pure testing if the database directory exists. Or is such testing simply impossible? Regards, Honza

2 1

Re: [Maria-developers] [All] MariaDB's best (download) month ever
by Kristian Nielsen 30 Sep '13

30 Sep '13

Colin Charles <byte(a)mariadb.com> writes: > It's 17.09 today, and we've hit 50,160 total downloads (this number is > considered company-internal, we don't share this number externally). "company-internal"? What have you been smoking? Colin, you are supposed to be the community guy for crying out lout :-( This is *completely*, utterly unacceptable. MariaDB is a community project. We are about openness. We are about working together, all of us, to create a better world. We are *not* some individuals or companys playground. So no, the community's usage numbers of its own project is not "internal", thank you very much. So please cut out the crap already. - Kristian.

6 11

[Maria-developers] Prep for MariaDB Galera 5.5.33a
by Daniel Bartholomew 28 Sep '13

28 Sep '13

All, I've begun the preparations for the MariaDB Galera 5.5.33a release. Draft release notes and changelog are here: - https://mariadb.com/kb/en/mariadb-galera-5533a-release-notes/ - https://mariadb.com/kb/en/mariadb-galera-5533a-changelog/ Please make necessary changes/additions/etc... to the above pages (or let me know). Thanks. -- Daniel Bartholomew, Sr Tech Writer / Sys Admin MariaDB | t: +1 919-809-6069 | Skype: daniel-bartholomew

1 0

[Maria-developers] MySQL bug affects mariaDB
by Peter Laursen 27 Sep '13

27 Sep '13

I think the 'mysql' client with MariaDB has a similar problem as it has with MySQL - refer? http://bugs.mysql.com/bug.php?id=70443 MariaDB installer installs my.ini to /datadir - what with MariaDB is a subfolder to /basedir. The server will look for it correctly as the registry key defining the service has a --default-file specification. But the command line clients don't read the registry key. @Wlad? -- Peter -- Webyog

3 7

[Maria-developers] EXPLAIN INSERT in MySQL - not useful after all?
by Sergei Petrunia 27 Sep '13

27 Sep '13

4 4

[Maria-developers] [GSoC] MariaDB Contributor Agreement for Kerberos Authentication Plugin
by QIU Shuang 27 Sep '13

27 Sep '13

Hi community, I declare that all the work done so far (and future) on the project Kerberize MariaDB is licenced by the new BSD Lisence. Please contact me if any problems with the license. Thank you all and best regards! Sincerely, Shuang

1 0

[Maria-developers] [GSoC] Request for code review
by QIU Shuang 27 Sep '13

27 Sep '13

Hi Sergei, This is Shuang, a participant for GSoC 2013 Kerberize MariaDB project. I want to ask the community to review the code before requesting for pull and send the MCA. How could I send the codes? In a diff file? or from other approaches? Thank you very much and best regards! Sincerely, Shuang

2 2

[Maria-developers] Review of base64.diff and german2.diff
by Michael Widenius 27 Sep '13

27 Sep '13

Hi! Here is the review for the code that we should put into 10.0 First the base64: > === modified file 'mysql-test/t/func_str.test' > --- mysql-test/t/func_str.test 2013-05-07 11:05:09 +0000 > +++ mysql-test/t/func_str.test 2013-08-28 13:14:24 +0000 > @@ -1555,3 +1555,118 @@ drop table t1,t2; > --echo # End of 5.5 tests > --echo # > > + > +--echo # > +--echo # Start of 5.6 tests > +--echo # Shouldn't this be start of 10.0 tests ? (I know that this code is from MySQL 5.6, but still for us this is 10.0...) > === modified file 'mysys/base64.c' > --- mysys/base64.c 2011-06-30 15:46:53 +0000 > +++ mysys/base64.c 2013-03-09 06:22:59 +0000 > @@ -1,5 +1,4 @@ > -/* Copyright (c) 2003-2008 MySQL AB, 2009 Sun Microsystems, Inc. > - Use is subject to license terms. > +/* Copyright (c) 2003, 2010, Oracle and/or its affiliates. All rights reserved. Removed 'all rights reserved'. You can't have that for GPL code. If there is any new code from us, please add a copyright message for the MariaDB foundation too! > > This program is free software; you can redistribute it and/or modify > it under the terms of the GNU General Public License as published by > @@ -25,6 +24,28 @@ static char base64_table[] = "ABCDEFGHIJ > "abcdefghijklmnopqrstuvwxyz" > "0123456789+/"; > > +/** > + * Maximum length base64_needed_encoded_length() > + * can handle without overflow. > + */ > +int > +base64_encode_max_arg_length() > +{ > +#if (SIZEOF_INT == 8) > + /* > + 6827690988321067803 -> 9223372036854775805 > + 6827690988321067804 -> -9223372036854775807 > + */ Please describe from where the following number comes from (so that anyone can recalculate and check the number if needed): > + return 0x5EC0D4C77B03531BLL > +#else > + /* > + 1589695686 -> 2147483646 > + 1589695687 -> -2147483645 > + */ > + return 0x5EC0D4C6; > +#endif > +} > + > > int > base64_needed_encoded_length(int length_of_data) > @@ -39,6 +60,21 @@ base64_needed_encoded_length(int length_ > } > > > +/** > + * Maximum length base64_needed_decoded_length() > + * can handle without overflow. > + */ > +int > +base64_decode_max_arg_length() > +{ The following also need a comment. > +#if (SIZEOF_INT == 8) > + return 0x2AAAAAAAAAAAAAAALL; > +#else > + return 0x2AAAAAAA; > +#endif > +} > + > + <cut> > +/** > + * Get the next character from a base64 encoded stream. > + * Skip spaces, then scan the next base64 character or a pad character > + * and collect bits into decoder->c. > + * > + * @param decoder Pointer to MY_BASE64_DECODER > + * @return > + * FALSE on success (a valid base64 encoding character found) > + * TRUE on error (unexpected character or unexpected end-of-input found) > + */ Add an empty line here. It would also be good to have a good description of the base64 format in this file so that one can understand what the functions are supposed to do (or at least a link to the specification). Some thing I figured out by asking bar on IRC: - We never generate spaces - When decoding, we allow spaces anywhere. - The end of an 64 base decoded string is always padded with '=' so that the final length is dividable with 4. > +static inline my_bool > +my_base64_decoder_getch(MY_BASE64_DECODER *decoder) Remove inline from the above; It's likely to make the code slower, not faster as this is a big function with lot of if's. > { > - char b[3]; > - size_t i= 0; > - char *dst_base= (char *)dst; > - char const *src= src_base; > - char *d= dst_base; > - size_t j; > + if (my_base64_decoder_skip_spaces(decoder)) > + return TRUE; /* End-of-input */ > > - while (i < len) > + if (!my_base64_add(decoder)) /* Valid base64 character found */ > { > - unsigned c= 0; > - size_t mark= 0; > + if (decoder->mark) > + { > + /* If we have scanned '=' already, then only '=' is valid */ > + DBUG_ASSERT(decoder->state == 3); > + decoder->error= 1; > + decoder->src--; > + return TRUE; /* expected '=', but encoding character found */ > + } > + decoder->state++; > + return FALSE; > + } If you want to have the funtion inline, then move the following to another not inline function; We don't need to generate code for this for every function call: > + > + /* Process error */ > + switch (decoder->state) > + { > + case 0: > + case 1: > + decoder->src--; > + return TRUE; /* base64 character expected */ > + break; > + > + case 2: > + case 3: > + if (decoder->src[-1] == '=') > + { > + decoder->error= 0; /* Not an error - it's a pad character */ > + decoder->mark++; > + } > + else > + { > + decoder->src--; > + return TRUE; /* base64 character or '=' expected */ > + } > + break; > + default: > + DBUG_ASSERT(0); > + return TRUE; /* Wrong state, should not happen */ > + } <cut> Add to the comment for base64_decode that the 'dst' buffer has to be at least 2 character longer than what is needed to decode src_base. > +int > +base64_decode(const char *src_base, size_t len, > + void *dst, const char **end_ptr, int flags) > +{ > + if (my_base64_decoder_getch(&decoder) || > + my_base64_decoder_getch(&decoder) || > + my_base64_decoder_getch(&decoder) || > + my_base64_decoder_getch(&decoder)) > + break; Generating the error handling with a switch for every of the above is wasteful. See previous comment. > + /* Return error if there are more non-space characters */ > + decoder.state= 0; > + if (!my_base64_decoder_skip_spaces(&decoder)) > + decoder.error= 1; > + > if (end_ptr != NULL) Note that base64_needed_decoded_length() used ceil() when it's not needed. <cut> > === modified file 'sql/item_strfunc.cc' > @@ -451,6 +452,101 @@ void Item_func_aes_decrypt::fix_length_a > set_persist_maybe_null(1); > } > > + > +void Item_func_to_base64::fix_length_and_dec() > +{ > + maybe_null= args[0]->maybe_null; > + collation.set(default_charset(), DERIVATION_COERCIBLE, MY_REPERTOIRE_ASCII); Maybe better to cast both arguments to (ulonglong) as the rest of the code is depending on this. Also, why is base64_encode_max_arg_length() int instead of uint or even better size_t? > + if (args[0]->max_length > (uint) base64_encode_max_arg_length()) > + { > + maybe_null= 1; > + fix_char_length_ulonglong((ulonglong) base64_encode_max_arg_length()); > + } > + else > + { > + int length= base64_needed_encoded_length((int) args[0]->max_length); > + DBUG_ASSERT(length > 0); Don't think assert is needed as the function gurantees it already. > + fix_char_length_ulonglong((ulonglong) length - 1); > + } > +} <cut> > +String *Item_func_from_base64::val_str(String *str) > +{ > + String *res= args[0]->val_str_ascii(str); > + bool too_long= false; > + int length; > + const char *end_ptr; > + > + if (!res || > + res->length() > (uint) base64_decode_max_arg_length() || > + (too_long= > + ((uint) (length= base64_needed_decoded_length((int) res->length())) > > + current_thd->variables.max_allowed_packet)) || > + tmp_value.alloc((uint) length) || > + (length= base64_decode(res->ptr(), (int) res->length(), > + (char *) tmp_value.ptr(), &end_ptr, 0)) < 0 || > + end_ptr < res->ptr() + res->length()) > + { Shouldn't we get a readable error or warning if the string contained wrong characters? Now it looks like we will just return null. Something like 'Malformed base64 string. Error at position %d" would be nice. > + null_value= 1; // NULL input, too long input, OOM, or badly formed input > + if (too_long) > + { > + push_warning_printf(current_thd, Sql_condition::WARN_LEVEL_WARN, > + ER_WARN_ALLOWED_PACKET_OVERFLOWED, > + ER(ER_WARN_ALLOWED_PACKET_OVERFLOWED), func_name(), > + current_thd->variables.max_allowed_packet); > + } > + return 0; > + } > + tmp_value.length((uint) length); > + null_value= 0; > + return &tmp_value; > +} <cut> Review of german2.diff === modified file 'include/my_global.h' --- include/my_global.h 2013-07-21 14:39:19 +0000 +++ include/my_global.h 2013-08-29 08:27:09 +0000 @@ -1218,4 +1218,11 @@ static inline double rint(double x) #define HAVE_EXTERNAL_CLIENT #endif /* EMBEDDED_LIBRARY */ + +enum loglevel { + ERROR_LEVEL= 0, + WARNING_LEVEL= 1, + INFORMATION_LEVEL= 2 +}; + #endif /* my_global_h */ Not sure why the above, which is a my_sys construct, should be in my_global.h. Please add at least a comment about this. === modified file 'mysys/charset.c' --- mysys/charset.c 2012-08-14 14:23:34 +0000 +++ mysys/charset.c 2013-08-29 12:38:44 +0000 @@ -66,6 +66,9 @@ static my_bool init_state_maps(struct ch if (!(cs->ident_map= ident_map= (uchar*) my_once_alloc(256, MYF(MY_WME)))) return 1; + state_map= (uchar *) cs->state_map; + ident_map= (uchar *) cs->ident_map; Why is the above needed? They are already set one line above. (In addition cs->state_map and cs->ident_map are already uchar *) --- strings/ctype-simple.c 2013-07-21 14:39:19 +0000 +++ strings/ctype-simple.c 2013-08-29 09:05:07 +0000 @@ -1163,12 +1163,12 @@ static int pcmp(const void * f, const vo return res; } -static my_bool create_fromuni(struct charset_info_st *cs, - void *(*alloc)(size_t)) +static my_bool +create_fromuni(struct charset_info_st *cs, + MY_CHARSET_LOADER *loader) { uni_idx idx[PLANE_NUM]; int i,n; - struct my_uni_idx_st *tab_from_uni; /* Check that Unicode map is loaded. @@ -1209,18 +1209,18 @@ static my_bool create_fromuni(struct cha for (i=0; i < PLANE_NUM; i++) { int ch,numchars; - uchar *tab; /* Skip empty plane */ if (!idx[i].nchars) break; numchars=idx[i].uidx.to-idx[i].uidx.from+1; - if (!(idx[i].uidx.tab= tab= (uchar*) - alloc(numchars * sizeof(*idx[i].uidx.tab)))) + if (!(idx[i].uidx.tab= (uchar *) + (loader->once_alloc) (numchars * + sizeof(*idx[i].uidx.tab)))) return TRUE; - bzero(tab,numchars*sizeof(*tab)); + bzero((char*) idx[i].uidx.tab, numchars * sizeof(*idx[i].uidx.tab)); for (ch=1; ch < PLANE_SIZE; ch++) { @@ -1228,32 +1228,32 @@ static my_bool create_fromuni(struct cha if (wc >= idx[i].uidx.from && wc <= idx[i].uidx.to && wc) { int ofs= wc - idx[i].uidx.from; - tab[ofs]= ch; + ((char *) idx[i].uidx.tab)[ofs]= ch; } } } Why remove tab to be a shortcut for idx[i].uidx.from ? Shouldn't it be faster and shorter to use tab than using idx[i].uidx.from? (At least in this place) -static const uint16 page0FCdata[]= { /* FC00 (3 weights per char) */ +static uint16 page0FCdata[]= { /* FC00 (3 weights per char) */ 0x134F,0x135E,0x0000, 0x134F,0x1364,0x0000, 0x134F,0x13B0,0x0000, 0x134F,0x13C7,0x0000, 0x134F,0x13C8,0x0000, 0x1352,0x135E,0x0000, 0x1352,0x1364,0x0000, 0x1352,0x1365,0x0000, 0x1352,0x13B0,0x0000, @@ -6006,7 +6005,7 @@ static const uint16 page0FCdata[]= { /* 0x1381,0x13C8,0x0000, 0x1382,0x13C7,0x0000, 0x1382,0x13C8,0x0000, 0x1364,0x13C7,0x0000 }; If we are not going to change the above, better to keep these as const! They will then be in the const segment and you will have protection from anyone trying to change these! <cut> +my_uca_add_contraction(MY_CONTRACTIONS *list, my_wc_t *wc, size_t len, + my_bool with_context) { - MY_CONTRACTIONS *list= (MY_CONTRACTIONS*) cs->contractions; MY_CONTRACTION *next= &list->item[list->nitems]; - DBUG_ASSERT(len == 2); /* We currently support only contraction2 */ - next->ch[0]= wc[0]; - next->ch[1]= wc[1]; + size_t i; + /* + Contraction is always at least 2 characters. + Contraction is never longer than MY_UCA_MAX_CONTRACTION, + which is guaranteed by using my_coll_rule_expand() with proper limit. + */ + DBUG_ASSERT(len > 1 && len <= MY_UCA_MAX_CONTRACTION); + for (i= 0; i < len; i++) + { + /* + We don't support contractions with U+0000. + my_coll_rule_expand() guarantees there're no U+0000 in a contraction. + */ + DBUG_ASSERT(wc[i] != 0); + next->ch[i]= wc[i]; + } + if (i < MY_UCA_MAX_CONTRACTION) + next->ch[i]= 0; /* Add end-of-line marker */ Wouldn't it make life easier to always have the marker there? (Ie, always have place for the marker). At least you can remove one if from the code. If there is more than one if to remove, then it's a simple optimzation do to... <cut> +static int +my_coll_parser_too_long_error(MY_COLL_RULE_PARSER *p, const char *name) +{ + my_snprintf(p->errstr, sizeof(p->errstr), "%s is too long", name); + return 0; +} You should limit '%s' to 64 characters so that one gets the name in the output even if it's over sizeof()... %-.64s .... <cut> Regards, Monty

7 40

[Maria-developers] Review request : MDEV-4476 (mytop tracker)
by Jean Weisbuch 26 Sep '13

26 Sep '13

Hi, I have been doing modifications to the MariaDB mytop version (which is shipped on the /scripts directory), especially for bugfix, small display optimizations, use when available the I_S.PROCESSLIST instead of SHOW FULL PROCESSLIST and recently to show 1/10th of seconds on queries running time on servers having the TIME_MS column on the PROCESSLIST. A more detailed list : - Control chars on queries are not displayed anymore to avoid possible command executions/mytop misbehaving : when a query having control chars (ASCII 000 to 037) in it is shown in mytop, it can act like a key was pressed and end up doing strange/unwanted things. - Display cannot overlap the terminal size anymore when fullqueries is activated. - If not all the queries are shown on screen, a "-- Truncated query list --" is shown on the bottom of the query list. - It is now using the informations from INFORMATION_SCHEMA.PROCESSLIST when available instead of the SHOW FULL PROCESSLIST output for a better modularity is a similar maneer as Innotop does. - The query time is now shown with the 1/10th of second precision on servers having the TIME_MS patch (MariaDB and Percona) for queries running for less than 10000 seconds. - Command summary view fixed to retreive/show only the Com_* values and not the Com* (was showing a "Compression" stats row for this boolean variable). - Better have_query_cache test in case other values than ON/OFF/DEMAND would be introduced. Another detail that could be good to fix for MariaDB 10 GA is that the script is shipped as mytop.sh on MariaDB while its a Perl script (it is for example shipped as /usr/bin/mytop on Debian/Ubuntu official packages of mytop). The JIRA task : https://mariadb.atlassian.net/browse/MDEV-4476 The GIT repository where i put the latest revisions (i only have been using it lately but i commented all the changes and bugfixes along the time on the JIRA task and could provide older revisions/diff if required) : https://github.com/jb-boin/mariadb/blob/5.5.30/scripts/mytop.sh Feel free to contact me for more informations. Thanks in advance, Jean Weisbuch

1 0

[Maria-developers] Review of MDEV-4506, parallel replication, part 1
by Michael Widenius 26 Sep '13

26 Sep '13

Hi! Here is the first part of the review. I have 1/2 of one file left (rpl_parallel.cc) to review but I wanted you to have a chance to read what I have done so far. I plan to finish the review tomorrow and start working on the code during the weekend. >>>>> "Kristian" == Kristian Nielsen <knielsen(a)knielsen-hq.org> writes: Kristian> Hi Monty, Kristian> Here is a copy of the status mail I sent earlier. The status is mostly still Kristian> current. Kristian> The code is in this tree: Kristian> lp:~maria-captains/maria/10.0-knielsen Kristian> In that tree, you can extract the parallel replication code as a single patch Kristian> with a command like this: Kristian> bzr diff -rrevid:knielsen@knielsen-hq.org-20130608103621-g91eaielv4n22aha Kristian> In the forwarded mail, I describe getting a 4.5 times speedup on a quad-core Kristian> laptop, so that's quite promising. === modified file 'mysql-test/r/mysqld--help.result' --- mysql-test/r/mysqld--help.result 2013-05-28 11:28:31 +0000 +++ mysql-test/r/mysqld--help.result 2013-08-29 08:18:22 +0000 <cut> @@ -783,6 +794,11 @@ --slave-net-timeout=# Number of seconds to wait for more data from any master/slave connection before aborting the read + --slave-parallel-threads=# + If non-zero, number of threads to spawn to apply in + parallel events on the slave that were group-committed on + the master or were logged with GTID in different + replication domains. What happens if --slave-parallel-threads is 0 ? Does this mean that there will be no parallel threads? Is this the same as 1 ? If same as 1, why not have the option to take values starting from 1 ? IRC: K> yes, if --slave-parallel-threads is 0, then the feature is disabled. The old code is used, and the SQL thread applies the events itself K> if --slave-parallel-threads is > 0, then a pool of threads are spawned. The SQL threads do not apply events themselves, they just put it in queues for the parallel threads to apply M> do we ever need to do this with only one thread? K> (we might want to give an error if user tries to do this, as it does not make much sense) M> So why not have the value from 1 to N M> where 1 is old code and > 1 is using new code ? K> That might make sense === modified file 'mysql-test/suite/perfschema/r/dml_setup_instruments.result' --- mysql-test/suite/perfschema/r/dml_setup_instruments.result 2013-03-26 09:35:34 +0000 +++ mysql-test/suite/perfschema/r/dml_setup_instruments.result 2013-08-29 08:18:22 +0000 @@ -38,14 +38,14 @@ order by name limit 10; NAME ENABLED TIMED wait/synch/cond/sql/COND_flush_thread_cache YES YES wait/synch/cond/sql/COND_manager YES YES +wait/synch/cond/sql/COND_parallel_entry YES YES +wait/synch/cond/sql/COND_prepare_ordered YES YES wait/synch/cond/sql/COND_queue_state YES YES wait/synch/cond/sql/COND_rpl_status YES YES +wait/synch/cond/sql/COND_rpl_thread YES YES +wait/synch/cond/sql/COND_rpl_thread_pool YES YES wait/synch/cond/sql/COND_server_started YES YES wait/synch/cond/sql/COND_thread_cache YES YES -wait/synch/cond/sql/COND_thread_count YES YES -wait/synch/cond/sql/Delayed_insert::cond YES YES -wait/synch/cond/sql/Delayed_insert::cond_client YES YES -wait/synch/cond/sql/Event_scheduler::COND_state YES YES Any idea why the above was deleted? Don't see anything in your patch that could affect that. <cut> === added file 'mysql-test/suite/rpl/t/rpl_parallel.test' --- mysql-test/suite/rpl/t/rpl_parallel.test 1970-01-01 00:00:00 +0000 +++ mysql-test/suite/rpl/t/rpl_parallel.test 2013-08-29 08:18:22 +0000 I didn't find any test results for rpl_parallel.test or rpl_paralell2 in your tree. Did you forget to commit these? I was also missing some comment in these files what they was testing. === modified file 'sql/log.cc' --- sql/log.cc 2013-06-06 15:51:28 +0000 +++ sql/log.cc 2013-08-29 08:18:22 +0000 @@ -6541,44 +6543,201 @@ MYSQL_BIN_LOG::write_transaction_to_binl } Could you please document in detail the arguments and return value for the following function. - I spent 3 hours to try to understand this function in detail and having a full description of the arguments would have helped a bit. bool -MYSQL_BIN_LOG::write_transaction_to_binlog_events(group_commit_entry *entry) +MYSQL_BIN_LOG::queue_for_group_commit(group_commit_entry *entry, + wait_for_commit *wfc) { + group_commit_entry *orig_queue; + wait_for_commit *list, *cur, *last; + /* To facilitate group commit for the binlog, we first queue up ourselves in the group commit queue. Then the first thread to enter the queue waits for the LOCK_log mutex, and commits for everyone in the queue once it gets the lock. Any other threads in the queue just wait for the first one to finish the commit and wake them up. Do you still use LOCK_log for this? It's a bit confusing to mention LOCK_log here when you take the LOCK_prepare_ordered in the function. If LOCK_log is required, shouldn't there be an assert for it in this function? + + To support in-order parallel replication with group commit, after we add + some transaction to the queue, we check if there were other transactions + already prepared to commit but just waiting for the first one to commit. + If so, we add those to the queue as well, transitively for all waiters. */ entry->thd->clear_wakeup_ready(); mysql_mutex_lock(&LOCK_prepare_ordered); <cut> + orig_queue= group_commit_queue; + + /* + Iteratively process everything added to the queue, looking for waiters, + and their waiters, and so on. If a waiter is ready to commit, we + immediately add it to the queue; if not we just wake it up. + + This would be natural to do with recursion, but we want to avoid + potentially unbounded recursion blowing the C stack, so we use the list + approach instead. Add here: cur->next_subsequent_commit is at this point an empty list. 'last' will always point to the last element of the list. We use this list to simulate recursion. I did find it confusing that here you threat next_subsequent_commit as a way to avoid recursion, but you use this list for other things in register_wait_for_prior_commit(). Suggestion: - If you want to reuse memory for the list, please use an union over next_subsequent_commit and have two names for it, so that you don't in the code use the same name for two different list. + */ + list= wfc; + cur= list; + last= list; + for (;;) + { + /* Add the entry to the group commit queue. */ + entry->next= group_commit_queue; + group_commit_queue= entry; + + if (entry->cache_mngr->using_xa) + { + DEBUG_SYNC(entry->thd, "commit_before_prepare_ordered"); + run_prepare_ordered(entry->thd, entry->all); + DEBUG_SYNC(entry->thd, "commit_after_prepare_ordered"); + } + + if (!cur) + break; // Can happen if initial entry has no wait_for_commit + Add a comment what the following if is testing (As there is several lists involved, the code is not that easy to understand) + if (cur->subsequent_commits_list) + { + bool have_lock; + wait_for_commit *waiter; + + mysql_mutex_lock(&cur->LOCK_wait_commit); + have_lock= true; + waiter= cur->subsequent_commits_list; + /* Check again, now safely under lock. */ + if (waiter) + { + /* Grab the list of waiters and process it. */ + cur->subsequent_commits_list= NULL; + do + { + wait_for_commit *next= waiter->next_subsequent_commit; + group_commit_entry *entry2= + (group_commit_entry *)waiter->opaque_pointer; + if (entry2) + { + /* + This is another transaction ready to be written to the binary + log. We can put it into the queue directly, without needing a + separate context switch to the other thread. We just set a flag + so that the other thread will know when it wakes up that it was + already processed. + + So put it at the end of the list to be processed in a subsequent + iteration of the outer loop. + */ + entry2->queued_by_other= true; + last->next_subsequent_commit= waiter; + last= waiter; + /* + As a small optimisation, we do not actually need to set + waiter->next_subsequent_commit to NULL, as we can use the + pointer `last' to check for end-of-list. + */ + } + else + { + /* + Wake up the waiting transaction. + + For this, we need to set the "wakeup running" flag and release + the waitee lock to avoid a deadlock, see comments on + THD::wakeup_subsequent_commits2() for details. + */ + if (have_lock) + { + cur->wakeup_subsequent_commits_running= true; + mysql_mutex_unlock(&cur->LOCK_wait_commit); + have_lock= false; Move have_lock= false to just after the 'if (have_lock)' - Easier to see that you are really reseting the variable. - Faster as there is no need to reload the variable from stack after mysql_mutex_unlock() function. - I try to nowadays in my code to always reset flag variables just after testing them to achive the above. + } + waiter->wakeup(); + } + waiter= next; + } while (waiter); + } You can optimze away one row, one 'if level' and one 'if' away by doing this: Move: 'cur->subsequent_commits_list= NULL;' before the if. Change 'if (waiter)' to 'while (waiter)' Remove 'do' and the corresponding 'while (waiter);' + if (have_lock) + mysql_mutex_unlock(&cur->LOCK_wait_commit); + } + if (cur == last) + break; Add a comment here of what is in next_subsequent_commit list. + cur= cur->next_subsequent_commit; + entry= (group_commit_entry *)cur->opaque_pointer; + DBUG_ASSERT(entry != NULL); } + + /* Now we need to clear the wakeup_subsequent_commits_running flags. */ + if (list) + { + for (;;) + { + if (list->wakeup_subsequent_commits_running) + { + mysql_mutex_lock(&list->LOCK_wait_commit); + list->wakeup_subsequent_commits_running= false; + mysql_mutex_unlock(&list->LOCK_wait_commit); Why do we need the mutex above? The only place where we test this variable, as far as I can find, is in wait_for_commit::register_wait_for_prior_commit() This thread doesn't know if the other thread is just before mysql_mutex_lock(&waitee->LOCK_wait_commit), inside the lock or after the lock. So when we reset the variable should not matter. But somehow this sounds strange. What happens if the above variable is reset just before mysql_mutex_lock(&waitee->LOCK_wait_commit) in wait_for_commit::register_wait_for_prior_commit() ? Isn't the waitee added to the list when it shouldn't? + } + if (list == last) + break; + list= list->next_subsequent_commit; + } + } + + if (opt_binlog_commit_wait_count > 0) + mysql_cond_signal(&COND_prepare_ordered); mysql_mutex_unlock(&LOCK_prepare_ordered); DEBUG_SYNC(entry->thd, "commit_after_release_LOCK_prepare_ordered"); + return orig_queue == NULL; +} + +bool +MYSQL_BIN_LOG::write_transaction_to_binlog_events(group_commit_entry *entry) +{ + wait_for_commit *wfc; + bool is_leader; + + wfc= entry->thd->wait_for_commit_ptr; + entry->queued_by_other= false; + if (wfc && wfc->waiting_for_commit) + { Add comment, something like: /* We have to wait for the threads in wfc->waiting_for_commit */ + mysql_mutex_lock(&wfc->LOCK_wait_commit); + /* Do an extra check here, this time safely under lock. */ + if (wfc->waiting_for_commit) + { + wfc->opaque_pointer= entry; + do + { + mysql_cond_wait(&wfc->COND_wait_commit, &wfc->LOCK_wait_commit); + } while (wfc->waiting_for_commit); + wfc->opaque_pointer= NULL; + } + mysql_mutex_unlock(&wfc->LOCK_wait_commit); + } <cut> @@ -6597,7 +6756,10 @@ MYSQL_BIN_LOG::write_transaction_to_binl if (next) { - next->thd->signal_wakeup_ready(); + if (next->queued_by_other) + next->thd->wait_for_commit_ptr->wakeup(); What does the above code mean ? (add a comment). Why do we wakeup the thread we where waiting for instead of ourselves? + else + next->thd->signal_wakeup_ready(); } else { <cut> @@ -6968,6 +7140,48 @@ MYSQL_BIN_LOG::write_transaction_or_stmt return 0; } + +void +MYSQL_BIN_LOG::wait_for_sufficient_commits() +{ + size_t count; + group_commit_entry *e; + group_commit_entry *last_head; + struct timespec wait_until; + + mysql_mutex_assert_owner(&LOCK_log); + mysql_mutex_assert_owner(&LOCK_prepare_ordered); + + count= 0; + for (e= last_head= group_commit_queue; e; e= e->next) + ++count; + if (count >= opt_binlog_commit_wait_count) + return; Assuming that the queue is bigger may be better: for (e= last_head= group_commit_queue, count=0; e ; e= e->next) if (++count >= opt_binlog_commit_wait_count) return; + mysql_mutex_unlock(&LOCK_log); Here you are going to take LOCK_log and LOCK_prepare_ordered in the wrong order. safe_mutex() should have noticed this. In other code you are first taking LOCK_log and then LOCK_prepare_ordered. If someone else is calling trx_group_commit_leader() while you are in this code you would get a mutex deadlock. + set_timespec_nsec(wait_until, (ulonglong)1000*opt_binlog_commit_wait_usec); + + for (;;) + { + int err; + group_commit_entry *head; + + err= mysql_cond_timedwait(&COND_prepare_ordered, &LOCK_prepare_ordered, + &wait_until); + if (err == ETIMEDOUT) + break; + head= group_commit_queue; + for (e= head; e && e != last_head; e= e->next) + ++count; + if (count >= opt_binlog_commit_wait_count) + break; + last_head= head; + } + + mysql_mutex_lock(&LOCK_log); +} <cut> === modified file 'sql/log_event.cc' --- sql/log_event.cc 2013-06-06 15:51:28 +0000 +++ sql/log_event.cc 2013-08-29 08:18:22 +0000 <cut> @@ -6101,6 +6110,18 @@ Gtid_log_event::Gtid_log_event(const cha domain_id= uint4korr(buf); buf+= 4; flags2= *buf; + if (flags2 & FL_GROUP_COMMIT_ID) + { + if (event_len < (uint)header_size + GTID_HEADER_LEN + 2) + { + seq_no= 0; // So is_valid() returns false + return; + } + ++buf; + commit_id= uint8korr(buf); + } + else + commit_id= 0; } Setting commit_id=0 first instead in the if will generate smaller and faster code (setting a variable is faster than an a jmp). === modified file 'sql/log_event.h' --- sql/log_event.h 2013-06-06 15:51:28 +0000 +++ sql/log_event.h 2013-08-29 08:18:22 +0000 @@ -1317,9 +1317,9 @@ class Log_event @see do_apply_event */ - int apply_event(Relay_log_info const *rli) + int apply_event(struct rpl_group_info *rgi) What is the simple rule to know when to use Relay_log_info and when to use rpl_group_info ? If I understand things correct, relay_log_info should only be used for direct manipulation of data in the relay log. By the way, you should probably change 'struct rpl_group_info' to just rpl_group_info; Now you are mixing both versions in the code. === modified file 'sql/log_event_old.cc' --- sql/log_event_old.cc 2013-02-19 10:45:29 +0000 +++ sql/log_event_old.cc 2013-08-29 08:18:22 +0000 @@ -67,7 +68,7 @@ Old_rows_log_event::do_apply_event(Old_r do_apply_event(). We still check here to prevent future coding errors. */ - DBUG_ASSERT(rli->sql_thd == ev_thd); + DBUG_ASSERT(rgi->thd == ev_thd); Should we change all test for rli->sql_thd to rgi->thd ? You have at least this test in log_event.c and slave.cc. I assume that rli should not have a sql_thd member anymore ? <cut> === added file 'sql/rpl_parallel.cc' --- sql/rpl_parallel.cc 1970-01-01 00:00:00 +0000 +++ sql/rpl_parallel.cc 2013-08-29 08:18:22 +0000 @@ -0,0 +1,699 @@ +#include "my_global.h" +#include "rpl_parallel.h" +#include "slave.h" +#include "rpl_mi.h" + + +/* + Code for optional parallel execution of replicated events on the slave. + + ToDo list: + + - Review every field in Relay_log_info, and all code that accesses it. + Split out the necessary parts into rpl_group_info, to avoid conflicts + between parallel execution of events. (Such as deferred events ...) + + - Error handling. If we fail in one of multiple parallel executions, we + need to make a best effort to complete prior transactions and roll back + following transactions, so slave binlog position will be correct. + And all the retry logic for temporary errors like deadlock. + + - Stopping the slave needs to handle stopping all parallel executions. And + the logic in sql_slave_killed() that waits for current event group to + complete needs to be extended appropriately... + + - Audit the use of Relay_log_info::data_lock. Make sure it is held + correctly in all needed places also when using parallel replication. + + - We need some user-configurable limit on how far ahead the SQL thread will + fetch and queue events for parallel execution (otherwise if slave gets + behind we will fill up memory with pending malloc()'ed events). + + - Fix update of relay-log.info and master.info. In non-GTID replication, + they must be serialised to preserve correctness. In GTID replication, we + should not update them at all except at slave thread stop. + + - All the waits (eg. in struct wait_for_commit and in + rpl_parallel_thread_pool::get_thread()) need to be killable. And on kill, + everything needs to be correctly rolled back and stopped in all threads, + to ensure a consistent slave replication state. I can add kill checking in all functions that waits for a mutex. Will do it with adding enter_cond()/exit_cond() in all functions waiting for mysql_cond_wait(). + + - Handle the case of a partial event group. This occurs when the master + crashes in the middle of writing the event group to the binlog. The + slave rolls back the transaction; parallel execution needs to be able + to deal with this wrt. commit_orderer and such. + + - Relay_log_info::is_in_group(). This needs to be handled correctly in all + callers. I think it needs to be split into two, one version in + Relay_log_info to be used from next_event() in slave.cc, one to be used in + per-transaction stuff. + + - We should fail if we connect to the master with opt_slave_parallel_threads + greater than zero and master does not support GTID. Just to avoid a bunch + of potential problems, we won't be able to do any parallel replication + in this case anyway. If the master doesn't support GTID, I think we should instead of stopping, just issue a warning in the log and continue with normal replication. Otherwise we would have problems when using multi-source replication if a single slave doesn't support GTID. +*/ + +struct rpl_parallel_thread_pool global_rpl_thread_pool; + + +static void +rpt_handle_event(rpl_parallel_thread::queued_event *qev, + struct rpl_parallel_thread *rpt) +{ + int err; + struct rpl_group_info *rgi= qev->rgi; + Relay_log_info *rli= rgi->rli; + THD *thd= rgi->thd; + + thd->rli_slave= rli; + thd->rpl_filter = rli->mi->rpl_filter; + /* ToDo: Get rid of rli->group_info, it is not thread safe. */ + rli->group_info= rgi; + + /* ToDo: Access to thd, and what about rli, split out a parallel part? */ + mysql_mutex_lock(&rli->data_lock); + err= apply_event_and_update_pos(qev->ev, thd, rgi, rpt); We should also get rid of the thd argument and change the function to be a bool so that we can return an error. + /* ToDo: error handling. */ +} + + +pthread_handler_t +handle_rpl_parallel_thread(void *arg) +{ <cut> + mysql_mutex_lock(&rpt->LOCK_rpl_thread); + rpt->thd= thd; + + while (rpt->delay_start) + mysql_cond_wait(&rpt->COND_rpl_thread, &rpt->LOCK_rpl_thread); A much easier way to achive the wait is to have the caller lock mysql_mutex_lock(&rpt->LOCK_rpl_thread) and then just release it when it's time for this thread to continue. - No need for signaling or checking if the thread got the signal - No need to have a cond wait here - No need to have a delay_start flag + rpt->running= true; + + while (!rpt->stop && !thd->killed) + { + rpl_parallel_thread *list; + + old_msg= thd->proc_info; + thd->enter_cond(&rpt->COND_rpl_thread, &rpt->LOCK_rpl_thread, + "Waiting for work from SQL thread"); + while (!rpt->stop && !thd->killed && !(events= rpt->event_queue)) + mysql_cond_wait(&rpt->COND_rpl_thread, &rpt->LOCK_rpl_thread); Add comment: /* Mark that this thread is now executing */ + rpt->free= false; + rpt->event_queue= rpt->last_in_queue= NULL; + thd->exit_cond(old_msg); + Note that events may be undefined here if thd->killed was set above (for the first loop) It would be good to check for thd->killed or rpt->stop here and then abort the loop. Another question: do we really need rpt->stop ? Isn't it enough with using the thd->killed for this ? Otherwise we would need to check for both rpt->top and thd->killed in every loop that may be aborted. + more_events: + while (events) + { + struct rpl_parallel_thread::queued_event *next= events->next; + Log_event_type event_type= events->ev->get_type_code(); + rpl_group_info *rgi= events->rgi; + rpl_parallel_entry *entry= rgi->parallel_entry; + uint64 wait_for_sub_id; + uint64 wait_start_sub_id; + bool end_of_group; + Add a comment why this is handled here and not in rpt_handle_event() + if (event_type == GTID_EVENT) + { + in_event_group= true; Add a comment what the following variable stands for + group_standalone= + (0 != (static_cast<Gtid_log_event *>(events->ev)->flags2 & + Gtid_log_event::FL_STANDALONE)); + Rest of this file will be reviewed tomorrow. <cut> === modified file 'sql/rpl_rli.cc' --- sql/rpl_rli.cc 2013-06-06 15:51:28 +0000 +++ sql/rpl_rli.cc 2013-08-29 08:18:22 +0000 @@ -1194,13 +1194,15 @@ bool Relay_log_info::cached_charset_comp void Relay_log_info::stmt_done(my_off_t event_master_log_pos, - time_t event_creation_time, THD *thd) + time_t event_creation_time, THD *thd, + struct rpl_group_info *rgi) Do we need thd anymore as this is in rgi->thd? If they are not the same, please add a comment about this. It's not otherwise clear when to use rgi->thd and thd as this function uses both. @@ -1265,7 +1267,7 @@ void Relay_log_info::cleanup_context(THD { DBUG_ENTER("Relay_log_info::cleanup_context"); - DBUG_ASSERT(sql_thd == thd); + DBUG_ASSERT(opt_slave_parallel_threads > 0 || sql_thd == thd); Add a comment which THD is passed to the function. <cut> + +int +event_group_new_gtid(rpl_group_info *rgi, Gtid_log_event *gev) +{ + uint64 sub_id= rpl_global_gtid_slave_state.next_subid(gev->domain_id); + if (!sub_id) + { Add a comment that this is an end of memory error. <cut> + +void +delete_or_keep_event_post_apply(Relay_log_info *rli, + Log_event_type typ, Log_event *ev) +{ + /* + ToDo: This needs to work on rpl_group_info, not Relay_log_info, to be + thread-safe for parallel replication. + */ + switch (typ) { + case FORMAT_DESCRIPTION_EVENT: + /* + Format_description_log_event should not be deleted because it + will be used to read info about the relay log's format; + it will be deleted when the SQL thread does not need it, + i.e. when this thread terminates. + */ + break; + case ANNOTATE_ROWS_EVENT: + /* + Annotate_rows event should not be deleted because after it has + been applied, thd->query points to the string inside this event. + The thd->query will be used to generate new Annotate_rows event + during applying the subsequent Rows events. + */ + rli->set_annotate_event((Annotate_rows_log_event*) ev); I assume we should create relay_group_info->set_annotate_event instead of the above. + break; + case DELETE_ROWS_EVENT: + case UPDATE_ROWS_EVENT: + case WRITE_ROWS_EVENT: + /* + After the last Rows event has been applied, the saved Annotate_rows + event (if any) is not needed anymore and can be deleted. + */ + if (((Rows_log_event*)ev)->get_flags(Rows_log_event::STMT_END_F)) + rli->free_annotate_event(); + /* fall through */ + default: + DBUG_PRINT("info", ("Deleting the event after it has been executed")); + if (!rli->is_deferred_event(ev)) + delete ev; I assume that is_deferred_event should also be moved to relay_group_info. + break; + } +} + #endif === modified file 'sql/rpl_rli.h' --- sql/rpl_rli.h 2013-06-06 15:51:28 +0000 +++ sql/rpl_rli.h 2013-08-29 08:18:22 +0000 @@ -22,6 +22,7 @@ #include "log.h" /* LOG_INFO, MYSQL_BIN_LOG */ #include "sql_class.h" /* THD */ #include "log_event.h" +#include "rpl_parallel.h" struct RPL_TABLE_LIST; class Master_info; @@ -52,6 +53,8 @@ class Master_info; *****************************************************************************/ +struct rpl_group_info; + class Relay_log_info : public Slave_reporting_capability { public: @@ -311,13 +314,9 @@ class Relay_log_info : public Slave_repo char slave_patternload_file[FN_REFLEN]; size_t slave_patternload_file_size; - /* - Current GTID being processed. - The sub_id gives the binlog order within one domain_id. A zero sub_id - means that there is no active GTID. - */ - uint64 gtid_sub_id; - rpl_gtid current_gtid; + /* ToDo: We need to remove this, always use the per-transaction one to work with parallel replication. */ + struct rpl_group_info *group_info; + rpl_parallel parallel; Relay_log_info(bool is_slave_recovery); ~Relay_log_info(); @@ -459,7 +458,8 @@ class Relay_log_info : public Slave_repo the <code>Seconds_behind_master</code> field. */ void stmt_done(my_off_t event_log_pos, - time_t event_creation_time, THD *thd); + time_t event_creation_time, THD *thd, + struct rpl_group_info *rgi); /** @@ -594,6 +594,60 @@ class Relay_log_info : public Slave_repo }; +/* + This is data for various state needed to be kept for the processing of + one event group in the SQL thread. + + For single-threaded replication it is linked from the RLI, for parallel + replication it is linked into each event group being executed in parallel. +*/ Could you please rewrite the above to make it more clear. After discussing with you, I think the following could be added: /* In single-threaded replication, there will be one global rpl_group_info and one global RPLI per master connection. They will be linked together. In parallel replication, there will be one rpl_group_info object for each running thd. All rpl_group_info will share the same RLI. */ +struct rpl_group_info +{ + Relay_log_info *rli; + THD *thd; <cut> === modified file 'sql/sql_class.cc' --- sql/sql_class.cc 2013-06-06 15:51:28 +0000 +++ sql/sql_class.cc 2013-08-29 08:18:22 +0000 +void +wait_for_commit::wakeup() +{ + /* + We signal each waiter on their own condition and mutex (rather than using + pthread_cond_broadcast() or something like that). + + Otherwise we would need to somehow ensure that they were done + waking up before we could allow this THD to be destroyed, which would + be annoying and unnecessary. + */ + mysql_mutex_lock(&LOCK_wait_commit); + waiting_for_commit= false; + mysql_cond_signal(&COND_wait_commit); + mysql_mutex_unlock(&LOCK_wait_commit); +} In this particular case it looks safe to move the cond signal out of the mutex. I don't see how anyone could miss the signal. + + +/* + Register that the next commit of this THD should wait to complete until + commit in another THD (the waitee) has completed. + + The wait may occur explicitly, with the waiter sitting in + wait_for_prior_commit() until the waitee calls wakeup_subsequent_commits(). + + Alternatively, the TC (eg. binlog) may do the commits of both waitee and + waiter at once during group commit, resolving both of them in the right + order. + + Only one waitee can be registered for a waiter; it must be removed by + wait_for_prior_commit() or unregister_wait_for_prior_commit() before a new + one is registered. But it is ok for several waiters to register a wait for + the same waitee. It is also permissible for one THD to be both a waiter and + a waitee at the same time. +*/ +void +wait_for_commit::register_wait_for_prior_commit(wait_for_commit *waitee) +{ + waiting_for_commit= true; + DBUG_ASSERT(!this->waitee /* No prior registration allowed */); + this->waitee= waitee; + + mysql_mutex_lock(&waitee->LOCK_wait_commit); + /* + If waitee is in the middle of wakeup, then there is nothing to wait for, + so we need not register. This is necessary to avoid a race in unregister, + see comments on wakeup_subsequent_commits2() for details. + */ + if (waitee->wakeup_subsequent_commits_running) + waiting_for_commit= false; + else + { Please add a comment here what the following code is doing. As this is two different lists involved, it's not trivial code. You also seam to use the list next_subsequent_commit for something different in queue_for_group_commit() code. + this->next_subsequent_commit= waitee->subsequent_commits_list; + waitee->subsequent_commits_list= this; + } + mysql_mutex_unlock(&waitee->LOCK_wait_commit); +} <cut> +/* + Wakeup anyone waiting for us to have committed. + + Note about locking: + + We have a potential race or deadlock between wakeup_subsequent_commits() in + the waitee and unregister_wait_for_prior_commit() in the waiter. + + Both waiter and waitee needs to take their own lock before it is safe to take + a lock on the other party - else the other party might disappear and invalid + memory data could be accessed. But if we take the two locks in different + order, we may end up in a deadlock. + + The waiter needs to lock the waitee to delete itself from the list in + unregister_wait_for_prior_commit(). Thus wakeup_subsequent_commits() can not + hold its own lock while locking waiters, lest we deadlock. lest ? + So we need to prevent unregister_wait_for_prior_commit() running while wakeup + is in progress - otherwise the unregister could complete before the wakeup, + leading to incorrect spurious wakeup or accessing invalid memory. + <cut> + Then also register_wait_for_prior_commit() needs to check if + wakeup_subsequent_commits() is running, and skip the registration if + so. This is needed in case a new waiter manages to register itself and + immediately try to unregister while wakeup_subsequent_commits() is + running. Else the new waiter would also wait rather than unregister, but it + would not be woken up until next wakeup, which could be potentially much + later than necessary. +*/ Add new line here +void +wait_for_commit::wakeup_subsequent_commits2() +{ <cut> === modified file 'sql/sql_class.h' --- sql/sql_class.h 2013-06-06 15:51:28 +0000 +++ sql/sql_class.h 2013-08-29 08:18:22 +0000 @@ -1553,6 +1553,116 @@ class Global_read_lock }; +/* + Class to facilitate the commit of one transactions waiting for the commit of + another transaction to complete first. + + This is used during (parallel) replication, to allow different transactions + to be applied in parallel, but still commit in order. + + The transaction that wants to wait for a prior commit must first register + to wait with register_wait_for_prior_commit(waitee). Such registration + must be done holding the waitee->LOCK_wait_commit, to prevent the other + THD from disappearing during the registration. + + Then during commit, if a THD is registered to wait, it will call + wait_for_prior_commit() as part of ha_commit_trans(). If no wait is + registered, or if the waitee for has already completed commit, then + wait_for_prior_commit() returns immediately. + + And when a THD that may be waited for has completed commit (more precisely + commit_ordered()), then it must call wakeup_subsequent_commits() to wake + up any waiters. Note that this must be done at a point that is guaranteed + to be later than any waiters registering themselves. It is safe to call + wakeup_subsequent_commits() multiple times, as waiters are removed from + registration as part of the wakeup. + + The reason for separate register and wait calls is that this allows to + register the wait early, at a point where the waited-for THD is known to + exist. And then the actual wait can be done much later, where the + waited-for THD may have been long gone. By registering early, the waitee + can signal before disappearing. +*/ Instead of taking locks to ensure that THD will disapper, why not keep all THD objects around until all transactions in a group has been executed? +struct wait_for_commit +{ <cut> + bool waiting_for_commit; + /* + Flag set when wakeup_subsequent_commits_running() is active, see commonts commonts -> comments === modified file 'sql/sys_vars.cc' --- sql/sys_vars.cc 2013-06-07 07:31:11 +0000 +++ sql/sys_vars.cc 2013-08-29 08:18:22 +0000 <cut> +static bool +check_slave_parallel_threads(sys_var *self, THD *thd, set_var *var) +{ + bool running; + + mysql_mutex_lock(&LOCK_active_mi); + running= master_info_index->give_error_if_slave_running(); + mysql_mutex_unlock(&LOCK_active_mi); + if (running) + return true; + + return false; +} + +static bool +fix_slave_parallel_threads(sys_var *self, THD *thd, enum_var_type type) +{ + bool running; + bool err= false; + + mysql_mutex_unlock(&LOCK_global_system_variables); + mysql_mutex_lock(&LOCK_active_mi); + running= master_info_index->give_error_if_slave_running(); + mysql_mutex_unlock(&LOCK_active_mi); You can call check_slave_parallel_threads() above to reuse code. + if (running || rpl_parallel_change_thread_count(&global_rpl_thread_pool, + opt_slave_parallel_threads)) + err= true; + mysql_mutex_lock(&LOCK_global_system_variables); + + return err; +} + + +static Sys_var_ulong Sys_slave_parallel_threads( + "slave_parallel_threads", + "If non-zero, number of threads to spawn to apply in parallel events " + "on the slave that were group-committed on the master or were logged " + "with GTID in different replication domains.", + GLOBAL_VAR(opt_slave_parallel_threads), CMD_LINE(REQUIRED_ARG), + VALID_RANGE(0,16383), DEFAULT(0), BLOCK_SIZE(1), NO_MUTEX_GUARD, + NOT_IN_BINLOG, ON_CHECK(check_slave_parallel_threads), + ON_UPDATE(fix_slave_parallel_threads)); #endif We have talked about having 1 to be the minimum, which would give the old behavior. <cut> Overal things looks very good and it should not be too much work to add the things you have written on the todo part.a Regards Monty

2 10

[Maria-developers] [Spatial] On current implementation approach
by Mateusz Loskot 25 Sep '13

25 Sep '13

Hi, I'm going to ask question about how the current Spatial Extensions are implemented. I have spent some time reading the source code in the current trunk (spatial.h|cc, gcal*.h|cc, related Field and Item definitions, etc.), so I have a rough understanding of the overall structure, how the geometry data types are implemented and exposed to SQL, how the spatial functions are defined and registered. I did not looked into details of implementation of geospatial algorithms, but that's too low level for the question I'm going to ask. Initially, I was going to ask very detailed question, listing all the relevant code definitions and asking separately about each of them, but that's not necessary at this stage, I think. Instead, I'm going to simplify and ask about the bigger picture, more about MariaDB extensions API: 1. Is it possible to implement MariaDB extensions like Spatial (custom type + set of functions) without such a tight coupling with the internal implementation of the type system (without messing Field class with geometry types directly, etc.)? 2. Is it possible to implement Spatial using User-Defined Functions (UDF) defined in shared binary? 3. What is the reason behind using Well-Known-Binary (WKB) stream of bytes to transport geometry values into/from functions? Is it due to limitations of MariaDB type system where String is the only universal carrier for complex data? This concern is related to necessity of encoding/decoding WKB when chaining spatial function calls, and possibilities to avoid it. Best regards, -- Mateusz Loskot, http://mateusz.loskot.net "Participation in this whole process is a form of torture" ~~ Szalony

3 10

[Maria-developers] close mdev-4387
by Roberto Spadim 23 Sep '13

23 Sep '13

hi guys, please close https://mariadb.atlassian.net/browse/MDEV-4387 it's duplicated by alread commited https://mariadb.atlassian.net/browse/MDEV-5025 thanks =) -- Roberto Spadim

1 0

Re: [Maria-developers] [Roles] Final Status
by Vicentiu Ciorbaru 23 Sep '13

23 Sep '13

Hi Sergei! >> Even though I didn't manage to implement everything yet, I want to >> finish this project and get it ready for an upstream push, outside of >> GSoC. > 1. I'd need either from you in email (but then cc: it to > maria-developers) or in your blog (or both) a statement about the > license. A simple one would be to say that this your code is available > to us under BSD license. Another option is our MariaDB Contributor > Agreement: https://mariadb.com/kb/en/mca/ I hereby declare that all my work done so far (and future) by me on the Roles project, is released under the BSD licence. I will also make a statement on my blog in my wrap up post a bit later. If there are any more clarifications needed, let me know! Regards, Vicențiu

2 2

[Maria-developers] Review of MDEV-4506, parallel replication, part 3
by Michael Widenius 22 Sep '13

22 Sep '13

Hi! This is a review of the latest code pushed to your tree: bzr diff -r 3662.. Note that I will apply some of the trivial changes myself to the tree to speed up things (this is mostly about comments). === added file 'mysql-test/suite/rpl/r/rpl_parallel.result' --- mysql-test/suite/rpl/r/rpl_parallel.result 1970-01-01 00:00:00 +0000 +++ mysql-test/suite/rpl/r/rpl_parallel.result 2013-09-22 20:25:20 +0000 Great that you got these done! Will make my life much easier when I work the parallel replication code! <cut> === modified file 'sql/log.cc' --- sql/log.cc 2013-07-04 22:26:15 +0000 +++ sql/log.cc 2013-09-22 20:25:20 +0000 @@ -6542,27 +6542,93 @@ MYSQL_BIN_LOG::write_transaction_to_binl } } + +/* + Put a transaction that is ready to commit in the group commit queue. + The transaction is identified by the ENTRY object passed into this function. + + To facilitate group commit for the binlog, we first queue up ourselves in + this function. Then later the first thread to enter the queue waits for + the LOCK_log mutex, and commits for everyone in the queue once it gets the + lock. Any other threads in the queue just wait for the first one to finish + the commit and wake them up. This way, all transactions in the queue get + committed in a single disk operation. + + The return value of this function is TRUE if queued as the first entry in + the queue (meaning this is the leader), FALSE otherwise. I moved this last in the comment as: @retval TRUE If queued as the first entry in the queue (meaning this is the leader) @retval FALSE Otherwise + + The main work in this function is when the commit in one transaction has + been marked to wait for the commit of another transaction to happen + first. This is used to support in-order parallel replication, where + transactions can execute out-of-order but need to be committed in-order with + how they happened on the master. The waiting of one commit on another needs + to be integrated with the group commit queue, to ensure that the waiting + transaction can participate in the same group commit as the waited-for + transaction. + + So when we put a transaction in the queue, we check if there were other + transactions already prepared to commit but just waiting for the first one + to commit. If so, we add those to the queue as well, transitively for all + waiters. +*/ + <cut> @@ -6574,10 +6640,28 @@ MYSQL_BIN_LOG::queue_for_group_commit(gr This would be natural to do with recursion, but we want to avoid potentially unbounded recursion blowing the C stack, so we use the list approach instead. + + We keep a list of all the waiters that need to be processed in `list', + linked through the next_subsequent_commit pointer. Initially this list + contains only the entry passed into this function. + + We process entries in the list one by one. The element currently being + processed is pointed to by `cur`, and the element at the end of the list + is pointed to by `last` (we do not use NULL to terminate the list). + + As we process an element, it is first added to the group_commit_queue. + Then any waiters for that element are added at the end of the list, to + be processed in subsequent iterations. This continues until the list + is exhausted, with all elements ever added eventually processed. + + The end result is a breath-first traversal of the tree of waiters, + re-using the next_subsequent_commit pointers in place of extra stack + space in a recursive traversal. I added: The temporary list created in next_subsequent_commit is not used by the caller or any other function. @@ -6670,6 +6763,18 @@ MYSQL_BIN_LOG::queue_for_group_commit(gr { if (list->wakeup_subsequent_commits_running) { + /* + ToDo: We should not need a full lock/unlock of LOCK_wait_commit + here. All we need is a single (full) memory barrier, to ensure that + the reads of the list above are not reordered with the write of + wakeup_subsequent_commits_running, or with the writes to the list + from other threads that is allowed to happen after + wakeup_subsequent_commits_running has been set to false. + + We do not currently have explicit memory barrier primitives in the + source tree, but if we get them the below mysql_mutex_lock() could + be replaced with a full memory barrier just before the loop. + */ ok (for now). Another way would of course to only do a mutex lock for the first entry we find... No more comments. Everything looked fine! I will start coordinate with you tomorrow about things to do. Regards, Monty

1 0

[Maria-developers] Status and future of spatial extensions
by Mateusz Loskot 22 Sep '13

22 Sep '13

Hi, I haven't monitored MariaDB development closely, neither I tracked what's happening in the MySQL sandbox, so I'm hoping to catch up by asking questions here. I'm interested in the Spatial Extensions. So, I looked around the KB and the documentation and I found a few pages: general overview [1], 5.3 features [2] and page about plans [3]. I've looked into JIRA issues and recent commits too. Still, I don't see a clear big picture about the current status, roadmap and developers of the Spatial Extensions. Could anyone shed more light on the Spatial part of the MariaDB project, where it is now, where it is going, who is behind the wheel, etc.? What is (if any) the relation with geo-spatial development in MySQL? Is that correct understanding that MySQL and MariaDB follow separate paths, thus the GIS implementations go separately and in future they may seriously diverge and become no longer equivalent drop-in replacements? [1] https://mariadb.com/kb/en/gis-functionality/ [2] https://mariadb.com/kb/en/the-maria53-gis-tree-on-launchpad/ [3] https://mariadb.com/kb/en/mariadb-plans-gis/ Best regards, -- Mateusz Loskot, http://mateusz.loskot.net "Participation in this whole process is a form of torture" ~~ Szalony

1 0

Re: [Maria-developers] Rev 3818: Check for duplicate index (port from mysql)
by Sergei Golubchik 22 Sep '13

22 Sep '13

Hi, Sanja! On Sep 20, sanja(a)askmonty.org wrote: > ------------------------------------------------------------ > revno: 3818 > revision-id: sanja(a)askmonty.org-20130920193019-vr2q24c8g255aofu > parent: sanja(a)askmonty.org-20130920141533-nwk5jt07ojwmmctr > committer: sanja(a)askmonty.org > branch nick: work-maria-10.0-fts2 > timestamp: Fri 2013-09-20 22:30:19 +0300 > message: > Check for duplicate index (port from mysql) > (pre fts) ok to push Regards, Sergei

1 0

[Maria-developers] Review of MDEV-4506, parallel replication, part 2
by Michael Widenius 22 Sep '13

22 Sep '13

Hi! Part 2 of review of parallel replication === added file 'sql/rpl_parallel.cc' <cut> +pthread_handler_t +handle_rpl_parallel_thread(void *arg) +{ <cut> + if (end_of_group) + { + in_event_group= false; + Add comment: /* All events for this group has now been executed and logged (but not necessaerly synced). Inform the other event groups that are waiting for this thread that we are done. */ + rgi->commit_orderer.unregister_wait_for_prior_commit(); + thd->wait_for_commit_ptr= NULL; + + /* + Record that we have finished, so other event groups will no + longer attempt to wait for us to commit. + + We can race here with the next transactions, but that is fine, as + long as we check that we do not decrease last_committed_sub_id. If + this commit is done, then any prior commits will also have been + done and also no longer need waiting for. + */ + mysql_mutex_lock(&entry->LOCK_parallel_entry); + if (entry->last_committed_sub_id < event_gtid_sub_id) + { + entry->last_committed_sub_id= event_gtid_sub_id; + mysql_cond_broadcast(&entry->COND_parallel_entry); + } + mysql_mutex_unlock(&entry->LOCK_parallel_entry); + Add: /* Tell storage engines that they can stop waiting. See comments in sql_class.cc::wakeup_subsequent_commits2() for why we can't do this part of the wakeup in unregister_wait_for_prior_commit(). */ + rgi->commit_orderer.wakeup_subsequent_commits(); + delete rgi; + } + + events= next; + } + + mysql_mutex_lock(&rpt->LOCK_rpl_thread); + if ((events= rpt->event_queue) != NULL) + { + rpt->event_queue= rpt->last_in_queue= NULL; + mysql_mutex_unlock(&rpt->LOCK_rpl_thread); + goto more_events; + } + + if (!in_event_group) + { + rpt->current_entry= NULL; As far as I can see, rpt->free is always 0 here. - It's set to 0 at the top of this loop and only set to 1 here. Can we remove the rpt->free variable? + if (!rpt->stop && !rpt->free) + { Add comment. /* Take next group of events from the replication pool. This is faster than having to wakeup the pool manager thread to give us a new event. */ + mysql_mutex_lock(&rpt->pool->LOCK_rpl_thread_pool); + list= rpt->pool->free_list; + rpt->next= list; + rpt->pool->free_list= rpt; + if (!list) + mysql_cond_broadcast(&rpt->pool->COND_rpl_thread_pool); + mysql_mutex_unlock(&rpt->pool->LOCK_rpl_thread_pool); + rpt->free= true; + } + } + } <cut> +int +rpl_parallel_change_thread_count(rpl_parallel_thread_pool *pool, + uint32 new_count, bool skip_check) +{ + uint32 i; + rpl_parallel_thread **new_list= NULL; + rpl_parallel_thread *new_free_list= NULL; + + /* + Allocate the new list of threads up-front. + That way, if we fail half-way, we only need to free whatever we managed + to allocate, and will not be left with a half-functional thread pool. + */ + if (new_count && + !(new_list= (rpl_parallel_thread **)my_malloc(new_count*sizeof(*new_list), + MYF(MY_WME)))) + { + my_error(ER_OUTOFMEMORY, MYF(0), (int(new_count*sizeof(*new_list)))); + goto err;; + } Why not allocate new_list[] elements at the same time? Faster, less memory not allocation to check and freeing is also esier. if (!my_multi_malloc(MYF(MY_WME), &new_list, (uint) new_count*sizeof(*new_list), &list_part, new_count*sizeof(**new_list), NULL)) ... + + for (i= 0; i < new_count; ++i) + { + pthread_t th; + + if (!(new_list[i]= (rpl_parallel_thread *)my_malloc(sizeof(*(new_list[i])), + MYF(MY_WME)))) + { + my_error(ER_OUTOFMEMORY, MYF(0), sizeof(*(new_list[i]))); + goto err; + } The above would then simple be: new_list[i]= list_part++; Instead of setting everything to false and NULL, why not simple bzero() the structure? That way we can be more sure that we don't forget to initialize any new element in the future. + new_list[i]->delay_start= true; + new_list[i]->running= false; + new_list[i]->stop= false; + new_list[i]->free= false; + mysql_mutex_init(key_LOCK_rpl_thread, &new_list[i]->LOCK_rpl_thread, + MY_MUTEX_INIT_SLOW); + mysql_cond_init(key_COND_rpl_thread, &new_list[i]->COND_rpl_thread, NULL); + new_list[i]->pool= pool; + new_list[i]->current_entry= NULL; + new_list[i]->event_queue= NULL; + new_list[i]->last_in_queue= NULL; <cut> Add: /* The following loops wait until each thread is done it's work for each event group and then stops it when the thread has nothing to do. Note that this may take some time as rpl_paralell:do_event() may request threads at the same time. */ To make this faster and ensuring we don't start new request, wouldn't it be better to go through pool->threads and signal each of them? As long as rpt->stop is defined as 'stop when you are done processing one group' then this is fine (which is how it looks it's defined now). + for (i= 0; i < pool->count; ++i) + { + rpl_parallel_thread *rpt= pool->get_thread(NULL); + rpt->stop= true; + mysql_cond_signal(&rpt->COND_rpl_thread); + mysql_mutex_unlock(&rpt->LOCK_rpl_thread); + } + + for (i= 0; i < pool->count; ++i) + { + rpl_parallel_thread *rpt= pool->threads[i]; + mysql_mutex_lock(&rpt->LOCK_rpl_thread); + while (rpt->running) + mysql_cond_wait(&rpt->COND_rpl_thread, &rpt->LOCK_rpl_thread); + mysql_mutex_unlock(&rpt->LOCK_rpl_thread); + mysql_mutex_destroy(&rpt->LOCK_rpl_thread); + mysql_cond_destroy(&rpt->COND_rpl_thread); + my_free(rpt); By using my_multi_malloc() you can remove the above free + } + + my_free(pool->threads); + pool->threads= new_list; + pool->free_list= new_free_list; + pool->count= new_count; + for (i= 0; i < pool->count; ++i) + { + mysql_mutex_lock(&pool->threads[i]->LOCK_rpl_thread); + pool->threads[i]->delay_start= false; + mysql_cond_signal(&pool->threads[i]->COND_rpl_thread); + mysql_mutex_unlock(&pool->threads[i]->LOCK_rpl_thread); + } + + if (!skip_check) + { + mysql_mutex_lock(&LOCK_active_mi); + pool->changing= false; + mysql_mutex_unlock(&LOCK_active_mi); + } + return 0; + +err: + if (new_list) + { + while (new_free_list) + { + rpl_parallel_thread *next= new_free_list->next; + mysql_mutex_lock(&new_free_list->LOCK_rpl_thread); + new_free_list->delay_start= false; + new_free_list->stop= true; + mysql_cond_signal(&new_free_list->COND_rpl_thread); + while (!new_free_list->running) + mysql_cond_wait(&new_free_list->COND_rpl_thread, + &new_free_list->LOCK_rpl_thread); + while (new_free_list->running) + mysql_cond_wait(&new_free_list->COND_rpl_thread, + &new_free_list->LOCK_rpl_thread); + mysql_mutex_unlock(&new_free_list->LOCK_rpl_thread); + my_free(new_free_list); By using my_multi_malloc() you can remove the above free and even the 'next' variable + new_free_list= next; + } + my_free(new_list); + } <cut> <cut> +void +rpl_parallel::wait_for_done() +{ + struct rpl_parallel_entry *e; + uint32 i; + + for (i= 0; i < domain_hash.records; ++i) + { + e= (struct rpl_parallel_entry *)my_hash_element(&domain_hash, i); + mysql_mutex_lock(&e->LOCK_parallel_entry); + while (e->current_sub_id > e->last_committed_sub_id) + mysql_cond_wait(&e->COND_parallel_entry, &e->LOCK_parallel_entry); + mysql_mutex_unlock(&e->LOCK_parallel_entry); + } +} How to handle errors in the above case? For example when we get an error and we had to abort things? I assume we should also abort for: e->rpl_thread->thd->killed() but this is hard as thd and even rpl_thread may be deleted. One way to fix this would be to add an entry 'killed' to rpl_parell_entry that we can set. However it will be hard to keep this up to date. The other way would be to, on error, set last_committed_sub_id back to it's last value. We could also have some kind of shutdown flag that we check to avoid hang on shutdown. What do you suggest? Assuming setting the value back would be the easiest solution.. + for (i= 0; i < domain_hash.records; ++i) + { + e= (struct rpl_parallel_entry *)my_hash_element(&domain_hash, i); + mysql_mutex_lock(&e->LOCK_parallel_entry); + while (e->current_sub_id > e->last_committed_sub_id) + mysql_cond_wait(&e->COND_parallel_entry, &e->LOCK_parallel_entry); + mysql_mutex_unlock(&e->LOCK_parallel_entry); + } +bool +rpl_parallel::do_event(struct rpl_group_info *serial_rgi, Log_event *ev) +{ + rpl_parallel_entry *e; + rpl_parallel_thread *cur_thread; + rpl_parallel_thread::queued_event *qev; + struct rpl_group_info *rgi; + Relay_log_info *rli= serial_rgi->rli; + enum Log_event_type typ; + + /* ToDo: what to do with this lock?!? */ + mysql_mutex_unlock(&rli->data_lock); + + if (!(qev= (rpl_parallel_thread::queued_event *)my_malloc(sizeof(*qev), + MYF(0)))) + { + my_error(ER_OUT_OF_RESOURCES, MYF(0)); + return true; + } + qev->ev= ev; + qev->next= NULL; + + if ((typ= ev->get_type_code()) == GTID_EVENT) + { + Gtid_log_event *gtid_ev= static_cast<Gtid_log_event *>(ev); + + if (!(e= find(gtid_ev->domain_id)) || + !(rgi= new rpl_group_info(rli)) || + event_group_new_gtid(rgi, gtid_ev)) + { Here add delete of rgi. Also set rgi to 0 before. + my_error(ER_OUT_OF_RESOURCES, MYF(MY_WME)); + return true; + } + + if ((gtid_ev->flags2 & Gtid_log_event::FL_GROUP_COMMIT_ID) && + e->last_commit_id == gtid_ev->commit_id) + { + /* + We are already executing something else in this domain. But the two + event groups were committed together in the same group commit on the + master, so we can still do them in parallel here on the slave. + + However, the commit of this event must wait for the commit of the prior + event, to preserve binlog commit order and visibility across all + servers in the replication hierarchy. + */ + rpl_parallel_thread *rpt= global_rpl_thread_pool.get_thread(e); Here we could add a test if get_thread() is 0, which could happen on shutdown or when killing a replication thread. I think it's relatively safe to use get_thread() as a end-synchronize point as we should never have anything half-done when we call get_thread()). As get_thread() can take a very long time and is not called in many places, it's also sounds like a resonable thing to do. + rgi->wait_commit_sub_id= e->current_sub_id; + rgi->wait_commit_group_info= e->current_group_info; + rgi->wait_start_sub_id= e->prev_groupcommit_sub_id; + e->rpl_thread= cur_thread= rpt; + /* get_thread() returns with the LOCK_rpl_thread locked. */ Add the abvove comments also where get_thread() is defined. + } + else + { + /* Check if we already have a worker thread for this entry. */ I had a hard time to understand what the above meant. Is this new suggested comments correct? /* Get the last or current thread for the domain */ + cur_thread= e->rpl_thread; + if (cur_thread) + { /* Active thread exists. The thread may still be executing queries for this domain or it may have been rescheduled to work for another domain. */ + mysql_mutex_lock(&cur_thread->LOCK_rpl_thread); + if (cur_thread->current_entry != e) + { + /* Not ours anymore, we need to grab a new one. */ Suggested new comment: /* The thread has been reused for another domain. (e->rpl_thread was not reset or was changed during the mutex_lock) As there is no threads working on this domain, it's safe to assign a new thread to work on it */ + mysql_mutex_unlock(&cur_thread->LOCK_rpl_thread); + e->rpl_thread= cur_thread= NULL; + } + } <cut> Add here: /* Current is not set for the case where the master did not have GTID, like a MariaDB 5.5 or MySQL master. */ + else if (!Log_event::is_group_event(typ) || !current) + { + /* + Events like ROTATE and FORMAT_DESCRIPTION. Do not run in worker thread. + Same for events not preceeded by GTID (we should not see those normally, + but they might be from an old master). + */ + qev->rgi= serial_rgi; + rpt_handle_event(qev, NULL); + delete_or_keep_event_post_apply(rli, typ, qev->ev); Why is delete_or_keep_event_post_apply() not part of rpt_handle_event()? You always call these two together... + + return false; + } + else + { The following code is identical (except one line) to the code earlier in this function. Better to create a function for this to reuse code and to clarify the differences. + cur_thread= current->rpl_thread; + if (cur_thread) + { + mysql_mutex_lock(&cur_thread->LOCK_rpl_thread); + if (cur_thread->current_entry != current) + { + /* Not ours anymore, we need to grab a new one. */ + mysql_mutex_unlock(&cur_thread->LOCK_rpl_thread); + cur_thread= NULL; + } + } + if (!cur_thread) + { + cur_thread= current->rpl_thread= + global_rpl_thread_pool.get_thread(current); + } + } + qev->rgi= current->current_group_info; + + /* + Queue the event for processing. + */ + if (cur_thread->last_in_queue) + cur_thread->last_in_queue->next= qev; + else + cur_thread->event_queue= qev; + cur_thread->last_in_queue= qev; + mysql_cond_signal(&cur_thread->COND_rpl_thread); + mysql_mutex_unlock(&cur_thread->LOCK_rpl_thread); In this case it's safe to move the cond_signal outside of the mutex_unlock. (Should make things notable faster as we win a sleep-wakup in other thread). <cut> === added file 'sql/rpl_parallel.h' --- sql/rpl_parallel.h 1970-01-01 00:00:00 +0000 +++ sql/rpl_parallel.h 2013-08-29 08:18:22 +0000 @@ -0,0 +1,92 @@ +#ifndef RPL_PARALLEL_H +#define RPL_PARALLEL_H + +#include "log_event.h" + + +struct rpl_parallel; +struct rpl_parallel_entry; +struct rpl_parallel_thread_pool; + +class Relay_log_info; +struct rpl_parallel_thread { + bool delay_start; I think we can remove deley_start. Se comments from previous review. + bool running; + bool stop; We need to define 'stop' properly. Like 'stop after we have executed the current event group' (So that we don't have to do rollback). + bool free; I think we can remove free. Se comments above. Otherwise it would be good to have a good descripition for each struct in this file. Will now start reviewing your new commits to the parallel replication tree. Regards, Monty

1 0

[Maria-developers] MDEV-5019 - THREADPOOL - Create Information Schema Table for Threadpool
by Roberto Spadim 22 Sep '13

22 Sep '13

Hi guys! I openned a new MDEV https://mariadb.atlassian.net/browse/MDEV-5019 the idea is expose the threadpool group information like oracle thread pool (more information about oracle thread pool here: http://mikaelronstrom.blogspot.com.br/2011_10_01_archive.html) I can help, but i don't know how access the threadpool at scheduler.h/scheduler.c from a information schema plugin 1) lock important mutex and others mdl 2) acess variables 3) write information schema based on variables 1 and 2 i don't know how to do :( can anyone help ? :) thanks guys :) -- Roberto Spadim

2 9

[Maria-developers] Perf tuning for hot key updating with high concurrency
by Hui Liu 22 Sep '13

22 Sep '13

It's an old issue which discussed in Percona-google-group: https://groups.google.com/forum/?fromgroups#!topic/percona-discussion/z3r4-… The patch provided in that discuss group is a rough idea, based on Percona-Server, and later I almost re-wrote the patch for real-case testing. I think it's more simple to implement in the InnoDB layer, compared to the the idea post by Monty, that introduce 'Incremental lock' or 'Relative update lock' to solve this problem. I backported them to Mariadb-10.0.4, and use simple simulate test with oltp_cc.lua, which is provided as an attachment. These notes migth be helpful to understand of this patch. 1) Why InnoDB's concurrency control is BAD for high concurrency hot key updating case? It's described in the percona-disscuss-group, please refer that. 2) What's the basic logic for high concurrency hot key updating? The idea is still the same as before, but the implement is different now. With a simple case that 1000 threads updating the same record id=1 and threshold=2 to describe the basic idea for this patch. conn (1000) ... (6) -> (5) -> (4) (1) (running) (2) (suspended) (3) (suspended) That is, thread 1 is running, and thread 2 and 3 are suspended, waiting for thread 1 relase the record lock for id=1. For thread 4, it detected they are already two threads suspended, and it would be rescheduled in hash queue. conn (1000) .. (6) -> (5) (1) (running) (2) (suspended) (3) (suspended) bucket1: (4) bucket2: And for 5 and 6 ... 1000, they are also rescheduled in that queue (1) (running) (2) (suspended) (3) (suspended) bucket1: (1000)...(6) -> (5) -> (4) bucket2: After 1 is committed, 2 is waked up and running, then 4 detects it would entry now, so it enters. (1) (commit/exit) (2) (running) (3) (suspended) (4) (suspended) bucket1: (1000) ... (6) -> (5) bucket2: The same logic for 5 and 6...1000. While for the old logic, a snapshot might be: (1) (commit/exit) (2) (running) (3) (suspended) (4) (suspended) (5) (suspended) .... (1000) (suspended) Hope the demo is not too naive. You can see, in such case, there are always threshold threads running inside innodb, while not all are suspended inside. 3) Why not use the new concurrency control with AOTMIC add/sub for active conc? The result is bad for new concurrency control in 5.6/10.0, that no FIFO any more. So I changed to default (5.1/5.5) control strategy again. Note the changes in CMakeLists.txt that make -DUSE_DEFAULT_CONC=ON as default. 4) How about the concurrency counter for rescheduled thread in bucket queue? If thread number waiting for the record lock is over the configured threshold, it would be rescheduled into queue in one hash bucket, hashed by (space,space_no, heap_no) a) for the first one in the queue, that is the queue header, it would timed-wait retrying whether it would enter into innodb layer, by checking the n_ref_count for that record stored by HASH. b) for the non-head threads, they would be FAKE released the concurrent slot, that means threads in FIFO would be waked up, enter/exit innodb with faked inside and tickets. Until they are force to exit innodb, the active slot number is equal to real case without rescheduling. 5) What about the basic test result? $ ./sysbench/sysbench --test=sysbench/tests/db/oltp_cc.lua --num-threads=1000 --mysql-socket=/u01/md1004/run/mysql.sock --mysql-db=bmw --oltp-tables-count=4 --mysql-user=root --oltp-table-size=4 --max-requests=0 --max-time=120000 --oltp-read-only=off run 1000conn, oltp-cc.lua, 4x4(16 hotkey): patched (innodb_lock_wait_threshold=12 as default) : 3.1K QPS, 9.1K TPS unpatched (innodb_lock_wait_threshold=0 as disabled) : 1.2K QPS, 3.5K TPS $ ./sysbench/sysbench --test=sysbench/tests/db/oltp_cc.lua --num-threads=1000 --mysql-socket=/u01/md1004/run/mysql.sock --mysql-db=bmw --oltp-tables-count=1 --mysql-user=root --oltp-table-size=4 --max-requests=0 --max-time=120000 --oltp-read-only=off run 1000conn, oltp-cc.lua, 1x4(4 hotkey): patched (innodb_lock_wait_threshold=12 as default) : 1.3K QPS, 3.5K TPS unpatched (innodb_lock_wait_threshold=0 as disabled) : 200 QPS, 720 TPS 6) Any rest issues? The rescheduling of the waiting thread breaks the normal mechanism: each lock request must be registered in lock-table, then deadlock could be found if exists. As I am trying to reduce n in lock relaetd O(n) and O(n^2) operations, for the deadlock case, it does not work very well. For example, the update sequance: t1 updates 1, and t2 updates 1, then t3/t4/t5/t6 update 1, and then t1 updates 2, after that t2 updates 1: t1 1 2 t2 2 1 t3 1 t4 1 t5 1 t6 1 As we can see, in the new schedule model, t3/t4/t5 are suspended, and might be rescheduled in queue. Then t1 is suspended as it updates 2 which record lock is hold by t2. t2 is rescheduled as it detected too many lock waiting for rec 1. The snapshot might be: (t1[2]) (suspended) (t3) (suspended) (t4) (suspended) bucket1: (t2[1]) -> (t6) -> (t5) bucket2: The deadlock can be break until t2 enters with 1. Then for t5/t6, they would wait until timeout, and then force enter into innodb side, suspended as it should be. It's same for t2. Until t2 enters, the deadlock is found. So, the cost for deadlock detecting might be much large than it should be, depends on the timeout and retry counts. Hope Jan or Monty would have a look for this patch, or other ideas for solve this problem, if you are going.

1 0

[Maria-developers] Prep for MariaDB 5.5.33a
by Daniel Bartholomew 20 Sep '13

20 Sep '13

I've begun the prep for the special 5.5.33a release. Draft changelog and release notes are here: https://mariadb.com/kb/en/mariadb-5533a-changelog/ https://mariadb.com/kb/en/mariadb-5533a-release-notes/ As always, fixes, additions, and such are welcome. I'm currently just waiting for buildbot to finish building and then I'll package up the release so that it can get mirrored over the next few hours and we can release tomorrow after some final testing of the finished repos. Assuming things go well, my plan is to release around noon US Eastern time. Thanks. -- Daniel Bartholomew, Sr Tech Writer / Sys Admin MariaDB

2 1

Re: [Maria-developers] [Commits] Rev 3816: MDEV-4883 - Not all host_cache tests have been merged in lp:maria/10.0
by Roberto Spadim 20 Sep '13

20 Sep '13

Hi guys, what's the "MySQL variables auto-tuning code" means? i didn't saw that in mysql source and at mysql docs, any idea what this means? 2013/9/18 Sergey Vojtovich <svoj(a)mariadb.org> > At lp:maria/10.0 > > ------------------------------------------------------------ > revno: 3816 > revision-id: svoj(a)mariadb.org-20130918071716-yu2a5489uldzfuy9 > parent: sanja(a)askmonty.org-20130917190515-g3xncb1slqwy4cix > committer: Sergey Vojtovich <svoj(a)mariadb.org> > branch nick: 10.0-mdev4883 > timestamp: Wed 2013-09-18 11:17:16 +0400 > message: > MDEV-4883 - Not all host_cache tests have been merged > > Merged main.host_cache_size_functionality test. > > Test had to be adjusted because MariaDB doesn't have > MySQL variables auto-tuning code. > === added file 'mysql-test/r/host_cache_size_functionality.result' > --- a/mysql-test/r/host_cache_size_functionality.result 1970-01-01 > 00:00:00 +0000 > +++ b/mysql-test/r/host_cache_size_functionality.result 2013-09-18 > 07:17:16 +0000 > @@ -0,0 +1,78 @@ > +'#________________________VAR_06_Host_Cache_Size__________________#' > +echo '##' > +--echo '#---------------------WL6372_VAR_6_01----------------------#' > +#################################################################### > +# Checking default value # > +#################################################################### > +SELECT COUNT(@@GLOBAL.Host_Cache_Size) > +1 Expected > +set @Default_host_cache_size=128; > +select @@global.Host_Cache_Size=@Default_host_cache_size; > +@@global.Host_Cache_Size=@Default_host_cache_size > +1 > +1 Expected > +'#---------------------WL6372_VAR_6_02----------------------#' > +# Restart server with Host_Cache_Size 1 > +SELECT @@GLOBAL.Host_Cache_Size; > +@@GLOBAL.Host_Cache_Size > +1 > +1 Expected > +set @Default_host_cache_size=128; > +SET @@GLOBAL.Host_Cache_Size=DEFAULT; > +select @@global.Host_Cache_Size=@Default_host_cache_size; > +@@global.Host_Cache_Size=@Default_host_cache_size > +1 > +1 Expected > +'#---------------------WL6372_VAR_6_03----------------------#' > +SET @@local.Host_Cache_Size=1; > +ERROR HY000: Variable 'host_cache_size' is a GLOBAL variable and should > be set with SET GLOBAL > +Expected error 'Global variable' > +SET @@session.Host_Cache_Size=1; > +ERROR HY000: Variable 'host_cache_size' is a GLOBAL variable and should > be set with SET GLOBAL > +Expected error 'Global variable' > +SET @@GLOBAL.Host_Cache_Size=1; > +SET @@GLOBAL.Host_Cache_Size=DEFAULT; > +SELECT COUNT(@@GLOBAL.Host_Cache_Size); > +COUNT(@@GLOBAL.Host_Cache_Size) > +1 > +1 Expected > +select @@global.Host_Cache_Size=@Default_host_cache_size; > +@@global.Host_Cache_Size=@Default_host_cache_size > +1 > +1 Expected > +'#---------------------WL6372_VAR_6_04----------------------#' > +SELECT @@GLOBAL.Host_Cache_Size = VARIABLE_VALUE > +FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES > +WHERE VARIABLE_NAME='Host_Cache_Size'; > +@@GLOBAL.Host_Cache_Size = VARIABLE_VALUE > +1 > +1 Expected > +SELECT COUNT(@@GLOBAL.Host_Cache_Size); > +COUNT(@@GLOBAL.Host_Cache_Size) > +1 > +1 Expected > +SELECT COUNT(VARIABLE_VALUE) > +FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES > +WHERE VARIABLE_NAME='Host_Cache_Size'; > +COUNT(VARIABLE_VALUE) > +1 > +1 Expected > +'#---------------------WL6372_VAR_6_05----------------------#' > +SELECT @@Host_Cache_Size = @@GLOBAL.Host_Cache_Size; > +@@Host_Cache_Size = @@GLOBAL.Host_Cache_Size > +1 > +1 Expected > +SELECT COUNT(@@local.Host_Cache_Size); > +ERROR HY000: Variable 'host_cache_size' is a GLOBAL variable > +Expected error 'Variable is a GLOBAL variable' > +SELECT COUNT(@@SESSION.Host_Cache_Size); > +ERROR HY000: Variable 'host_cache_size' is a GLOBAL variable > +Expected error 'Variable is a GLOBAL variable' > +SELECT COUNT(@@GLOBAL.Host_Cache_Size); > +COUNT(@@GLOBAL.Host_Cache_Size) > +1 > +1 Expected > +SELECT Host_Cache_Size = @@SESSION.Host_Cache_Size; > +ERROR 42S22: Unknown column 'Host_Cache_Size' in 'field list' > +Expected error 'Unknown column Host_Cache_Size in field list' > +SET @@GLOBAL.Host_Cache_Size=DEFAULT; > > === added file 'mysql-test/t/host_cache_size_functionality.test' > --- a/mysql-test/t/host_cache_size_functionality.test 1970-01-01 > 00:00:00 +0000 > +++ b/mysql-test/t/host_cache_size_functionality.test 2013-09-18 > 07:17:16 +0000 > @@ -0,0 +1,185 @@ > > +############################################################################### > +# > # > +# Variable Name: Host_Cache_Size > # > +# Scope: Global > # > +# Access Type: Dynamic > # > +# Data Type: numeric > # > +# > # > +# > # > +# Creation Date: 2012-08-31 > # > +# Author : Tanjot Singh Uppal > # > +# > # > +# > # > +# Description:Test Cases of Dynamic System Variable Host_Cache_Size > # > +# that checks the behavior of this variable in the following > ways # > +# * Value Check > # > +# * Scope Check > # > +# * Functionality Check > # > +# * Accessability Check > # > +# > # > +# This test does not perform the crash recovery on this variable > # > +# For crash recovery test on default change please run the ibtest > # > > +############################################################################### > + > +-- source include/have_innodb.inc > +-- source include/not_embedded.inc > +-- source include/have_innodb_16k.inc > +--disable_warnings > +echo '#________________________VAR_06_Host_Cache_Size__________________#' > +echo '##' > +--echo '#---------------------WL6372_VAR_6_01----------------------#' > +#################################################################### > +# Checking default value # > +#################################################################### > +SELECT COUNT(@@GLOBAL.Host_Cache_Size); > +--echo 1 Expected > + > +#set @Default_host_cache_size=(select > if(if(@@global.max_connections<500,128+@@global.max_connections,128+@ > @global.max_connections+floor((@@global.max_connections-500)/20))>2000,2000,if(@@global.max_connections<500,128+@ > @global.max_connections,128+@ > @global.max_connections+floor((@@global.max_connections-500)/20)))); > +set @Default_host_cache_size=128; > + > +select @@global.Host_Cache_Size=@Default_host_cache_size; > +--echo 1 Expected > + > + > +--echo '#---------------------WL6372_VAR_6_02----------------------#' > > +################################################################################# > +# Checking the Default value post starting the server with other value > # > > +################################################################################# > +--echo # Restart server with Host_Cache_Size 1 > + > +let $restart_file= $MYSQLTEST_VARDIR/tmp/mysqld.1.expect; > +--exec echo "wait" > $restart_file > +--shutdown_server 10 > +--source include/wait_until_disconnected.inc > +-- exec echo "restart:--host_cache_size=1 " > > $MYSQLTEST_VARDIR/tmp/mysqld.1.expect > +-- enable_reconnect > +-- source include/wait_until_connected_again.inc > + > +--disable_warnings > + > +SELECT @@GLOBAL.Host_Cache_Size; > +--echo 1 Expected > + > +#set @Default_host_cache_size=(select > if(if(@@global.max_connections<500,128+@@global.max_connections,128+@ > @global.max_connections+floor((@@global.max_connections-500)/20))>2000,2000,if(@@global.max_connections<500,128+@ > @global.max_connections,128+@ > @global.max_connections+floor((@@global.max_connections-500)/20)))); > +set @Default_host_cache_size=128; > +SET @@GLOBAL.Host_Cache_Size=DEFAULT; > +select @@global.Host_Cache_Size=@Default_host_cache_size; > +--echo 1 Expected > + > + > +--echo '#---------------------WL6372_VAR_6_03----------------------#' > +#################################################################### > +# Checking Value can be set - Dynamic # > +#################################################################### > +--error ER_GLOBAL_VARIABLE > +SET @@local.Host_Cache_Size=1; > +--echo Expected error 'Global variable' > + > +--error ER_GLOBAL_VARIABLE > +SET @@session.Host_Cache_Size=1; > +--echo Expected error 'Global variable' > + > +SET @@GLOBAL.Host_Cache_Size=1; > +SET @@GLOBAL.Host_Cache_Size=DEFAULT; > + > +SELECT COUNT(@@GLOBAL.Host_Cache_Size); > +--echo 1 Expected > + > +select @@global.Host_Cache_Size=@Default_host_cache_size; > +--echo 1 Expected > + > +--echo '#---------------------WL6372_VAR_6_04----------------------#' > +################################################################# > +# Check if the value in GLOBAL Table matches value in variable # > +################################################################# > +SELECT @@GLOBAL.Host_Cache_Size = VARIABLE_VALUE > +FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES > +WHERE VARIABLE_NAME='Host_Cache_Size'; > +--echo 1 Expected > + > +SELECT COUNT(@@GLOBAL.Host_Cache_Size); > +--echo 1 Expected > + > +SELECT COUNT(VARIABLE_VALUE) > +FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES > +WHERE VARIABLE_NAME='Host_Cache_Size'; > +--echo 1 Expected > + > +--echo '#---------------------WL6372_VAR_6_05----------------------#' > > +################################################################################ > +# Checking Variable Scope > # > > +################################################################################ > +SELECT @@Host_Cache_Size = @@GLOBAL.Host_Cache_Size; > +--echo 1 Expected > + > +--Error ER_INCORRECT_GLOBAL_LOCAL_VAR > +SELECT COUNT(@@local.Host_Cache_Size); > +--echo Expected error 'Variable is a GLOBAL variable' > + > +--Error ER_INCORRECT_GLOBAL_LOCAL_VAR > +SELECT COUNT(@@SESSION.Host_Cache_Size); > +--echo Expected error 'Variable is a GLOBAL variable' > + > +SELECT COUNT(@@GLOBAL.Host_Cache_Size); > +--echo 1 Expected > + > +--Error ER_BAD_FIELD_ERROR > +SELECT Host_Cache_Size = @@SESSION.Host_Cache_Size; > +--echo Expected error 'Unknown column Host_Cache_Size in field list' > + > +#The below check has been commented out as the IP fetch is different in a > P2P connection than BroadBand connection > +#--echo '#---------------------WL6372_VAR_6_06----------------------#' > > +############################################################################### > +# Checking the Host cahce functionality > # > > +############################################################################### > + > +#SET @@GLOBAL.Host_Cache_Size=2; > +#--disable_warnings > + > +#--perl > +#my $ip=`ifconfig | egrep "inet addr|inet" | sed -e 's/^.*inet addr://' > -e 's/^.*inet//'| sed 's/ .*\$//'|egrep -i "broadcast|bcast"|head -1|awk > '{print $1}'`; > +#open (LOGFH, ">" . $ENV{'MYSQL_TMP_DIR'} . "/bind_ip"); > +#print LOGFH "let \$bind_ip = $ip;\n"; > +#close LOGFH; > +#EOF > + > +#--source $MYSQL_TMP_DIR/bind_ip > +#--remove_file $MYSQL_TMP_DIR/bind_ip > + > +#let $restart_file= $MYSQLTEST_VARDIR/tmp/mysqld.1.expect; > +#--exec echo "wait" > $restart_file > +#--shutdown_server 10 > +#--source include/wait_until_disconnected.inc > +#-- exec echo "restart:--bind-address=$bind_ip " > > $MYSQLTEST_VARDIR/tmp/mysqld.1.expect > +#-- enable_reconnect > +#-- source include/wait_until_connected_again.inc > + > +#connection default; > +#--disable_warnings > + > +#create user binduser; > +#grant all on *.* to binduser; > + > +#select count(IP) from performance_schema.host_cache; > +#--echo 0 Expected > + > +#connect (con1,$bind_ip,binduser,,); > +#select count(IP) from performance_schema.host_cache; > +#--echo 1 Expected > + > +#disconnect con1; > +#connection default; > + > +#--disable_warnings > + > +# The below check is hashed until the BUG14689561# is fixed > +# > +#flush hosts; > + > +#connect (con2,$bind_ip,binduser,,); > +#select count(IP) from performance_schema.host_cache; > +#--echo 1 Expected > + > +#disconnect con2; > + > +SET @@GLOBAL.Host_Cache_Size=DEFAULT; > > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits > -- Roberto Spadim SPAEmpresarial

2 1

Re: [Maria-developers] Rev 3814: Client attributes
by Sergei Golubchik 19 Sep '13

19 Sep '13

Hi, Sanja! On Sep 19, sanja(a)askmonty.org wrote: > === modified file 'sql/sql_acl.cc' > --- a/sql/sql_acl.cc 2013-07-21 14:39:19 +0000 > +++ b/sql/sql_acl.cc 2013-09-19 10:19:36 +0000 > @@ -4615,7 +4615,8 @@ bool check_grant(THD *thd, ulong want_ac > case ACL_INTERNAL_ACCESS_GRANTED: > /* > Currently, > - - the information_schema does not subclass ACL_internal_table_access, > + - the information_schema does not subclass ACL_internal_table_access > +, Eh? What was that? > there are no per table privilege checks for I_S, > - the performance schema does use per tables checks, but at most > returns 'CHECK_GRANT', and never 'ACCESS_GRANTED'. > @@ -8222,6 +8223,43 @@ static bool find_mpvio_user(MPVIO_EXT *m > mpvio->acl_user->plugin.str)); > DBUG_RETURN(0); > } > + > +static bool > +read_client_connect_attrs(char **ptr, size_t *max_bytes_available, > + const CHARSET_INFO *from_cs) > +{ > + size_t length, length_length; > + char *ptr_save; > + /* not enough bytes to hold the length */ > + if (*max_bytes_available < 1) > + return true; > + > + /* read the length */ > + ptr_save= *ptr; > + length= net_field_length_ll((uchar **) ptr); > + length_length= *ptr - ptr_save; > + if (*max_bytes_available < length_length) > + return true; This is incorrect, it might read past the end of the buffer. it's too late to check the length *after* you've read the data. Correct code might look like if (*max_bytes_available >= 9) { ptr_save= *ptr; length= net_field_length_ll((uchar **) ptr); length_length= *ptr - ptr_save; DBUG_ASSERT(length_length <= 9); *max_bytes_available-= length_length; } else { char buf[10], *len_ptr= buf; strncpy(buf, *ptr, 9) length= net_field_length_ll((uchar **) &len_ptr); length_length= len_ptr - buf; *ptr+= length_length; if (*max_bytes_available < length_length) return true; *max_bytes_available-= length_length; } > + > + *max_bytes_available-= length_length; > + > + /* length says there're more data than can fit into the packet */ > + if (length > *max_bytes_available) > + return true; > + > + /* impose an artificial length limit of 64k */ > + if (length > 65535) > + return true; > + > +#ifdef HAVE_PSI_THREAD_INTERFACE > + if (PSI_THREAD_CALL(set_thread_connect_attrs)(*ptr, length, from_cs) && > + current_thd->variables.log_warnings) > + sql_print_warning("Connection attributes of length %lu were truncated", > + (unsigned long) length); > +#endif > + return false; > +} > + > #endif > > /* the packet format is described in send_change_user_packet() */ > @@ -8349,6 +8387,13 @@ static bool parse_com_change_user_packet > } > } > > + size_t bytes_remaining_in_packet= end - ptr; 1. This is very wrong, did you ever test it? You need a pointer to *after* the plugin name. But 'ptr' is two bytes *before* the plugin. 2. remove bytes_remaining_in_packet, pass 'end' instead to read_client_connect_attrs(). > + > + if ((mpvio->thd->client_capabilities & CLIENT_CONNECT_ATTRS) && > + read_client_connect_attrs(&ptr, &bytes_remaining_in_packet, > + mpvio->thd->charset())) > + return packet_error; > + > DBUG_PRINT("info", ("client_plugin=%s, restart", client_plugin)); > /* > Remember the data part of the packet, to present it to plugin in > @@ -8487,7 +8532,21 @@ static ulong parse_client_handshake_pack > /* strlen() can't be easily deleted without changing protocol */ > db_len= db ? strlen(db) : 0; > > - char *client_plugin= passwd + passwd_len + (db ? db_len + 1 : 0); > + char *client_plugin= end= passwd + passwd_len + (db ? db_len + 1 : 0); > + > + if ((thd->client_capabilities & CLIENT_PLUGIN_AUTH) && > + (client_plugin < (char *)net->read_pos + pkt_len)) > + { > + end+= strlen(end) + 1; > + } > + > + size_t bytes_remaining_in_packet= (((char *)net->read_pos) + pkt_len) - end; This is very-very wrong. if there're no connection attributes, then your 'end' will be after the end of the packet, it'll be after net->read_pos + pkt_len. But then you subtract, get -1 and cast it to unsigned size_t ! Don't worry, when you remove bytes_remaining_in_packet as I wrote above, this bug will go away. > + > + if (db && > + (thd->client_capabilities & CLIENT_CONNECT_ATTRS) && > + read_client_connect_attrs(&end, &bytes_remaining_in_packet, > + mpvio->thd->charset())) > + return packet_error; Put it in the same place as in parse_com_change_user_packet, that is after reading the plugin name (and fix_plugin_ptr). And rename 'end' to 'connection_attributes' > > /* Since 4.1 all database names are stored in utf8 */ > if (db) > > === modified file 'sql/sys_vars.cc' > --- a/sql/sys_vars.cc 2013-08-14 08:48:50 +0000 > +++ b/sql/sys_vars.cc 2013-09-19 10:19:36 +0000 > @@ -71,6 +71,10 @@ > #define export /* not static */ > > #ifdef WITH_PERFSCHEMA_STORAGE_ENGINE > +#ifndef EMBEDDED_LIBRARY > +#define PFS_TRAILING_PROPERTIES \ > + NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(NULL), ON_UPDATE(NULL), \ > + NULL, sys_var::PARSE_EARLY Remove PFS_TRAILING_PROPERTIES define (not used in MariaDB) > > static Sys_var_mybool Sys_pfs_enabled( > "performance_schema", > @@ -331,8 +335,10 @@ static Sys_var_long Sys_pfs_connect_attr > GLOBAL_VAR(pfs_param.m_session_connect_attrs_sizing), > CMD_LINE(REQUIRED_ARG), VALID_RANGE(-1, 1024 * 1024), > DEFAULT(-1), > - BLOCK_SIZE(1)); > + BLOCK_SIZE(1), > + NO_MUTEX_GUARD, NOT_IN_BINLOG); Remove this change too (sysvars are NO_MUTEX_GUARD and NOT_IN_BINLOG by default) > > +#endif /* EMBEDDED_LIBRARY */ > #endif /* WITH_PERFSCHEMA_STORAGE_ENGINE */ > > static Sys_var_ulong Sys_auto_increment_increment( > > === modified file 'libmysqld/lib_sql.cc' > --- a/libmysqld/lib_sql.cc 2013-08-01 12:09:26 +0000 > +++ b/libmysqld/lib_sql.cc 2013-09-19 10:19:36 +0000 > @@ -698,12 +698,38 @@ err: > } > > > +static void > +emb_transfer_connect_attrs(MYSQL *mysql) > +{ > +#ifdef HAVE_PSI_THREAD_INTERFACE > + if (mysql->options.extension && > + mysql->options.extension->connection_attributes_length) > + { > + uchar *buf, *ptr; > + THD *thd= (THD*)mysql->thd; > + size_t length= mysql->options.extension->connection_attributes_length; > + > + /* 9 = max length of the serialized length */ > + ptr= buf= (uchar *) my_alloca(length + 9); > + send_client_connect_attrs(mysql, buf); > + net_field_length_ll(&ptr); > + PSI_THREAD_CALL(set_thread_connect_attrs)((char *) ptr, length, thd->charset()); > + my_afree(buf); > + } > +#endif > +} > + > + > #ifdef NO_EMBEDDED_ACCESS_CHECKS > int check_embedded_connection(MYSQL *mysql, const char *db) > { > int result; > LEX_STRING db_str = { (char*)db, db ? strlen(db) : 0 }; > THD *thd= (THD*)mysql->thd; > + > + /* the server does the same as the client */ > + mysql->server_capabilities= mysql->client_flag; > + Remove the similar line in init_embedded_mysql() > thd_init_client_charset(thd, mysql->charset->number); > thd->update_charset(); > Security_context *sctx= thd->security_ctx; > @@ -728,11 +755,17 @@ int check_embedded_connection(MYSQL *mys > we emulate a COM_CHANGE_USER user here, > it's easier than to emulate the complete 3-way handshake > */ > - char buf[USERNAME_LENGTH + SCRAMBLE_LENGTH + 1 + 2*NAME_LEN + 2], *end; > + char *buf, *end; > NET *net= &mysql->net; > THD *thd= (THD*)mysql->thd; > Security_context *sctx= thd->security_ctx; > + size_t connect_attrs_len= > + (mysql->server_capabilities & CLIENT_CONNECT_ATTRS && This doesn't seem to make any sense. Why does it check for CLIENT_CONNECT_ATTRS if it's part of the server (embedded). If a client connects to the server, then the client needs to check whether the server supports CLIENT_CONNECT_ATTRS, and the server needs to check whether the client supports it. But a server doesn't need to check whether it itself support it, it knows it already :) > + mysql->options.extension) ? > + mysql->options.extension->connection_attributes_length : 0; > > + buf= my_alloca(USERNAME_LENGTH + SCRAMBLE_LENGTH + 1 + 2*NAME_LEN + 2 + > + connect_attrs_len + 2); > if (mysql->options.client_ip) > { > sctx->host= my_strdup(mysql->options.client_ip, MYF(0)); > @@ -766,6 +799,13 @@ int check_embedded_connection(MYSQL *mys > int2store(end, (ushort) mysql->charset->number); > end+= 2; > > + //end= strmake(end, "mysql_native_password", NAME_LEN) + 1; Why is it commented out? > + > + /* the server does the same as the client */ > + mysql->server_capabilities= mysql->client_flag; > + > + end= (char *) send_client_connect_attrs(mysql, (uchar *) end); > + > /* acl_authenticate() takes the data from thd->net->read_pos */ > thd->net.read_pos= (uchar*)buf; Regards, Sergei

1 1

[Maria-developers] Please review: EXPLAIN UPDATE/DELETE, post-review changes.
by Sergei Petrunia 17 Sep '13

17 Sep '13

Hello Igor, I think I'm ready to submit the EXPLAIN UPDATE/DELETE code refactoring patch for another round of review. The code is pushed to lp:~~maria-captains/maria/10.0-base-explain-update-r2/ . I am also attaching the combined patch (against 10.0-base). BR Sergei -- Sergei Petrunia, Software Developer MariaDB | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

[Maria-developers] BUG? at windows versino of mariadb 10.0.4
by Roberto Spadim 16 Sep '13

16 Sep '13

Hi guys, i was trying the mariadb with lower value of thread_pool_max_threads to 3 and after my fourth connection i got a error and server crashed, it's a new server, i downloaded the 32bits version of mariadb, unziped and run this command line: C:\Program Files\mariadb-10.0.4-win32\bin>mysqld --datadir=..\data\ --log_error=..\log --port=3306 --thread_pool_max_threads=3 the error output at log was: 130915 21:43:37 [ERROR] mysqld got exception 0x80000003 ; This could be because you hit a bug. It is also possible that this binary or one of the libraries it was linked against is corrupt, improperly built, or misconfigured. This error can also be caused by malfunctioning hardware. To report this bug, see http://kb.askmonty.org/en/reporting-bugs We will try our best to scrape up some info that will hopefully help diagnose the problem, but since we have already crashed, something is definitely wrong and this may fail. Server version: 10.0.4-MariaDB key_buffer_size=134217728 read_buffer_size=131072 max_used_connections=0 max_threads=4 thread_count=0 It is possible that mysqld could use up to key_buffer_size + (read_buffer_size + sort_buffer_size)*max_threads = 141750 K bytes of memory Hope that's ok; if not, decrease some variables in the equation. Thread pointer: 0x0x0 Attempting backtrace. You can use the following information to find out where mysqld died. If you see no messages after this, something went terribly wrong... mysqld.exe!os_aio_windows_handle()[os0file.cc:4786] mysqld.exe!fil_aio_wait()[fil0fil.cc:5396] mysqld.exe!io_handler_thread()[srv0start.cc:477] kernel32.dll!BaseThreadInitThunk() ntdll.dll!RtlInitializeExceptionChain() ntdll.dll!RtlInitializeExceptionChain() The manual page at http://dev.mysql.com/doc/mysql/en/crashing.html contains information that should help you find out what is causing the crash. any help is wellcome, maybe the max_threads is bigger than thread_pool_max_threads? that's could be the problem? i didn't checked the source yet -- Roberto Spadim SPAEmpresarial

2 2

Re: [Maria-developers] review of mdev-4911
by Roberto Spadim 16 Sep '13

16 Sep '13

Hi! MDEV Created 1. MDEV-5018 <https://mariadb.atlassian.net/browse/MDEV-5018> 2013/9/14 Roberto Spadim <roberto(a)spadim.com.br> > Hi sergey :) well 10.1 with kill and subquery :) nice feature :) maybe too > much, but very nice > > > Em 14/09/2013 13:58, "Sergei Golubchik" <serg(a)mariadb.org> escreveu: > > Hi, Roberto! >> >> On Sep 13, Roberto Spadim wrote: >> > >> > > My preference is to kill connection by thread_id and query by >> > > query_id, because I normally either want to stop a particular query, >> > > or stop all activity in particular connection. But it is >> > > incompatible change. >> > > >> > there's mysql work with this kind of syntax? i didn't found it at >> bug.mysql >> > at mail list, Justin at Percona, talked about patchs in others forks >> maybe >> > a single unique syntax is better than maridb only syntax, check message >> > from MDEV description (copied from mail list): >> > >> > Justin Swanhartgreenlion(a)gmail.com Percona, Inc >> > >> > KILL THREAD THREAD_ID WITH QUERY_ID QUERY_ID >> > and >> > KILL QUERY THREAD_ID WITH QUERY_ID QUERY_ID >> > and possibly >> > KILL QUERY WITH QUERY_ID >> > >> > should be supported. This is a very important and missing feature >> > which is included in other forks. >> >> It's not in MySQL 5.6 and not in Percona Server 5.5 or 5.6. >> I did not check Google patches, Facebook patches and other sets of MySQL >> patches, though. >> > i didn`t found too :/ > >> >> > my problem isn't the program allowing a kill command since it can >> restart >> > the work or stop, it's not a problem >> > the "problem" is the boring time lost at a wrong kill command, since i >> use >> > persistent connections at php, and a thread running a script can be >> used in >> > another script without changing it thread_id (can be confirmed at show >> > processlist) >> > my problem is sending a kill command to the wrong thread since i'm using >> > the thread_id to kill the connection and not the query_id, check i use >> > "kill connection xxx" not "kill query xxx" >> >> Okay... This makes sense. If you use a connection pool that, indeed, >> connection id does not correspond to a logical connection. >> >> Still, while KILL CONNECTION QUERY_ID is kind of ok, KILL QUERY QUERY_ID >> is very silly. I'd rather allow subqueries in KILL, to support >> > kill query query_id is very ugly hehehe, kill query id is ok, and kill > connection query_id or query id with space is ok too > > >> >> KILL CONNECTION (select thread_id from information_schema.processlist >> where ...) >> >> then you won't need to kill by query id or state or if_idle - you can >> have everything in the where clause. >> >> but this is a larger change that we cannot do 10.0, we simply don't have >> time for it. We could try to do it in 10.1 though, properly and >> generally, so that you can kill using as complex conditions as you like. >> Instead of creating many limited shortcut syntax variants for special >> cases. >> > yeah =) in future we can go back and do some job here > > >> >> > i'm not using the threadpool yet and i don't know how processlist is >> > reported with thread pool, is the id isn't unique in this case (using >> > threadpool)? >> >> Unique. Every connection has its own connection id, with or without >> thread pool. Internal scheduling implementation doesn't affect >> user-visible connection ids. >> > nice i will test thread pool and check how it works > > >> >> > i don't know if it's what mariadb/mysql should do inside code when using >> > threadpool, i'm using only one process per connection, and don't have >> this >> > kind of problem >> >> threadpool doesn't have this problem either. >> >> > other doubt now... when we have a daemon process (plugin) there's a >> > query id for it? >> >> No. >> >> > query_id=0? in this case we only have an "unique query id" with >> > thread_id+query_id? maybe we should avoid the KILL QUERY_ID = 0 >> >> Right, thanks. >> >> > again about sintax... maybe a WHERE could be added to KILL instead of a >> > DELETE FROM INFORMATION_SCHEMA... >> > >> > KILL [CONNECTION | QUERY] [WHERE some_fields some_operators some_values >> > and_no_subquery | <thread_id> | QUERY_ID <query_id>] >> > >> > about WHERE, we could use the same fields of show processlist: >> > >> > ID, USER, HOST, DB, COMMAND, TIME, STATE, INFO, TIME_MS, STAGE, >> > MAX_STAGE, PROGRESS, MEMORY_USED, EXAMINED_ROWS, QUERY_ID >> >> I'd rather allow subqueries there, it'll be much more natural. >> > yes maybe information schema engine, or kill subquery? > > >> >> Regards, >> Sergei >> > > bye :) > -- Roberto Spadim SPAEmpresarial

1 0

[Maria-developers] Review of patch for MDEV-4820
by Kristian Nielsen 16 Sep '13

16 Sep '13

I took a close look at your patch for MDEV-4820. I think there is a fundamental disconnect. In MariaDB GTID, I do not require or rely on monotonically increasing seqeunce numbers (monoticity is requred per-server-id, but not between different servers). Nor do I enforce or rely on absence of holes in the sequence numbers. This decision was a hard one to make and I spent considerable thought on this point quite early. It is true that this design reduces possibilities to detect some kinds of errors, like missing events and alternate futures. I can understand if this design is not optimal for what you are trying to do. However, implementing two different designs (eg. based on value of gtid_strict_mode) is not workable. I believe at least for the first version, the current design is what we have to work with. The gtid_strict_mode is about helping the user by giving an error instead of logging certain undeirable event sequences to the binlog. It should not cause different non-error behaviour ever, nor should it prevent working with binlogs that were logged when strict mode was not enabled. > diff --git a/sql/rpl_gtid.cc b/sql/rpl_gtid.cc > --- a/sql/rpl_gtid.cc > +++ b/sql/rpl_gtid.cc > @@ -993,7 +993,7 @@ rpl_binlog_state::check_strict_sequence(uint32 domain_id, uint32 server_id, > > if ((elem= (element *)my_hash_search(&hash, > (const uchar *)(&domain_id), 0)) && > - elem->last_gtid && elem->last_gtid->seq_no >= seq_no) > + elem->last_gtid && elem->last_gtid->seq_no + 1 != seq_no) If I understand this part correctly, this is about enforcing that there are no holes in sequence numbers. This goes against the GTID design. > @@ -854,8 +857,16 @@ contains_all_slave_gtid(slave_connection_state *st, Gtid_list_log_event *glev) > */ > return false; > } > - if (gtid->server_id == glev->list[i].server_id && > - gtid->seq_no <= glev->list[i].seq_no) > + if (slave_gtid_strict_mode) > + { > + if (gtid->seq_no < glev->list[i].seq_no) > + { > + has_greater_seq_no= true; > + break; > + } > + } I think here you want to depend on sequence numbers being monotonic. This does not work, as you cannot be sure that strict mode was enabled when this binlog file was written. > - if (!rpl_global_gtid_slave_state.domain_to_gtid(slave_gtid->domain_id, > - &master_replication_gtid) || > + bool master_repl_gtid_found = > + rpl_global_gtid_slave_state.domain_to_gtid(slave_gtid->domain_id, > + &master_replication_gtid); > + if (!master_repl_gtid_found || > slave_gtid->server_id != master_replication_gtid.server_id || > slave_gtid->seq_no != master_replication_gtid.seq_no) > { Right, so slave requests something in a domain that we do not have anything about in the binlog, but we do have something in the slave state (but not the correct one). So this is a bug in the current code. If slave requests to start at a position in a domain we have nothing in, either that domain is served by a different master, or slave wants to start from the very first event in that domain. Currently, we fail to check in the second case that we actually have the correct GTID as the first event. But this is the wrong way to fix it. A master should not be prevented from serving some domain D1 just because it has some old junk for another domain D2 in the slave state. Instead, when we allow to start in a domain that is missing, and then later actually encounter an event in this domain, we should check that it matches what the slave originally requested. If not, we should give an error. I'll try to find time to do this. > @@ -1191,6 +1239,16 @@ gtid_find_binlog_file(slave_connection_state *state, char *out_name, > */ > state->remove(gtid); > } > + else if (slave_gtid_strict_mode && > + gtid->seq_no == glev->list[i].seq_no) > + { > + *errormsg= "Requested slave GTID state have different server id " > + "than the one in binlog"; > + *error_gtid= *gtid; > + *good_gtid= glev->list[i]; > + error= ER_SLAVE_IS_FROM_ALTERNATE_FUTURE; > + goto end; > + } This can fail if the binlog file was written when strict mode was not enabled. > > @@ -1559,7 +1620,39 @@ send_event_to_slave(THD *thd, NET *net, String* const packet, ushort flags, > event_gtid.seq_no <= gtid->seq_no) > *gtid_skip_group = (flags2 & Gtid_log_event::FL_STANDALONE ? > GTID_SKIP_STANDALONE : GTID_SKIP_TRANSACTION); > - if (event_gtid.server_id == gtid->server_id && > + if (slave_gtid_strict_mode) > + { > + if (event_gtid.seq_no == gtid->seq_no && > + event_gtid.server_id != gtid->server_id) > + { > + my_errno= ER_SLAVE_IS_FROM_ALTERNATE_FUTURE; > + *error_gtid= *gtid; > + *good_gtid= event_gtid; > + return "Requested slave GTID state have different server id " > + "than the one in binlog"; > + } Again, what if this is an old part of the binlog where strict mode was disabled? > + else if (*first_event && event_gtid.seq_no > gtid->seq_no) > + { > + /* > + Earlier we decided that this binlog file has event that we need, > + but the first event in the file has seq_no beyond what we need. > + This can happen in two cases: > + - this file is the first binlog we have and Gtid_list event > + in it is empty; > + - there is a gap between the last event mentioned in Gtid_list > + and the first event in the binlog file. > + As the first reason is the most probable one let's issue the > + appropriate error message. > + */ > + my_errno= ER_MASTER_FATAL_ERROR_READING_BINLOG; > + *error_gtid= *gtid; > + return "Could not find GTID state requested by slave in any " > + "binlog files. Probably the slave state is too old and " > + "required binlog files have been purged"; > + } > + } This "first_event" (which I admit I do not fully understand) looks wrong - all such things usually need to be per-domain. However, I think this is the place in the code I mention above, where we get an event in a domain that the slave requested, but we did not have anything at connect time. So here I want an even stricter check, and also in non-strict mode: The first GTID in the domain must be _equal_ to what the slave requested, or we should fail. ---- Next I will take a closer look at your test cases and see what can be done to fix it in a way compatible with the basic design. I also want to re-iterate the suggestion to not delete binlogs when cloning a master to a new master. The most recent binlog file has real information (in the initial Gtid_list event) that is needed for proper operation as a master, and it really doesn't make sense to first purposely corrupt the server state and subsequently try to hack the code to fix the damage. If one really wanted not to copy any binlog files, then a better idea would be a way to restore the state, eg. a RESET MASTER TO gtid_list='xxx,yyy,...' command or something like that. Hope this helps, - Kristian.

3 23

[Maria-developers] review of mdev-4911
by Sergei Golubchik 14 Sep '13

14 Sep '13

Hi. Good work. I only have one small comment about the code, see below. And one about the syntax. > === modified file 'sql/sql_parse.cc' > --- sql/sql_parse.cc 2013-08-14 08:48:50 +0000 > +++ sql/sql_parse.cc 2013-09-11 12:29:32 +0000 > @@ -7139,24 +7139,60 @@ THD *find_thread_by_id(ulong id) > > > /** > - kill on thread. > + Find a thread by query id and return it, locking it LOCK_thd_data > + > + @param id Identifier of the query we're looking for > + > + @return NULL - not found > + pointer - thread found, and its LOCK_thd_data is locked. > +*/ > + > +static THD *find_thread_by_query_id(longlong id) why didn't you reuse find_thread_by_id()? this function is almost identical to it. > +{ > + THD *tmp; > + mysql_mutex_lock(&LOCK_thread_count); // For unlink from list > + I_List_iterator<THD> it(threads); > + while ((tmp=it++)) > + { > + if (tmp->get_command() == COM_DAEMON) > + continue; > + if (tmp->query_id == id) > + { > + mysql_mutex_lock(&tmp->LOCK_thd_data); // Lock from delete > + break; > + } > + } > + mysql_mutex_unlock(&LOCK_thread_count); > + return tmp; > +} > + > + > === modified file 'sql/sql_yacc.yy' > --- sql/sql_yacc.yy 2013-08-13 11:35:36 +0000 > +++ sql/sql_yacc.yy 2013-09-11 12:29:32 +0000 > @@ -12890,6 +12891,11 @@ kill_expr: > Lex->users_list.push_back($2); > Lex->kill_type= KILL_TYPE_USER; > } > + | ID_SYM expr > + { > + Lex->value_list.push_front($2); > + Lex->kill_type= KILL_TYPE_QUERY; > + } So, you implemented KILL [ CONNECTION | QUERY ] [ ID ] expr It allows, in particular KILL CONNECTION ID 10 which looks strange, why would that mean that 10 is a query id? I originally suggested KILL [ CONNECTION | QUERY [ ID ] ] expr to allow only KILL CONNECTION expr KILL QUERY expr KILL QUERY ID expr because in this case it's quite clear "QUERY ID" and because I thought it's a bit strange to kill a connection by query_id. If you want to allow that (as I saw in the comments, Roberto didn't thought it's strange to kill a connection by query id), may be you'd better use KILL [ CONNECTION | QUERY ] [ QUERY_ID ] expr ? Regards, Sergei

3 5

[Maria-developers] Can't sign up to JIRA
by Kouhei Sutou 13 Sep '13

13 Sep '13

Hi, I tried to sign up to JIRA(*) for submitting a bug report. (*) https://mariadb.atlassian.net/secure/Signup!default.jspa But I couldn't sign up because I couldn't get captcha image. I used "Chromium 28.0.1500.95 Debian jessie/sid (213514)" and "Iceweasel 7.0.8" on Debian GNU/Linux sid as Web browsers. I got the same result with these Web browsers. I checked captcha image response by Chromium's developer tool. https://mariadb.atlassian.net/captcha returns "500 Internal Server Error" with the following body. Could you confirm it? ---- We had some technical problems during your last operation. We deeply apologize for that :( If you need further assistance, please: Expand the section "Technical details" below Copy the content from it Send it to your JIRA Administrator Technical details Cause Referer URL: https://mariadb.atlassian.net/secure/Signup!default.jspa java.lang.NoClassDefFoundError: Could not initialize class com.atlassian.jira.servlet.NoOpImageCaptchaService java.lang.NoClassDefFoundError: Could not initialize class com.atlassian.jira.servlet.NoOpImageCaptchaService at com.atlassian.jira.servlet.NoOpCaptchaServiceImpl.getImageCaptchaService(NoOpCaptchaServiceImpl.java:14) at com.atlassian.jira.servlet.JiraCaptchaServlet.doGet(JiraCaptchaServlet.java:50) at javax.servlet.http.HttpServlet.service(HttpServlet.java:617) at javax.servlet.http.HttpServlet.service(HttpServlet.java:717) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:290) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) at com.atlassian.jira.web.filters.steps.ChainedFilterStepRunner.doFilter(ChainedFilterStepRunner.java:87) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:235) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) at com.atlassian.jira.web.filters.XContentTypeOptionsNoSniffFilter.doFilter(XContentTypeOptionsNoSniffFilter.java:22) at com.atlassian.core.filters.AbstractHttpFilter.doFilter(AbstractHttpFilter.java:31) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:235) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) at com.atlassian.core.filters.HeaderSanitisingFilter.doFilter(HeaderSanitisingFilter.java:44) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:235) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) at com.atlassian.plugin.servlet.filter.IteratingFilterChain.doFilter(IteratingFilterChain.java:46) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter$1.doFilter(DelegatingPluginFilter.java:66) at com.atlassian.jira.tzdetect.IncludeResourcesFilter.doFilter(IncludeResourcesFilter.java:39) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter.doFilter(DelegatingPluginFilter.java:74) at com.atlassian.plugin.servlet.filter.IteratingFilterChain.doFilter(IteratingFilterChain.java:42) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter$1.doFilter(DelegatingPluginFilter.java:66) at com.atlassian.jira.baseurl.IncludeResourcesFilter.doFilter(IncludeResourcesFilter.java:38) at com.atlassian.core.filters.AbstractHttpFilter.doFilter(AbstractHttpFilter.java:31) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter.doFilter(DelegatingPluginFilter.java:74) at com.atlassian.plugin.servlet.filter.IteratingFilterChain.doFilter(IteratingFilterChain.java:42) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter$1.doFilter(DelegatingPluginFilter.java:66) at com.atlassian.applinks.core.rest.context.ContextFilter.doFilter(ContextFilter.java:25) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter.doFilter(DelegatingPluginFilter.java:74) at com.atlassian.plugin.servlet.filter.IteratingFilterChain.doFilter(IteratingFilterChain.java:42) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter$1.doFilter(DelegatingPluginFilter.java:66) at com.atlassian.mywork.client.filter.ServingRequestsFilter.doFilter(ServingRequestsFilter.java:37) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter.doFilter(DelegatingPluginFilter.java:74) at com.atlassian.plugin.servlet.filter.IteratingFilterChain.doFilter(IteratingFilterChain.java:42) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter$1.doFilter(DelegatingPluginFilter.java:66) at com.atlassian.studio.core.servlet.filter.AlacarteLicenseEnforcer.doFilter(AlacarteLicenseEnforcer.java:71) at com.atlassian.studio.jira.servlet.filter.AlacarteLicenseJiraEnforcer.doFilter(AlacarteLicenseJiraEnforcer.java:121) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter.doFilter(DelegatingPluginFilter.java:74) at com.atlassian.plugin.servlet.filter.IteratingFilterChain.doFilter(IteratingFilterChain.java:42) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter$1.doFilter(DelegatingPluginFilter.java:66) at com.atlassian.prettyurls.filter.PrettyUrlsDispatcherFilter.doFilter(PrettyUrlsDispatcherFilter.java:60) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter.doFilter(DelegatingPluginFilter.java:74) at com.atlassian.plugin.servlet.filter.IteratingFilterChain.doFilter(IteratingFilterChain.java:42) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter$1.doFilter(DelegatingPluginFilter.java:66) at com.atlassian.prettyurls.filter.PrettyUrlsSiteMeshFilter.doFilter(PrettyUrlsSiteMeshFilter.java:92) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter.doFilter(DelegatingPluginFilter.java:74) at com.atlassian.plugin.servlet.filter.IteratingFilterChain.doFilter(IteratingFilterChain.java:42) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter$1.doFilter(DelegatingPluginFilter.java:66) at com.atlassian.prettyurls.filter.PrettyUrlsMatcherFilter.doFilter(PrettyUrlsMatcherFilter.java:56) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter.doFilter(DelegatingPluginFilter.java:74) at com.atlassian.plugin.servlet.filter.IteratingFilterChain.doFilter(IteratingFilterChain.java:42) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter$1.doFilter(DelegatingPluginFilter.java:66) at com.atlassian.labs.botkiller.BotKillerFilter.doFilter(BotKillerFilter.java:36) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter.doFilter(DelegatingPluginFilter.java:74) at com.atlassian.plugin.servlet.filter.IteratingFilterChain.doFilter(IteratingFilterChain.java:42) at com.atlassian.plugin.servlet.filter.ServletFilterModuleContainerFilter.doFilter(ServletFilterModuleContainerFilter.java:77) at com.atlassian.plugin.servlet.filter.ServletFilterModuleContainerFilter.doFilter(ServletFilterModuleContainerFilter.java:63) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:235) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) at com.atlassian.jira.web.filters.accesslog.AccessLogFilter.executeRequest(AccessLogFilter.java:103) at com.atlassian.jira.web.filters.accesslog.AccessLogFilter.doFilter(AccessLogFilter.java:87) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:235) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) at com.atlassian.jira.security.xsrf.XsrfTokenAdditionRequestFilter.doFilter(XsrfTokenAdditionRequestFilter.java:54) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:235) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) at com.atlassian.jira.web.filters.steps.ChainedFilterStepRunner.doFilter(ChainedFilterStepRunner.java:87) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:235) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) at com.atlassian.plugin.servlet.filter.IteratingFilterChain.doFilter(IteratingFilterChain.java:46) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter$1.doFilter(DelegatingPluginFilter.java:66) at com.atlassian.plugin.remotable.plugin.module.permission.ApiScopingFilter.doFilter(ApiScopingFilter.java:63) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter.doFilter(DelegatingPluginFilter.java:74) at com.atlassian.plugin.servlet.filter.IteratingFilterChain.doFilter(IteratingFilterChain.java:42) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter$1.doFilter(DelegatingPluginFilter.java:66) at com.atlassian.prettyurls.filter.PrettyUrlsCombinedMatchDispatcherFilter.doFilter(PrettyUrlsCombinedMatchDispatcherFilter.java:61) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter.doFilter(DelegatingPluginFilter.java:74) at com.atlassian.plugin.servlet.filter.IteratingFilterChain.doFilter(IteratingFilterChain.java:42) at com.atlassian.plugin.servlet.filter.ServletFilterModuleContainerFilter.doFilter(ServletFilterModuleContainerFilter.java:77) at com.atlassian.plugin.servlet.filter.ServletFilterModuleContainerFilter.doFilter(ServletFilterModuleContainerFilter.java:63) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:235) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) at com.atlassian.seraph.filter.SecurityFilter.doFilter(SecurityFilter.java:234) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:235) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) at com.atlassian.security.auth.trustedapps.filter.TrustedApplicationsFilter.doFilter(TrustedApplicationsFilter.java:98) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:235) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) at com.atlassian.seraph.filter.BaseLoginFilter.doFilter(BaseLoginFilter.java:169) at com.atlassian.jira.web.filters.JiraLoginFilter.doFilter(JiraLoginFilter.java:70) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:235) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) at com.atlassian.plugin.servlet.filter.IteratingFilterChain.doFilter(IteratingFilterChain.java:46) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter$1.doFilter(DelegatingPluginFilter.java:66) at com.atlassian.oauth.serviceprovider.internal.servlet.OAuthFilter.doFilter(OAuthFilter.java:55) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter.doFilter(DelegatingPluginFilter.java:74) at com.atlassian.plugin.servlet.filter.IteratingFilterChain.doFilter(IteratingFilterChain.java:42) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter$1.doFilter(DelegatingPluginFilter.java:66) at com.atlassian.prettyurls.filter.PrettyUrlsCombinedMatchDispatcherFilter.doFilter(PrettyUrlsCombinedMatchDispatcherFilter.java:61) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter.doFilter(DelegatingPluginFilter.java:74) at com.atlassian.plugin.servlet.filter.IteratingFilterChain.doFilter(IteratingFilterChain.java:42) at com.atlassian.plugin.servlet.filter.ServletFilterModuleContainerFilter.doFilter(ServletFilterModuleContainerFilter.java:77) at com.atlassian.plugin.servlet.filter.ServletFilterModuleContainerFilter.doFilter(ServletFilterModuleContainerFilter.java:63) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:235) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) at org.tuckey.web.filters.urlrewrite.RuleChain.handleRewrite(RuleChain.java:176) at org.tuckey.web.filters.urlrewrite.RuleChain.doRules(RuleChain.java:145) at org.tuckey.web.filters.urlrewrite.UrlRewriter.processRequest(UrlRewriter.java:92) at org.tuckey.web.filters.urlrewrite.UrlRewriteFilter.doFilter(UrlRewriteFilter.java:394) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:235) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) at com.atlassian.gzipfilter.GzipFilter.doFilterInternal(GzipFilter.java:80) at com.atlassian.gzipfilter.GzipFilter.doFilter(GzipFilter.java:51) at com.atlassian.jira.web.filters.gzip.JiraGzipFilter.doFilter(JiraGzipFilter.java:55) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:235) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) at com.atlassian.plugin.servlet.filter.IteratingFilterChain.doFilter(IteratingFilterChain.java:46) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter$1.doFilter(DelegatingPluginFilter.java:66) at com.atlassian.plugin.remotable.plugin.module.oauth.OAuth2LOFilter.doFilter(OAuth2LOFilter.java:81) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter.doFilter(DelegatingPluginFilter.java:74) at com.atlassian.plugin.servlet.filter.IteratingFilterChain.doFilter(IteratingFilterChain.java:42) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter$1.doFilter(DelegatingPluginFilter.java:66) at com.atlassian.plugin.remotable.host.common.service.http.bigpipe.BigPipeRequestIdFilter.doFilter(BigPipeRequestIdFilter.java:33) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter.doFilter(DelegatingPluginFilter.java:74) at com.atlassian.plugin.servlet.filter.IteratingFilterChain.doFilter(IteratingFilterChain.java:42) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter$1.doFilter(DelegatingPluginFilter.java:66) at com.atlassian.analytics.client.filter.AnalyticsFilter.doFilter(AnalyticsFilter.java:28) at com.atlassian.core.filters.AbstractHttpFilter.doFilter(AbstractHttpFilter.java:31) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter.doFilter(DelegatingPluginFilter.java:74) at com.atlassian.plugin.servlet.filter.IteratingFilterChain.doFilter(IteratingFilterChain.java:42) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter$1.doFilter(DelegatingPluginFilter.java:66) at com.atlassian.prettyurls.filter.PrettyUrlsCombinedMatchDispatcherFilter.doFilter(PrettyUrlsCombinedMatchDispatcherFilter.java:61) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter.doFilter(DelegatingPluginFilter.java:74) at com.atlassian.plugin.servlet.filter.IteratingFilterChain.doFilter(IteratingFilterChain.java:42) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter$1.doFilter(DelegatingPluginFilter.java:66) at com.sysbliss.jira.plugins.workflow.servlet.JWDSendRedirectFilter.doFilter(JWDSendRedirectFilter.java:25) at com.atlassian.plugin.servlet.filter.DelegatingPluginFilter.doFilter(DelegatingPluginFilter.java:74) at com.atlassian.plugin.servlet.filter.IteratingFilterChain.doFilter(IteratingFilterChain.java:42) at com.atlassian.plugin.servlet.filter.ServletFilterModuleContainerFilter.doFilter(ServletFilterModuleContainerFilter.java:77) at com.atlassian.plugin.servlet.filter.ServletFilterModuleContainerFilter.doFilter(ServletFilterModuleContainerFilter.java:63) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:235) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) at com.atlassian.jira.web.filters.steps.ChainedFilterStepRunner.doFilter(ChainedFilterStepRunner.java:87) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:235) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) at com.atlassian.core.filters.cache.AbstractCachingFilter.doFilter(AbstractCachingFilter.java:33) at com.atlassian.core.filters.AbstractHttpFilter.doFilter(AbstractHttpFilter.java:31) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:235) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) at com.atlassian.core.filters.encoding.AbstractEncodingFilter.doFilter(AbstractEncodingFilter.java:41) at com.atlassian.core.filters.AbstractHttpFilter.doFilter(AbstractHttpFilter.java:31) at com.atlassian.jira.web.filters.PathMatchingEncodingFilter.doFilter(PathMatchingEncodingFilter.java:49) at com.atlassian.core.filters.AbstractHttpFilter.doFilter(AbstractHttpFilter.java:31) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:235) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) at com.atlassian.jira.startup.JiraStartupChecklistFilter.doFilter(JiraStartupChecklistFilter.java:78) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:235) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) at com.atlassian.jira.web.filters.steps.ChainedFilterStepRunner.doFilter(ChainedFilterStepRunner.java:87) at com.atlassian.jira.web.filters.JiraFirstFilter.doFilter(JiraFirstFilter.java:57) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:235) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:233) at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:191) at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:127) at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:102) at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:109) at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:298) at org.apache.jk.server.JkCoyoteHandler.invoke(JkCoyoteHandler.java:190) at org.apache.jk.common.HandlerRequest.invoke(HandlerRequest.java:291) at org.apache.jk.common.ChannelSocket.invoke(ChannelSocket.java:776) at org.apache.jk.common.ChannelSocket.processConnection(ChannelSocket.java:705) at org.apache.jk.common.ChannelSocket$SocketConnection.runIt(ChannelSocket.java:898) at org.apache.tomcat.util.threads.ThreadPool$ControlRunnable.run(ThreadPool.java:690) at java.lang.Thread.run(Thread.java:662) Go back to the previous page ---- Thanks, -- kou

4 8

Re: [Maria-developers] [Commits] Rev 3810: MDEV-4994: Crash in dynamic column API in file:///home/bell/maria/bzr/work-maria-10.0-MDEV-4994/
by Kristian Nielsen 13 Sep '13

13 Sep '13

sanja(a)askmonty.org writes: > At file:///home/bell/maria/bzr/work-maria-10.0-MDEV-4994/ > > ------------------------------------------------------------ > revno: 3810 > revision-id: sanja(a)askmonty.org-20130911151436-rjz6pw6fh6rtj83e > parent: svoj(a)mariadb.org-20130904072837-u3zj89fq4xvb4cnp > committer: sanja(a)askmonty.org > branch nick: work-maria-10.0-MDEV-4994 > timestamp: Wed 2013-09-11 18:14:36 +0300 > message: > MDEV-4994: Crash in dynamic column API > Dynamic columns unittest fix. > > Correct processing of empty string added to creation dynamic string procedure > List function of numeric format fixed when empty string passed. > Unit test calls changed to new ones. > Unit test added to cmake file. > === modified file 'mysys/ma_dyncol.c' > --- a/mysys/ma_dyncol.c 2013-08-07 15:40:25 +0000 > +++ b/mysys/ma_dyncol.c 2013-09-11 15:14:36 +0000 > @@ -1618,15 +1618,22 @@ dynamic_new_column_store(DYNAMIC_COLUMN > > if (!(columns_order= malloc(sizeof(void*)*column_count))) > return ER_DYNCOL_RESOURCE; > - if (new_str) > + if (new_str || str->str == 0) > { > - if (dynamic_column_init_named(str, > - fmt->fixed_hdr + > - hdr->header_size + > - hdr->nmpool_size + > - hdr->data_size + > - DYNCOL_SYZERESERVE)) > - goto err; > + if (column_count) > + { > + if (dynamic_column_init_named(str, > + fmt->fixed_hdr + > + hdr->header_size + > + hdr->nmpool_size + > + hdr->data_size + > + DYNCOL_SYZERESERVE)) > + goto err; > + } > + else > + { > + dynamic_column_initialize(str); > + } > } > else > { > @@ -2438,7 +2445,8 @@ mariadb_dyncol_list(DYNAMIC_COLUMN *str, > uint i; > enum enum_dyncol_func_result rc; > > - (*nums)= 0; /* In case of errors */ > + (*nums)= 0; (*count)= 0; /* In case of errors */ > + > if (str->length == 0) > return ER_DYNCOL_OK; /* no columns */ > > > === modified file 'unittest/mysys/CMakeLists.txt' > --- a/unittest/mysys/CMakeLists.txt 2013-07-16 17:09:54 +0000 > +++ b/unittest/mysys/CMakeLists.txt 2013-09-11 15:14:36 +0000 > @@ -20,6 +20,9 @@ INCLUDE_DIRECTORIES(${CMAKE_SOURCE_DIR}/ > 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) > + > IF(WIN32) > MY_ADD_TESTS(my_delete LINK_LIBRARIES mysys) > ENDIF() > > === modified file 'unittest/mysys/ma_dyncol-t.c' > --- a/unittest/mysys/ma_dyncol-t.c 2012-07-31 17:29:07 +0000 > +++ b/unittest/mysys/ma_dyncol-t.c 2013-09-11 15:14:36 +0000 > @@ -35,17 +35,18 @@ > void test_value_single_null() > { > int rc= FALSE; > + uint ids[1]= {1}; > DYNAMIC_COLUMN_VALUE val, res; > DYNAMIC_COLUMN str; > /* init values */ > val.type= DYN_COL_NULL; > - dynamic_column_value_init(&res); > + mariadb_dyncol_value_init(&res); > /* create column */ > - if (dynamic_column_create(&str, 1, &val)) > + if (mariadb_dyncol_create_many(&str, 1, ids, &val, 1)) > goto err; > dynstr_append(&str, "\1"); str.length--; //check for overflow > /* read column */ > - if (dynamic_column_get(&str, 1, &res)) > + if (mariadb_dyncol_get(&str, 1, &res)) > goto err; > rc= (res.type == DYN_COL_NULL); > err: > @@ -57,18 +58,19 @@ err: > void test_value_single_uint(ulonglong num, const char *name) > { > int rc= FALSE; > + uint ids[1]= {1}; > DYNAMIC_COLUMN_VALUE val, res; > DYNAMIC_COLUMN str; > /* init values */ > val.type= DYN_COL_UINT; > val.x.ulong_value= num; > - dynamic_column_value_init(&res); > + mariadb_dyncol_value_init(&res); > /* create column */ > - if (dynamic_column_create(&str, 1, &val)) > + if (mariadb_dyncol_create_many(&str, 1, ids, &val, 1)) > goto err; > dynstr_append(&str, "\1"); str.length--; //check for overflow > /* read column */ > - if (dynamic_column_get(&str, 1, &res)) > + if (mariadb_dyncol_get(&str, 1, &res)) > goto err; > rc= (res.type == DYN_COL_UINT) && (res.x.ulong_value == num); > num= res.x.ulong_value; > @@ -81,18 +83,19 @@ err: > void test_value_single_sint(longlong num, const char *name) > { > int rc= FALSE; > + uint ids[1]= {1}; > DYNAMIC_COLUMN_VALUE val, res; > DYNAMIC_COLUMN str; > /* init values */ > val.type= DYN_COL_INT; > val.x.long_value= num; > - dynamic_column_value_init(&res); > + mariadb_dyncol_value_init(&res); > /* create column */ > - if (dynamic_column_create(&str, 1, &val)) > + if (mariadb_dyncol_create_many(&str, 1, ids, &val, 1)) > goto err; > dynstr_append(&str, "\1"); str.length--; //check for overflow > /* read column */ > - if (dynamic_column_get(&str, 1, &res)) > + if (mariadb_dyncol_get(&str, 1, &res)) > goto err; > rc= (res.type == DYN_COL_INT) && (res.x.long_value == num); > num= res.x.ulong_value; > @@ -106,18 +109,19 @@ err: > void test_value_single_double(double num, const char *name) > { > int rc= FALSE; > + uint ids[1]= {1}; > DYNAMIC_COLUMN_VALUE val, res; > DYNAMIC_COLUMN str; > /* init values */ > val.type= DYN_COL_DOUBLE; > val.x.double_value= num; > - dynamic_column_value_init(&res); > + mariadb_dyncol_value_init(&res); > /* create column */ > - if (dynamic_column_create(&str, 1, &val)) > + if (mariadb_dyncol_create_many(&str, 1, ids, &val, 1)) > goto err; > dynstr_append(&str, "\1"); str.length--; //check for overflow > /* read column */ > - if (dynamic_column_get(&str, 1, &res)) > + if (mariadb_dyncol_get(&str, 1, &res)) > goto err; > rc= (res.type == DYN_COL_DOUBLE) && (res.x.double_value == num); > num= res.x.ulong_value; > @@ -133,21 +137,22 @@ void test_value_single_decimal(const cha > char buff[80]; > int rc= FALSE; > int length= 80; > + uint ids[1]= {1}; > DYNAMIC_COLUMN_VALUE val, res; > DYNAMIC_COLUMN str; > > /* init values */ > - dynamic_column_prepare_decimal(&val); // special procedure for decimal!!! > + mariadb_dyncol_prepare_decimal(&val); // special procedure for decimal!!! > if (string2decimal(num, &val.x.decimal.value, &end) != E_DEC_OK) > goto err; > - dynamic_column_value_init(&res); > + mariadb_dyncol_value_init(&res); > > /* create column */ > - if (dynamic_column_create(&str, 1, &val)) > + if (mariadb_dyncol_create_many(&str, 1, ids, &val, 1)) > goto err; > dynstr_append(&str, "\1"); str.length--; //check for overflow > /* read column */ > - if (dynamic_column_get(&str, 1, &res)) > + if (mariadb_dyncol_get(&str, 1, &res)) > goto err; > rc= ((res.type == DYN_COL_DECIMAL) && > (decimal_cmp(&res.x.decimal.value, &val.x.decimal.value) == 0)); > @@ -206,6 +211,7 @@ void test_value_single_string(const char > CHARSET_INFO *cs) > { > int rc= FALSE; > + uint ids[1]= {1}; > DYNAMIC_COLUMN_VALUE val, res; > DYNAMIC_COLUMN str; > > @@ -214,14 +220,14 @@ void test_value_single_string(const char > val.x.string.value.str= (char*)string; > val.x.string.value.length= len; > val.x.string.charset= cs; > - dynamic_column_value_init(&res); > + mariadb_dyncol_value_init(&res); > > /* create column */ > - if (dynamic_column_create(&str, 1, &val)) > + if (mariadb_dyncol_create_many(&str, 1, ids, &val, 1)) > goto err; > dynstr_append(&str, "\1"); str.length--; //check for overflow > /* read column */ > - if (dynamic_column_get(&str, 1, &res)) > + if (mariadb_dyncol_get(&str, 1, &res)) > goto err; > rc= ((res.type == DYN_COL_STRING) && > (res.x.string.value.length == len) && > @@ -239,6 +245,7 @@ err: > void test_value_single_date(uint year, uint month, uint day, const char *name) > { > int rc= FALSE; > + uint ids[1]= {1}; > DYNAMIC_COLUMN_VALUE val, res; > DYNAMIC_COLUMN str; > /* init values */ > @@ -247,13 +254,13 @@ void test_value_single_date(uint year, u > val.x.time_value.year= year; > val.x.time_value.month= month; > val.x.time_value.day= day; > - dynamic_column_value_init(&res); > + mariadb_dyncol_value_init(&res); > /* create column */ > - if (dynamic_column_create(&str, 1, &val)) > + if (mariadb_dyncol_create_many(&str, 1, ids, &val, 1)) > goto err; > dynstr_append(&str, "\1"); str.length--; //check for overflow > /* read column */ > - if (dynamic_column_get(&str, 1, &res)) > + if (mariadb_dyncol_get(&str, 1, &res)) > goto err; > rc= ((res.type == DYN_COL_DATE) && > (res.x.time_value.time_type == MYSQL_TIMESTAMP_DATE) && > @@ -270,6 +277,7 @@ void test_value_single_time(uint neg, ui > uint mic, const char *name) > { > int rc= FALSE; > + uint ids[1]= {1}; > DYNAMIC_COLUMN_VALUE val, res; > DYNAMIC_COLUMN str; > /* init values */ > @@ -280,13 +288,13 @@ void test_value_single_time(uint neg, ui > val.x.time_value.minute= minute; > val.x.time_value.second= second; > val.x.time_value.second_part= mic; > - dynamic_column_value_init(&res); > + mariadb_dyncol_value_init(&res); > /* create column */ > - if (dynamic_column_create(&str, 1, &val)) > + if (mariadb_dyncol_create_many(&str, 1, ids, &val, 1)) > goto err; > dynstr_append(&str, "\1"); str.length--; //check for overflow > /* read column */ > - if (dynamic_column_get(&str, 1, &res)) > + if (mariadb_dyncol_get(&str, 1, &res)) > goto err; > rc= ((res.type == DYN_COL_TIME) && > (res.x.time_value.time_type == MYSQL_TIMESTAMP_TIME) && > @@ -308,6 +316,7 @@ void test_value_single_datetime(uint neg > uint mic, const char *name) > { > int rc= FALSE; > + uint ids[1]= {1}; > DYNAMIC_COLUMN_VALUE val, res; > DYNAMIC_COLUMN str; > /* init values */ > @@ -321,13 +330,13 @@ void test_value_single_datetime(uint neg > val.x.time_value.minute= minute; > val.x.time_value.second= second; > val.x.time_value.second_part= mic; > - dynamic_column_value_init(&res); > + mariadb_dyncol_value_init(&res); > /* create column */ > - if (dynamic_column_create(&str, 1, &val)) > + if (mariadb_dyncol_create_many(&str, 1, ids, &val, 1)) > goto err; > dynstr_append(&str, "\1"); str.length--; //check for overflow > /* read column */ > - if (dynamic_column_get(&str, 1, &res)) > + if (mariadb_dyncol_get(&str, 1, &res)) > goto err; > rc= ((res.type == DYN_COL_DATETIME) && > (res.x.time_value.time_type == MYSQL_TIMESTAMP_DATETIME) && > @@ -373,7 +382,7 @@ void test_value_multi(ulonglong num0, > val[1].x.long_value= num1; > val[2].type= DYN_COL_DOUBLE; > val[2].x.double_value= num2; > - dynamic_column_prepare_decimal(val + 3); // special procedure for decimal!!! > + mariadb_dyncol_prepare_decimal(val + 3); // special procedure for decimal!!! > if (string2decimal(num3, &val[3].x.decimal.value, &end3) != E_DEC_OK) > goto err; > val[4].type= DYN_COL_STRING; > @@ -404,14 +413,14 @@ void test_value_multi(ulonglong num0, > val[7].x.time_value.second_part= mic7; > val[8].type= DYN_COL_NULL; > for (i= 0; i < 9; i++) > - dynamic_column_value_init(res + i); > + mariadb_dyncol_value_init(res + i); > /* create column */ > - if (dynamic_column_create_many(&str, 9, column_numbers, val)) > + if (mariadb_dyncol_create_many(&str, 9, column_numbers, val, 1)) > goto err; > dynstr_append(&str, "\1"); str.length--; //check for overflow > /* read column */ > for (i= 0; i < 9; i++) > - if (dynamic_column_get(&str, column_numbers[i], res + i)) > + if (mariadb_dyncol_get(&str, column_numbers[i], res + i)) > goto err; > rc= ((res[0].type == DYN_COL_UINT) && > (res[0].x.ulong_value == num0) && > @@ -467,7 +476,7 @@ void test_value_multi_same_num() > for (i= 0; i < 5; i++) > val[i].type= DYN_COL_NULL; > /* create column */ > - if (!dynamic_column_create_many(&str, 5, column_numbers, val)) > + if (!mariadb_dyncol_create_many(&str, 5, column_numbers, val, 1)) > goto err; > rc= TRUE; > err: > @@ -487,68 +496,69 @@ void test_update_multi(uint *column_numb > > val.type= DYN_COL_UINT; > val.x.ulong_value= column_values[0]; > - if (dynamic_column_create(&str, column_numbers[0], &val)) > + if (mariadb_dyncol_create_many(&str, 1, column_numbers, &val, 1)) > goto err; > for (i= 1; i < all; i++) > { > val.type= (null_values[i] ? DYN_COL_NULL : DYN_COL_UINT); > val.x.ulong_value= column_values[i]; > - if (dynamic_column_update(&str, column_numbers[i], &val)) > + if (mariadb_dyncol_update_many(&str, 1, column_numbers +i, &val)) > goto err; > > /* check value(s) */ > for (j= i; j >= (i < only_add ? 0 : i); j--) > { > - if (dynamic_column_get(&str, column_numbers[j], &val)) > + if (mariadb_dyncol_get(&str, column_numbers[j], &val)) > goto err; > if (null_values[j]) > { > if (val.type != DYN_COL_NULL || > - dynamic_column_exists(&str, column_numbers[j]) == ER_DYNCOL_YES) > + mariadb_dyncol_exists(&str, column_numbers[j]) == ER_DYNCOL_YES) > goto err; > } > else > { > if (val.type != DYN_COL_UINT || > val.x.ulong_value != column_values[j] || > - dynamic_column_exists(&str, column_numbers[j]) == ER_DYNCOL_NO) > + mariadb_dyncol_exists(&str, column_numbers[j]) == ER_DYNCOL_NO) > goto err; > } > } > if (i < only_add) > { > - DYNAMIC_ARRAY num; > - if (dynamic_column_list(&str, &num)) > + uint elements, *num; > + if (mariadb_dyncol_list(&str, &elements, &num)) > + { > + my_free(num); > goto err; > + } > /* cross check arrays */ > - if ((int)num.elements != i + 1) > + if ((int)elements != i + 1) > { > - delete_dynamic(&num); > + my_free(num); > goto err; > } > for(j= 0; j { > int k; > for(k= 0; > - k - *dynamic_element(&num, k, uint*); > + k k++); > if (k >= i + 1) > { > - delete_dynamic(&num); > + my_free(num); > goto err; > } > for(k= 0; > - k - *dynamic_element(&num, j, uint*); > + k k++); > if (k >= i + 1) > { > - delete_dynamic(&num); > + my_free(num); > goto err; > } > } > - delete_dynamic(&num); > + my_free(num); > } > } > > @@ -563,32 +573,52 @@ void test_empty_string() > { > DYNAMIC_COLUMN_VALUE val, res; > DYNAMIC_COLUMN str; > - DYNAMIC_ARRAY array_of_uint; > + uint *array_of_uint; > + uint number_of_uint; > int rc; > + uint ids[1]= {1}; > + DYNAMIC_COLUMN_VALUE vals[0]; > /* empty string */ > bzero(&str, sizeof(str)); > > - rc= dynamic_column_get(&str, 1, &res); > + rc= mariadb_dyncol_get(&str, 1, &res); > ok( (rc == ER_DYNCOL_OK) && (res.type == DYN_COL_NULL), "%s", "empty get"); > > - rc= dynamic_column_delete(&str, 1); > - ok( (rc == ER_DYNCOL_OK), "%s", "empty delete"); > + vals[0].type= DYN_COL_NULL; > + rc= mariadb_dyncol_update_many(&str, 1, ids, vals); > + ok( (rc == ER_DYNCOL_OK) && (str.str == 0), "%s", "empty delete"); > > - rc= dynamic_column_exists(&str, 1); > + rc= mariadb_dyncol_exists(&str, 1); > ok( (rc == ER_DYNCOL_NO), "%s", "empty exists"); > > - rc= dynamic_column_list(&str, &array_of_uint); > - ok( (rc == ER_DYNCOL_OK) && (array_of_uint.elements == 0), > + rc= mariadb_dyncol_list(&str, &number_of_uint, &array_of_uint); > + ok( (rc == ER_DYNCOL_OK) && (number_of_uint == 0) && (str.str == 0), > "%s", "empty list"); > > val.type= DYN_COL_UINT; > val.x.ulong_value= 1212; > - rc= dynamic_column_update(&str, 1, &val); > + rc= mariadb_dyncol_update_many(&str, 1, ids, &val); > if (rc == ER_DYNCOL_OK) > - rc= dynamic_column_get(&str, 1, &res); > - ok( (rc == ER_DYNCOL_OK) && > + rc= mariadb_dyncol_get(&str, 1, &res); > + ok( (rc == ER_DYNCOL_OK) && (str.str != 0) && > (res.type == DYN_COL_UINT) && (res.x.ulong_value == val.x.ulong_value), > "%s", "empty update"); > + dynamic_column_column_free(&str); > +} > + > +void test_mdev_4994() > +{ > + DYNAMIC_COLUMN dyncol; > + LEX_STRING key= {0,0}; > + DYNAMIC_COLUMN_VALUE val; > + int rc; > + > + val.type= DYN_COL_NULL; > + > + dynamic_column_initialize(&dyncol); > + rc= mariadb_dyncol_create_many_named(&dyncol, 1, &key, &val, 0); /* crash */ > + ok( (rc == ER_DYNCOL_OK), "%s", "test_mdev_4994"); > + dynamic_column_column_free(&dyncol); > } > > > @@ -633,11 +663,11 @@ void test_update_many(uint *column_numbe > res[i].type= DYN_COL_UINT; > res[i].x.ulong_value= result_values[i]; > } > - if (dynamic_column_create_many(&str1, column_count, column_numbers, val)) > + if (mariadb_dyncol_create_many(&str1, column_count, column_numbers, val, 1)) > goto err; > - if (dynamic_column_update_many(&str1, update_count, update_numbers, upd)) > + if (mariadb_dyncol_update_many(&str1, update_count, update_numbers, upd)) > goto err; > - if (dynamic_column_create_many(&str2, result_count, result_numbers, res)) > + if (mariadb_dyncol_create_many(&str2, result_count, result_numbers, res, 1)) > goto err; > if (str1.length == str2.length && > memcmp(str1.str, str2.str, str1.length) ==0) > @@ -656,7 +686,7 @@ int main(int argc __attribute__((unused) > char *big_string= (char *)malloc(1024*1024); > > MY_INIT(argv[0]); > - plan(60); > + plan(61); > > if (!big_string) > exit(1); > @@ -664,21 +694,17 @@ int main(int argc __attribute__((unused) > big_string[i]= ('0' + (i % 10)); > test_value_single_null(); > test_value_single_uint(0, "0"); > - test_value_single_uint(ULL(0xffffffffffffffff), "0xffffffffffffffff"); > - test_value_single_uint(ULL(0xaaaaaaaaaaaaaaaa), "0xaaaaaaaaaaaaaaaa"); > - test_value_single_uint(ULL(0x5555555555555555), "0x5555555555555555"); > + test_value_single_uint(0xffffffffffffffffULL, "0xffffffffffffffff"); > + test_value_single_uint(0xaaaaaaaaaaaaaaaaULL, "0xaaaaaaaaaaaaaaaa"); > + test_value_single_uint(0x5555555555555555ULL, "0x5555555555555555"); > test_value_single_uint(27652, "27652"); > test_value_single_sint(0, "0"); > test_value_single_sint(1, "1"); > test_value_single_sint(-1, "-1"); > - test_value_single_sint(LL(0x7fffffffffffffff), > - "0x7fffffffffffffff"); > - test_value_single_sint(LL(0xaaaaaaaaaaaaaaaa), > - "0xaaaaaaaaaaaaaaaa"); > - test_value_single_sint(LL(0x5555555555555555), > - "0x5555555555555555"); > - test_value_single_sint(LL(0x8000000000000000), > - "0x8000000000000000"); > + test_value_single_sint(0x7fffffffffffffffLL, "0x7fffffffffffffff"); > + test_value_single_sint(0xaaaaaaaaaaaaaaaaLL, "0xaaaaaaaaaaaaaaaa"); > + test_value_single_sint(0x5555555555555555LL, "0x5555555555555555"); > + test_value_single_sint(0x8000000000000000LL, "0x8000000000000000"); > test_value_single_double(0.0, "0.0"); > test_value_single_double(1.0, "1.0"); > test_value_single_double(-1.0, "-1.0"); > @@ -735,7 +761,7 @@ int main(int argc __attribute__((unused) > } > { > uint column_numbers[]= {10,1,12,37,4,57,6,76,87}; > - test_value_multi(ULL(0xffffffffffffffff), LL(0x7fffffffffffffff), > + test_value_multi(0xffffffffffffffffULL, 0x7fffffffffffffffLL, > 99999999.999e120, "9999999999999999999999999999999", > big_string, 1024*1024, charset_list[0], > 9999, 12, 31, > @@ -791,5 +817,8 @@ int main(int argc __attribute__((unused) > update_numbers, update_values, update_nulls, 4, > result_numbers, result_values, 3); > } > + test_mdev_4994(); > + > + my_end(0); > return exit_status(); > } > > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits

1 1

[Maria-developers] Can there be a better storage engine API?
by MARK CALLAGHAN 13 Sep '13

13 Sep '13

The storage engine API has never been my favorite part of MySQL. I haven't written a storage engine but I used to maintain a custom storage engine and migrated that from 4.0 to 5.0 so I had some experience with the API. Is there a different API that would make it easier for innovation to come to MySQL/MariaDB? For example there is a lot going on in the write-optimized database space. Until recently we had nothing for MySQL open-source. Now we have open source TokuDB (great news). I haven't read their source but I have been told they use something that was derived from the BerkeleyDB API. Would the BerkeleyDB API make it easier to get new storage engines into MySQL? For example, we don't have an engine optimized for in-memory workloads. And while InnoDB will have great cache hit rates when all data is in-memory. It might write much more data to disk than required. Do we need a better API or is the current code good enough? I hope to hear from people who have done serious storage engine work. -- Mark Callaghan mdcallag(a)gmail.com

7 10

[Maria-developers] updating a string session variable from a storage engine
by Rich Prohaska 12 Sep '13

12 Sep '13

i have a MYSQL_THDVAR_STR session variable in my storage engine. i want to update its value from the storage engine. it was suggested that i use "THDVAR(x)=new_value" to do it. i noticed that update_func_str is NOT called, so i wonder about the contract. is the caller responsible for memory management? i used PLUGIN_VAR_MEMALLOC for the var def. thanks

3 2

[Maria-developers] Problems building debug version of MariaDB 5.5.32
by Weldon Whipple 11 Sep '13

11 Sep '13

Hello, I'm having problems building a Debug version of MariaDB 5.5.32. (It's a new problem, I think; I'm almost positive it was working about two weeks ago.) Here is what I'm doing to build MariaDB: $ cmake -DCMAKE_BUILD_TYPE=Debug . $ make package At 69% complete, I get about 14 lines like this: [ 69%] Building CXX object unittest/mysys/CMakeFiles/explain_filename-t.dir/explain_filename-t.cc.o Linking CXX executable explain_filename-t `.text._ZN7handler17ha_index_read_mapEPhPKhm16ha_rkey_function' referenced in section `.note.stapsdt' of ../../sql/libpartition.a(ha_partition.cc.o): defined in discarded section `.text._ZN7handler17ha_index_read_mapEPhPKhm16ha_rkey_function[handler::ha_index_read_map(unsigned char*, unsigned char const*, unsigned long, ha_rkey_function)]' of ../../sql/libpartition.a(ha_partition.cc.o) followed by: collect2: ld returned 1 exit status make[2]: *** [unittest/mysys/explain_filename-t] Error 1 make[1]: *** [unittest/mysys/CMakeFiles/explain_filename-t.dir/all] Error 2 make: *** [all] Error 2 Some comments: 1. I'm running on CentOS 6.4: $ uname -a Linux weldon 2.6.32-358.18.1.el6.x86_64 #1 SMP Wed Aug 28 17:19:38 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux 2. I've tried "cmake -DRPM=centos6 . " (without Debug specified) and it works just fine. 3. I found the line in (282:) in ./CMakeLists.txt that reads "ADD_SUBDIRECTORY(unittest/mysys)" and commented it out. Then the build continued until it was trying to link mysqld ... and failed with the similar errors at 99% completion. (I actually WANT mysqld, so I couldn't comment it out too :-)) 4. Something I read indicates that MariaDB requires a GCC build against recent binutils. yum seems to indicate that both GCC and binutils are up-to-date. 5. The cmake option I'm using is the one for 5.5+ as instructed at https://mariadb.com/kb/en/compiling-mariadb-for-debugging/ 6. Yes, I also tried the old ./BUILD/compile-pentium64-debug-max (but it failed similarly) I THINK it's probably an incompatible version of some build tool on my development box. Thanks in advance for any advice. Weldon Whipple

2 1

[Maria-developers] MDEV 5007 - help me! it's a old mdev closed as not a bug but don't solve the problem
by Roberto Spadim 10 Sep '13

10 Sep '13

Hi guys! I posted the MDEV-5007 and it's a old problem from MDEV-3795 (the same problem) It (MDEV-3795) was closed as a not a bug, but i think it's a bug, but i didn't explained well at the first time I changed from a mysql server to a mariadb server and the result order by isn't executed, i tried myisam,aria,innodb,memory engines and all of them return the same problem (order by not executed) On the wrong query i don't see the "using filesort" at explain, and at the right query (with right order by) it show "using filesort" Could anyone help me? or at least EXPLAIN why the order by shouldn't be executed on the first query, and maybe a workaround using sql_optimizer variable or something like it? it's a closed software and i need to contact developer to change the query, that's why i'm talking about the sql_optimizer variable instead of rewrite the query Thanks -- Roberto Spadim

2 3

[Maria-developers] Rollback causes assertion with Galera 10.0
by Jan Lindström 10 Sep '13

10 Sep '13

Hi, Background: I merged 10.0 to lp:~maria-captains/maria/maria-10.0-galera/, merge result is pushed to the tree. In testing I hit repeatable to following assertion #0 0x00007f1ff987f037 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56 #1 0x00007f1ff9882698 in __GI_abort () at abort.c:90 #2 0x00007f1ff9877e03 in __assert_fail_base (fmt=0x7f1ff99ce578 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=assertion@entry=0x109b4ff "empty()", file=file@entry=0x109b4d8 "/home/jan/mysql/galera-10.0/sql/log.cc", line=line@entry=294, function=function@entry=0x10a01e0 <binlog_cache_data::reset()::__PRETTY_FUNCTION__> "void binlog_cache_data::reset()") at assert.c:92 #3 0x00007f1ff9877eb2 in __GI___assert_fail (assertion=0x109b4ff "empty()", file=0x109b4d8 "/home/jan/mysql/galera-10.0/sql/log.cc", line=294, function=0x10a01e0 <binlog_cache_data::reset()::__PRETTY_FUNCTION__> "void binlog_cache_data::reset()") at assert.c:101 #4 0x00000000009497d4 in binlog_cache_data::reset (this=0x7f1f7403ad60) at /home/jan/mysql/galera-10.0/sql/log.cc:294 #5 0x0000000000949b6a in binlog_cache_mngr::reset (this=0x7f1f7403ab90, do_stmt=false, do_trx=true) at /home/jan/mysql/galera-10.0/sql/log.cc:462 #6 0x0000000000935a4c in binlog_truncate_trx_cache (thd=0x45b7d40, cache_mngr=0x7f1f7403ab90, all=true) at /home/jan/mysql/galera-10.0/sql/log.cc:1991 #7 0x0000000000936123 in binlog_rollback (hton=0x3891fb0, thd=0x45b7d40, all=true) at /home/jan/mysql/galera-10.0/sql/log.cc:2180 #8 0x000000000086b750 in ha_rollback_trans (thd=0x45b7d40, all=true) at /home/jan/mysql/galera-10.0/sql/handler.cc:1616 #9 0x00000000007a0c5f in trans_rollback (thd=0x45b7d40) at /home/jan/mysql/galera-10.0/sql/transaction.cc:321 #10 0x00000000006718b2 in mysql_execute_command (thd=0x45b7d40) at /home/jan/mysql/galera-10.0/sql/sql_parse.cc:4634 #11 0x0000000000677d52 in mysql_parse (thd=0x45b7d40, rawbuf=0x7f1f74005288 "ROLLBACK", length=8, parser_state=0x7f1fe83bb610) at /home/jan/mysql/galera-10.0/sql/sql_parse.cc:6816 #12 0x00000000006774d3 in wsrep_mysql_parse (thd=0x45b7d40, rawbuf=0x7f1f74005288 "ROLLBACK", length=8, parser_state=0x7f1fe83bb610) at /home/jan/mysql/galera-10.0/sql/sql_parse.cc:6647 #13 0x00000000006688cd in dispatch_command (command=COM_QUERY, thd=0x45b7d40, packet=0x44807c1 "ROLLBACK ", packet_length=9) at /home/jan/mysql/galera-10.0/sql/sql_parse.cc:1451 #14 0x00000000006678a3 in do_command (thd=0x45b7d40) at /home/jan/mysql/galera-10.0/sql/sql_parse.cc:1077 #15 0x000000000078ba4d in do_handle_one_connection (thd_arg=0x45b7d40) at /home/jan/mysql/galera-10.0/sql/sql_connect.cc:1404 #16 0x000000000078b78d in handle_one_connection (arg=0x45b7d40) at /home/jan/mysql/galera-10.0/sql/sql_connect.cc:1311 #17 0x0000000000b0c20a in pfs_spawn_thread (arg=0x45713a0) at /home/jan/mysql/galera-10.0/storage/perfschema/pfs.cc:1853 #18 0x00007f1ffa423f8e in start_thread (arg=0x7f1fe83bc700) at pthread_create.c:311 #19 0x00007f1ff9941e1d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:113 (gdb) We go from log.cc::binlog_rollback : else if (ending_trans(thd, all) || (!(thd->variables.option_bits & OPTION_KEEP_LOG) && (!stmt_has_updated_non_trans_table(thd) || WSREP_FORMAT(thd->variables.binlog_format) != BINLOG_FORMAT_STMT) && (!cache_mngr->trx_cache.changes_to_non_trans_temp_table() || WSREP_FORMAT(thd->variables.binlog_format) != BINLOG_FORMAT_MIXED))) error= binlog_truncate_trx_cache(thd, cache_mngr, all); end there to cache_mngr->reset(false, true); and there is DBUG_ASSERT(empty()); bool empty() const { return pending() == NULL && my_b_tell(&cache_log) == 0; } At empty() this is what does not hold : my_b_tell(&cache_log) == 0; // not sure what this means At test output there is at the end: # 2013-09-09T10:03:21 [6620] Query: UPDATE LOW_PRIORITY IGNORE `table100_int_autoinc` AS X SET `col_char_12` = 220 WHERE X . `col_char_12_key` IN ( 2427781120 , 1 , 'i' , 'm' , 'o' ) ORDER BY `col_char_12`,`col_char_12_key`,`col_int`,`col_int_key`,`pk` LIMIT 8 failed: 1213 Deadlock found when trying to get lock; try restarting transaction. Further errors of this kind will be suppressed. # 2013-09-09T10:04:14 [6620] Query: DELETE LOW_PRIORITY IGNORE FROM `table10_key_pk_parts_2_int_autoinc` WHERE `col_char_12_key` IN ( 4200660992 , 1646198784 , 3888709632 , 32 , 'c' ) ORDER BY `col_char_12`,`col_char_12_key`,`col_int`,`col_int_key`,`pk` LIMIT 1 failed: 1205 Lock wait timeout exceeded; try restarting transaction. Further errors of this kind will be suppressed. Unfortunately, general log contains so many statements from different connections that they are not really helpful for getting repeatable test case. R: Jan -- Jan Lindström Principal Engineer MariaDB | MaxScale | skype: jan_p_lindstrom www.skysql.com <http://www.skysql.com/> Twitter <http://twitter.com/skysql> Blog <http://www.skysql.com/blog/> Facebook <http://www.facebook.com/skysql> LinkedIn <http://www.linkedin.com/company/1214250> Google+ <https://plus.google.com/117544963211695643458/posts>

2 4

[Maria-developers] AIX "make fails" - related to: MDEV-4941
by Michael Felt 09 Sep '13

09 Sep '13

AIX make fails with syntax errors. One has been fixed, per https://mariadb.atlassian.net/browse/MDEV-4941. Rather than submit error by error I thought it would be more productive to provide the output from make -i. After trying, but not seeing it 'attach', I am going to try attaching the output of make -i here. regards, Michael

2 2

[Maria-developers] Some clarifications requested for C client
by Peter Laursen 09 Sep '13

09 Sep '13

This is probably for Georg Richter (and/or @Wlad). I have a few qeustion! 1) https://bazaar.launchpad.net/~maria-captains/mariadb-native-client/trunk/re… "Fix for CONC-44: LOAD DATA INFILE can't open utf16le encoded filenames" But we we can stil not specify UTF16LE as the CHARSET, I think? Say I have a file *named* "ÄÜÑБГД.csv". (both Western special characters and cyrillic characters in the file name). This file I will be able to read now (using revision 80), right? But if file *content* is this single line (one row, two columns) ÄÜÑ,БГД and if it is encoded as 'Windows Unicode'/utf16le I still will need to recode the file to utf8 before executing it with LOAD DATA (as UTF16LE still cannot be specified for CHARSET in LOAD DATA), right? Please confirm that I understand this! 2) https://bazaar.launchpad.net/~maria-captains/mariadb-native-client/trunk/re… "Added support for embedded (sqlite)". This happened by beginning of this year but I just noticed now. Now what can we actually do with SQLite and the MariaDB C-API and what we the purpose of this revision? Is it to make it easier to write client applications using SQLite for internal storage? Or to make the C client code 'a generic API' for connecting to both MySQL (of all flavors) and SQLite? Is the the code/files that that were added different from offical SQLite downloads from http://www.sqlite.org/? I am not able to find any documentation anywhere. Also not a blog or whatever either. There must be some reason to add sqlite3 code?! 3) This seems to be the latest revision: https://bazaar.launchpad.net/~maria-captains/mariadb-native-client/trunk/re… "Added dyamic column api; Added character set conversion" ('dyamic' is misspelled BTW!) Won't we need some documentation here? The claim that MariaDB's connector "has exactly the same API as the MySQL Connector/C for MySQL 5.5" ( https://kb.askmonty.org/en/mariadb-client-library-for-c/) cannot be true now as it adds funtionalities for managing MariaDB ' dymamic column waht oracle's API has not? The API from Oracle's library is now a *subset* or MariaDB API (or MariaDB's is a *superset* or Oracle). So much I can figure out, but should I need to code with it I would need description of functions' syntax, expected results and error listings. Also I am not able to figure out what the "character set conversion" referred to is doing and why it was added. 4) Why does the changelog here https://kb.askmonty.org/en/mariadb-client-library-for-c-100-changelog/ stop at revision 77? -- Peter Laursen -- Webyog

2 3

Re: [Maria-developers] Rev 3883: MDEV-4472 Audit-plugin. Server-related part of the task
by Sergei Golubchik 08 Sep '13

08 Sep '13

Hi, Holyfoot! Just one question, see below. Otherwise ok. On Sep 08, holyfoot(a)askmonty.org wrote: > revno: 3883 > revision-id: holyfoot(a)askmonty.org-20130908092209-xar471jadl7lmwzo > parent: sergii(a)pisem.net-20130907203634-htu89dqmwe5gqwy5 > committer: Alexey Botchkov <holyfoot(a)askmonty.org> > branch nick: 55-4472 > timestamp: Sun 2013-09-08 14:22:09 +0500 > message: > MDEV-4472 Audit-plugin. Server-related part of the task. > file_logger became the service. > Data like query_id now are sent to the audit plugin. > Fix for MDEV-4770 ported from 10.0. > Fix added for the read_maria_plugin_info(). > Log rotation can be disabled with 'set rotations=0'. > === renamed file 'plugin/sql_errlog/sql_logger.cc' => 'mysys/file_logger.c' > --- a/plugin/sql_errlog/sql_logger.cc 2013-06-22 12:02:03 +0000 > +++ b/mysys/file_logger.c 2013-09-08 09:22:09 +0000 > @@ -16,7 +16,7 @@ > > #include "my_global.h" > #include <my_sys.h> > -#include "service_logger.h" > +#include <mysql/service_logger.h> > #include <my_pthread.h> > > extern MYSQL_PLUGIN_IMPORT char *mysql_data_home; I believe we agreed to keep file_logger.c completely clean from dynamic-plugin-related code. Did you forget to remove the line above? Regards, Sergei

1 0

Re: [Maria-developers] MDEV-4472 Audit-plugin. Server-related part of the task
by Sergei Golubchik 08 Sep '13

08 Sep '13

Hi, Holyfoot! On Sep 05, holyfoot(a)askmonty.org wrote: > At file:///home/hf/wmar/55-4472/ > > === renamed file 'plugin/sql_errlog/sql_logger.cc' => 'mysys/file_logger.c' > --- a/plugin/sql_errlog/sql_logger.cc 2013-06-22 12:02:03 +0000 > +++ b/mysys/file_logger.c 2013-09-05 12:47:00 +0000 > @@ -16,10 +16,12 @@ > > #include "my_global.h" > #include <my_sys.h> > -#include "service_logger.h" > +#include <mysql/service_logger.h> > #include <my_pthread.h> > > +#ifndef _WIN32 > extern MYSQL_PLUGIN_IMPORT char *mysql_data_home; > +#endif /*!_WIN32*/ > > #ifdef HAVE_PSI_INTERFACE > /* These belong to the service initialization */ > @@ -51,6 +53,12 @@ LOGGER_HANDLE *logger_open(const char *p > unsigned int rotations) > { > LOGGER_HANDLE new_log, *l_perm; > + //char *local_mysql_data_home; > +//#ifdef _WIN32 > + //local_mysql_data_home= (char *)GetProcAddress(0, "mysql_data_home"); > +//#else > + //local_mysql_data_home= mysql_data_home; > +//#endif /*_WIN32*/ I think you can completely remove this and the #ifndef above. For your MySQL plugin you can simply #include this in the plugin body like extern char *mysql_data_home; #include "file_logger.c" and later in your plugin_init function: #ifdef _WIN32 mysql_data_home= (char *)GetProcAddress(0, "mysql_data_home"); #endif This way you will keep file_logger.c completely free of the dynamic plugin hacks. > /* > I don't think we ever need more rotations, > @@ -168,6 +176,29 @@ int logger_vprintf(LOGGER_HANDLE *log, c > int logger_rotate(LOGGER_HANDLE *log) > { > int result; I believe you've added recently a test to disable rotation if number of rotations is 0? Regards, Sergei

2 1

[Maria-developers] first contribution
by AugustQ 06 Sep '13

06 Sep '13

Hi, after some struggling I got this message from Bazaar: Created new stacked branch referring to /+branch-id/633549. I hope everything worked fine. If not let me know so that I can correct it. I created a branch to 10.0.4 of MariaDB and named it: contrib_AQ_01. I've uploaded this some minutes before. For some time I played with the source of MySQL 5.5.8. I've taken some of my modifications to MariaDB 10.0.4 and want to give them to you for further discussions. To say it first: this is not production-ready-code, it's a proof of a concept. My modifications are made for the MyIsam-engine in the handling of a full table-scan with a table of dynamic sized records. The reason for uploading this is: during my tests I executed statements like SELECT * FROM table INTO OUTFILE 'x.x'; With my test-data this took about 20 seconds with the original code and 12 seconds with my modifications. I've tested my code with a single table which contains 10 Mio records, a total size of 1GB. As i've written before: it's a proof of concept. I've not implemented all the possible cases. I wanted to know if a full implementation is a complete waste of time or if it is a bit useful (in this case I will do it). Some more details: in mi_dynrec.c there is a funtion _mi_get_block_info() and i've added a funtion AQ_mi_get_block_info(), these 2 functions do the same thing but in a different way. In my version of this function I've implemented only those case that I needed for my tests. Because I started with examining the source of MySQL I modified the code in the MyIsam-engine and applied these to the MyIsam-engine in MariaDB. For staying compatible with MySQL I think it's better to use these modifications in the Maria-engine of MariaDB, but only if you agree with my approach. This is what I modified: in myisamdef.h I added: #define USE_CODE_OF_AQ converting this line in a comment allowed me to use the old code, a define with this identifier activates my code after a recompile. This allowed me to do some comparisons. You will find my modifications in myisamdef.h, mi_dynrec.c and mi_extra.c if you look for this identifier. Please look at my approach. If you agree I will continue with this and check all the cases which i didn't touch for now. thank you AugustQ

1 0

Re: [Maria-developers] MDEV-4978 - Server cursor is broken with blobs in the select list
by Sergei Golubchik 06 Sep '13

06 Sep '13

Hi, Sergey! On Sep 06, Sergey Vojtovich wrote: > revno: 3878 > revision-id: svoj(a)mariadb.org-20130906111656-oy2c3pwy2o1q0zqn > parent: wlad(a)montyprogram.com-20130903204512-hgwu77prle0lw7a3 > committer: Sergey Vojtovich <svoj(a)mariadb.org> > branch nick: 5.5-mdev4978 > timestamp: Fri 2013-09-06 15:16:56 +0400 > message: > MDEV-4978 - Server cursor is broken with blobs in the select list, > ORDER BY does not work > > Use "dynamic" row format (instead of "block") for MARIA internal > temporary tables created for cursors. > With "block" row format MARIA may shuffle rows, with "dynamic" row > format records are inserted sequentially (there are no gaps in data > file while we fill temporary tables). > This is needed to preserve row order when scanning materialized cursors. > === modified file 'sql/sql_select.cc' > --- a/sql/sql_select.cc 2013-09-03 14:41:07 +0000 > +++ b/sql/sql_select.cc 2013-09-06 11:16:56 +0000 > @@ -15373,7 +15374,8 @@ create_tmp_table(THD *thd, TMP_TABLE_PAR > if (thd->is_fatal_error) // If end of memory > goto err; /* purecov: inspected */ > share->db_record_offset= 1; > - table->used_for_duplicate_elimination= (param->sum_func_count == 0 && > + table->used_for_duplicate_elimination= keep_row_order || > + (param->sum_func_count == 0 && > (table->group || table->distinct)); That's very confusing. Why keep_row_order means that the table is "used for duplicate elimination"? I had to check the code to see that used_for_duplicate_elimination affects the row format in Aria. This is not obvious too. And it forces the row format to be DYNAMIC for a very different reason, not for preserving row order. Perhaps it'd be better to rename used_for_duplicate_elimination to "force_dynamic_Aria_row_format" ? A prettier solution would be to introduce another flag: table->keep_row_order. Regards, Sergei

2 2

[Maria-developers] Bazaar-problem
by AugustQ 06 Sep '13

06 Sep '13

Hi, I'm struggling with Bazaar. Maybe you can help me. I've made some modifications to the source-code and committed these to the local repository. Next I tried to upload them via push. This is what I receive: Cannot lock LockDir(chroot-70341520:///% 2Bbranch/maria/.bzr/branch/lock): Transport operation not possible: readonly transport As I'm using Eclipse I found this within Eclipse on the console: Error while executing push Cannot lock LockDir(chroot-70341520:///% 2Bbranch/maria/.bzr/branch/lock): Transport operation not possible: readonly transport ------ XmlRpcClient Executing: push "bzr+ssh://bazaar.launchpad.net/+branch/maria/" Can you please give me a tip. Thanks AugustQ

5 9

[Maria-developers] Incorrect format description event skipping in queue_event()
by Pavel Ivanov 04 Sep '13

04 Sep '13

Kristian, You have the following comment in the queue_event() in sql/slave.cc: /* Do not queue any format description event that we receive after a reconnect where we are skipping over a partial event group received before the reconnect. (If we queued such an event, and it was the first format_description event after master restart, the slave SQL thread would think that the partial event group before it in the relay log was from a previous master crash and should be rolled back). */ I don't understand which failure scenario you are talking about here and I claim that this bypassing of queuing into relay log is incorrect. My reasoning is: relay log can always be rotated in the middle of event group. During rotation when new relay log file is created format description event is always written at the beginning of the file. So in this case SQL thread will see format description event in the middle of event group and according to my testing it doesn't rollback the active transaction because of that. So when do you think it will rollback? Now, you may say that there's no problem in skipping the format description event because everything works. But it's only for now. When IO thread is reconnecting it rotates relay log and as I said it writes format description event at the beginning of the new file. But it writes an event that it created itself, i.e. not the one that master have sent. And as format description event from master is not written into relay log SQL thread from this point on starts to use format description generated by slave which may be different from the one generated by master. It may lead to a broken replication and SQL thread may not recognize events in the relay log if difference between master's and slave's format descriptions is significant. Another somewhat related question: Gtid_log_event::peek() (as well as Gtid_log_event constructor from const char* buf) is implemented with assumption that Format_description_log_event::common_header_len is always equal to LOG_EVENT_HEADER_LEN. While currently it's true I believe common_header_len can be increased in future MariaDB versions. And when it happens old slave won't be able to replicate from a new master. I'd think this is a bug. What do you think? Pavel

2 10

Re: [Maria-developers] Rev 3876: MDEV-4472 Audit-plugin. Server-related part of the task
by Sergei Golubchik 04 Sep '13

04 Sep '13

Hi, Holyfoot! Mostly ok. There are few comments, see below: On Sep 03, holyfoot(a)askmonty.org wrote: > ------------------------------------------------------------ > revno: 3876 > revision-id: holyfoot(a)askmonty.org-20130903082521-xpia2wond6oggpsk > parent: igor(a)askmonty.org-20130831163309-vqtls4oubhvz9zcz > committer: Alexey Botchkov <holyfoot(a)askmonty.org> > branch nick: 55-4472 > timestamp: Tue 2013-09-03 13:25:21 +0500 > message: > MDEV-4472 Audit-plugin. Server-related part of the task. > file_logger became the service. > Data like query_id now are sent to the audit plugin. > Fix for MDEV-4770 ported from 10.0. > Fix added for the read_maria_plugin_info(). > === modified file 'include/mysql/plugin_audit.h' > --- a/include/mysql/plugin_audit.h 2013-04-19 10:50:16 +0000 > +++ b/include/mysql/plugin_audit.h 2013-09-03 08:25:21 +0000 > @@ -25,7 +25,7 @@ > > #define MYSQL_AUDIT_CLASS_MASK_SIZE 1 > > -#define MYSQL_AUDIT_INTERFACE_VERSION 0x0301 > +#define MYSQL_AUDIT_INTERFACE_VERSION 0x0302 > > > /************************************************************************* > @@ -59,6 +59,10 @@ struct mysql_event_general > struct charset_info_st *general_charset; > unsigned long long general_time; > unsigned long long general_rows; > + /* Added for 0x302 */ A bit confusing. Better "Added in version 0x0302" > + long long query_id; should it rather be unsigned? > + const char *database; > + unsigned int database_length; > }; > > > @@ -140,6 +144,8 @@ struct mysql_event_table > unsigned int new_database_length; > const char *new_table; > unsigned int new_table_length; > + /* Added for 0x302 */ > + long long query_id; > }; > > /************************************************************************* > > === renamed file 'plugin/sql_errlog/sql_logger.cc' => 'mysys/file_logger.c' > --- a/plugin/sql_errlog/sql_logger.cc 2013-06-22 12:02:03 +0000 > +++ b/mysys/file_logger.c 2013-09-03 08:25:21 +0000 > @@ -51,6 +53,12 @@ LOGGER_HANDLE *logger_open(const char *p > unsigned int rotations) > { > LOGGER_HANDLE new_log, *l_perm; > + char *local_mysql_data_home; > +#ifdef _WIN32 > + local_mysql_data_home= (char *)GetProcAddress(0, "mysql_data_home"); > +#else > + local_mysql_data_home= mysql_data_home; > +#endif /*_WIN32*/ You shouldn't need it, file_logger.c is part of the server, not a dynamically loaded plugin. > > /* > I don't think we ever need more rotations, > === modified file 'sql/sql_audit.h' > --- a/sql/sql_audit.h 2013-04-19 10:50:16 +0000 > +++ b/sql/sql_audit.h 2013-09-03 08:25:21 +0000 > @@ -93,7 +93,8 @@ void mysql_audit_general_log(THD *thd, t > > mysql_audit_notify(thd, MYSQL_AUDIT_GENERAL_CLASS, MYSQL_AUDIT_GENERAL_LOG, > 0, time, user, userlen, cmd, cmdlen, > - query, querylen, clientcs, 0); > + query, querylen, clientcs, (ha_rows) 0, > + thd->db, thd->db_length); no query_id? > } > } > > @@ -139,7 +140,8 @@ void mysql_audit_general(THD *thd, uint > > mysql_audit_notify(thd, MYSQL_AUDIT_GENERAL_CLASS, event_subtype, > error_code, time, user, userlen, msg, msglen, > - query.str(), query.length(), query.charset(), rows); > + query.str(), query.length(), query.charset(), rows, > + thd->db, thd->db_length); no query_id? > } > } > > === modified file 'sql/sql_plugin.cc' > --- a/sql/sql_plugin.cc 2013-06-13 18:19:11 +0000 > +++ b/sql/sql_plugin.cc 2013-09-03 08:25:21 +0000 > @@ -2645,13 +2647,16 @@ static void update_func_longlong(THD *th > static void update_func_str(THD *thd, struct st_mysql_sys_var *var, > void *tgt, const void *save) > { > - char *old= *(char **) tgt; > - *(char **)tgt= *(char **) save; > + char *value= *(char**) save; > if (var->flags & PLUGIN_VAR_MEMALLOC) > { > - *(char **)tgt= my_strdup(*(char **) save, MYF(0)); > + char *old= *(char**) tgt; > + if (value) > + *(char**) tgt= my_strdup(value, MYF(0)); add else *(char**) tgt= 0; > my_free(old); > } > + else > + *(char**) tgt= value; > } > Regards, Sergei

2 3

[Maria-developers] RFC - CREATE PROCEDURE clauses (question about docs from mariadb-discuss)
by Roberto Spadim 02 Sep '13

02 Sep '13

Hi mariadb-devel guys, could anyone help here in a documentation problem? (maybe not a problem...) *Please answer the BOLD questions* We are talking about CREATE PROCEDURE , about DETERMINISTIC / NO SQL http://dev.mysql.com/doc/refman/5.6/en/create-procedure.html *characteristic*: COMMENT '*string*' | LANGUAGE SQL | *[NOT] DETERMINISTIC* | { CONTAINS SQL | *NO SQL* | READS SQL DATA | MODIFIES SQL DATA } | SQL SECURITY { DEFINER | INVOKER } *--------* *from mariadb-discuss:* *--------* Hi Federico, sorry i read some points and start writing, i will read and write after... i will answer with my experience as an user, not as a mariadb/mysql developer... 2013/9/2 Federico Razzoli <federico_raz(a)yahoo.it> > Hi Roberto > I'll try to be more clear, please read carefully before answering > ok I know what a deterministic expression is - but *what kinds of output must > be deterministic for a procedure to be defined as DETERMINISTIC?* > hummm i don't know, but for procedure, there isn't a deterministic output right? just for funcitons... maybe it's a parser problem, allow deterministic keyword for procedures (must check) i'm thinking about, *why we use deterministic procs, and non deterministic procs?* *this change something important in mysql server? replication, query cache? other performace considerations?* > There are many types of output for a proc: > * OUT params > * SELECTs > * writes on tables > * SET @var > hum.... from what i remember procedure ~= function without return, something like a "void FUNCTION( any parameters )" function ~= a procedure that return values, in other words it must be different from void... that's why i think it's a problem at parser allowing deterministic keyword at procedure, but must check... i don't know if this keyword change something in mysql/mariadb code > For a deterministic proc, do all of these need to be deterministic? > well, for me, procs are always non deterministics since it don't change query cache invalidation and replication (must be done in binary i think) > * I guess the answer isn't easy. > * *yeah, i see, we need a simple answer not a simplified answer* > -- > For "NO SQL" a complete list of SQL statements would be needed. If we use > this as a reference: > http://dev.mysql.com/doc/refman/5.6/en/sql-syntax.html > , even RETURN seems to be a SQL statement, yes, maybe the point isn't the SQL statements list, but other functions that mysql/mariadb server do? i don't know... NO SQL maybe is different from NOSQL (memcache,mongo,etc...) since nosql is newer than mysql procedures the point is *what's NO SQL in CREATE PROCEDURE ?* but of course it can't be true, because it's impossible to write a func > without RETURN. > yeah, maybe we have a problem at docs... let's see if anyone else know about this... maybe we need help from mariadb-devel list, i will include in this email... > > Greetings > Federico > bye =) thanks guys -- Roberto Spadim

1 0

Re: [Maria-developers] FWD: Status on MDEV-4506, parallel replication
by Kristian Nielsen 02 Sep '13

02 Sep '13

Michael Widenius <monty(a)askmonty.org> writes: >> On the master, I implemented --binlog-commit-wait-count=N and >> --binlog-commit-wait-usec=T. A transaction will wait at most T microseconds >> for at least N transactions to queue up and be ready for group commit. This >> allows to deliberately delay transactions on the master in order to get bigger >> group commits and thus better opportunity for parallel execution (and again it >> makes testing easier). > > Do you think the above helps in any real world case, except testing? > > Assuming we are have N=4 and T=2 > > - We have 3 threads ready to commit > - We wait 2 milliseconds and get 2 more threads. > - We commit now 5 threads. During this time you get 3 other threads > waiting to commit. > - We wait again... > > In a scenario without waiting: > > - We have 3 threads ready to commit > - We commit 3 threads. During this time we get 5 more threads waiting > to commit. > - We commit 5 threads. > > In other words, as the group commit will anyway take up 50 > microseconds on a hard disk, and thus automaticly group things > together for the next commit, why do we need ever to wait more? > > I agree that with --binlog-commit-wait-count you may get less sync > calls, but at an expense that a lot of threads took up to T > microseconds longer to execute. > > The worst case scenario is when you have only one user doing a lot of > inserts with auto-commit. In this case using wait will slow down the > server with T microseconds for every query. > > Have you been able to run any kind of benchmark where using > binglog-commit-wait-count will give more performance? > > I agree that the options are good for testing. The main question I > have is if we want to have the variables in the production server and > how we should document when and how a user should use the variables. I agree that with respect to group commit, the --binlog-commit-wait-count in many cases will not improve performance (that is why I did not implement it earlier). However, for parallel replication things are a bit different. Suppose that the application is doing C transaction commits per second, and that the disk system is capable of F binlog fsyncs per second. Now, if C is significantly bigger than F, then things are as you describe. Generally several transactions will queue up while the previous group commit runs, and there will be sufficient parallelism without using --bicommit_wait-*. This will be typical for eg. a simple harddisk-based system (F=40 commits/second perhaps) where all data is cached in the InnoDB buffer pool (eg. C>500 transactions/second). On the other hand, if C is smaller than F (or of similar magnitide), then usually only few or no new transactions will have time to queue up while the previous transaction is committing. So there will not be much parallelism for parallel replication to exploit without using --binlog-commit-wait-*. This will be typical for eg. a good-quality server with battery-backup RAID controller (F > 1000 commits/second) where data is too big to fit in buffer pool and every update requires disk access to complete (C < 500 transactions/second for example). And in fact, it is the second case, where random I/O is the bottleneck and multiple disk spindels are needed to improve I/O throughput, that single-threaded slave hurts the most, and where increasing --binlog-commit-wait-usec can be used with least penalty. I agree we need to document clearly the risk that --binlog-commit-wait-* will decrease performance on the master. Basically, the user can look at the ratio of Binlog_commits and Binlog_group_commits to check if enough transactions are part of each group commit for parallel replication to be effective. >> 1. The existing code is not thread-safe for class Relay_log_info. This class >> contains a bunch of stuff that is specific to executed transactions, not >> related to relay-log at all. This needs to be moved to the new struct >> rpl_group_info I introduced, and all code updated to pass around a pointer to >> that struct instead. There may also be a need to add additional locking on >> Relay_log_info, existing code needs review for this. > > I could take a look working on the above tomorrow and next week. Ok, great. Ping me when you can so we can coordinate, I may have some partial patches for this lying around. - Kristian.

1 0