- developers - lists.mariadb.org

[Maria-developers] Comments for parallel replication patch (MDEV-4506)
by Kristian Nielsen 07 Oct '13

07 Oct '13

Hi Monty, I read through your latest patch for the MDEV-4506 task, parallel replication. Just to re-cap, this is a work-in-progress patch, so these are just some comments along the way that I hope will be useful. > -static bool sql_slave_killed(THD* thd, Relay_log_info* rli) > +static bool sql_slave_killed(rpl_group_info *rgi) > -static Log_event* next_event(Relay_log_info* rli) > +static Log_event* next_event(rpl_group_info *rgi) This does not look right to me. Can you explain the reason to use rgi here instead of rli? To me, rgi is about what happens in parallel in each worker thread. But these functions are about what happens in the driver SQL thread, fetching each event from the relay log and checking if the driver SQL thread is done and can exit. It looks from the patch like rgi is always passed the serial_rgi. I think it would be better if it did not use rgi at all, we could pass in any extra information needed directly. Note that serial_rgi should _only_ be used in the case where parallel replication is not activated and actual event execution happens inside the main driver SQL thread. Or did I mis something? > Log_event::enum_skip_reason > -Gtid_log_event::do_shall_skip(Relay_log_info *rli) > +Gtid_log_event::do_shall_skip(rpl_group_info *rgi) > { > + Relay_log_info *rli= rgi->rli; > /* > An event skipped due to @@skip_replication must not be counted towards the > number of events to be skipped due to @@sql_slave_skip_counter. > @@ -6315,10 +6319,13 @@ Gtid_log_event::do_shall_skip(Relay_log_ > if (rli->slave_skip_counter > 0) > { > if (!(flags2 & FL_STANDALONE)) > + { > thd->variables.option_bits|= OPTION_BEGIN; > - return Log_event::continue_group(rli); > + rgi->rli->set_flag(Relay_log_info::IN_TRANSACTION); > + } > + return Log_event::continue_group(rgi); > } > - return Log_event::do_shall_skip(rli); > + return Log_event::do_shall_skip(rgi); > } This looks to me like it cannot work. shall_skip() is called from apply_event_and_update_pos() which runs in each worker thread, in parallel. But rli->slave_skip_counter needs to be checked from the driver SQL thread, in the order that events are read from the relay log. Otherwise, if slave_skip_counter is 1 and we have transactions T1 and T2, T2 might run first and we would end up skipping T2 instead of T1 which is wrong. Besides, it seems better to in as many cases as possible to skip events by not queuing them for a worker thread in the first place. I am wondering if this change came as part of a mechanical change from rli to rgi when is_in_group() was moved from rli to rgi? That is why I had this ToDo comment: > - - 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. I think what needs to be done here is that event skip handling needs to be moved, so that it is done in exec_relay_log_event() rather than in apply_event_and_update_pos(). Then it can be done always in the driver SQL thread, both in the parallel and in the non-parallel case. It is possible that we then may need the flag associated with is_in_group() both in rli and in rgi, we will see when we do the change. What do you think? > @@ -71,6 +66,7 @@ rpt_handle_event(rpl_parallel_thread::qu > > /* ToDo: Access to thd, and what about rli, split out a parallel part? */ > mysql_mutex_lock(&rli->data_lock); > + qev->ev->thd= thd; Indeed, this seems to be needed. Wow. I'm surprised that I have been able to get anything to work when this was missing?!? Or is it because in most cases, THD is passed into Log_event:: functions directly, rather than using Log_event::thd ? > @@ -1094,9 +1090,9 @@ bool Relay_log_info::is_until_satisfied( > !replicate_same_server_id) > DBUG_RETURN(FALSE); > log_name= group_master_log_name; > - log_pos= (!ev)? group_master_log_pos : > - ((thd->variables.option_bits & OPTION_BEGIN || !ev->log_pos) ? > - group_master_log_pos : ev->log_pos - ev->data_written); > + log_pos= ((!ev)? group_master_log_pos : > + (get_flag(IN_TRANSACTION) || !ev->log_pos) ? > + group_master_log_pos : ev->log_pos - ev->data_written); Yes, it is clear that we need something like this. You told me on IRC that you still needed to work on this. Eg. I see you started to add a part_of_next_statement() but did not implement this. I also am worried how to make sure that we do not miss any cases here, and agree it needs to be checked. I will just mention that for GTID I already added Log_event::is_part_of_group(), which seems related to your part_of_next_statement()? Maybe they could use the same mechanism. One outstanding issue with is_part_of_group is for row-based replication with multi-table update statements. In such multi-table updates there will be two (or more) Rows_log_event events, and there is a flag to tell which is the last one. They need to be considered part of the same statement. I think this is not yet correctly handled in the GTID code (MDEV-4985). Probably this case also needs to be handled correctly here? > - if ((rgi->thd->variables.option_bits & OPTION_BEGIN) && opt_using_transactions) > + if ((rgi->rli->get_flag(IN_TRANSACTION)) && opt_using_transactions) There is more to be done here than this. Because this runs in each parallel worker thread, in parallel. Wouldn't it be more correct actually to use rgi->thd here, actually? But in any case, we need to think about how to handle old-style replication position updates. Since we already ensure that transactions commit in-order, maybe it is ok to do the updates to position in parallel, as long as we do not decrement the position. So if we have transactions T1 and T2 running in parallel, T1 will commit before T2, but T2 may then race ahead and update replication position before T1. That is ok (both T1 and T2 are committed), but then we need in T1 to check and see that position is already ahead, so we do not set it back to T1 (that could cause duplication of T2 in case of slave restart). And in the GTID case, where the replication position is not used in case of slave restart, then it would be best not to update the master.info and relay-log.info file after every transaction (to save the cost of write + possibly fsync). But it would be best to still update it in-memory so that SHOW SLAVE STATUS shows something meaningful. > + Flags for the state of reading the relay log. Note that these are > + bits "Note that these are bit masks" is less ambiguous. > + This is updated when a temporary table is created or drooped by > + a replication thread. "drooped" -> "dropped". > + /** > + Is the replication inside a group? > + > + Replication is inside a group if either: > + - The IN_TRANSACTION flag is set, meaning we're inside a transaction > + - The IN_STMT flag is set, meaning we're inside a statement > + > + @retval true Replication thread is currently inside a group > + @retval false Replication thread is currently not inside a group > + */ > + bool is_in_group() const { > + return (m_flags & (IN_STMT | IN_TRANSACTION)); > + } Do I understand correctly that this refers to the last event fetched from the relay log, not the last event executed? These are different in the case of parallel replication. I think the comment should be clarified to explicitly say one way or the other. Hm, I see now that this is not your comment (you just moved it), so it is ok, but it would still be good if we could clarify the comment. > -static bool sql_slave_killed(THD* thd, Relay_log_info* rli) > +static bool sql_slave_killed(rpl_group_info *rgi) > { > /* > - The transaction should always be binlogged if OPTION_KEEP_LOG is set > - (it implies that something can not be rolled back). And such case > - should be regarded similarly as modifing a non-transactional table > - because retrying of the transaction will lead to an error or inconsistency > - as well. > - Example: OPTION_KEEP_LOG is set if a temporary table is created or dropped. > + The transaction should always be binlogged if OPTION_KEEP_LOG is > + set (it implies that something can not be rolled back). And such > + case should be regarded similarly as modifing a > + non-transactional table because retrying of the transaction will > + lead to an error or inconsistency as well. > + > + Example: OPTION_KEEP_LOG is set if a temporary table is created > + or dropped. > */ > if ((thd->transaction.all.modified_non_trans_table || > (thd->variables.option_bits & OPTION_KEEP_LOG)) So checking thd->transaction is not meaningful in case of parallel replication. However, this is related to stopping the slave, which as we discussed before is still work-in-progress. I think we can keep this code to handle the non-parallel case, and then add a similar check in the worker threads (maybe in a new function sql_worker_killed()). And maybe then add a check here that all worker threads have stopped. I'm looking into this. So this should be ok for now, just wanted to mention it. - Kristian.

1 0

Re: [Maria-developers] Rev 3847: Client attributes
by Sergei Golubchik 04 Oct '13

04 Oct '13

Hi, Sanja! On Oct 02, sanja(a)askmonty.org wrote: > === modified file 'client/mysqltest.cc' > --- a/client/mysqltest.cc 2013-07-21 14:39:19 +0000 > +++ b/client/mysqltest.cc 2013-10-02 13:54:55 +0000 > @@ -5648,6 +5648,10 @@ void safe_connect(MYSQL* mysql, const ch > verbose_msg("Connecting to server %s:%d (socket %s) as '%s'" > ", connection '%s', attempt %d ...", > host, port, sock, user, name, failed_attempts); > + > + mysql_options(mysql, MYSQL_OPT_CONNECT_ATTR_RESET, 0); > + mysql_options4(mysql, MYSQL_OPT_CONNECT_ATTR_ADD, > + "program_name", "mysqltest"); Did you test connecting without connection attributes? e.g. 5.5 client and 10.0 server? and mysql-5.5 client? And mysql-5.6 client? please do, at least manually > while(!mysql_real_connect(mysql, host,user, pass, db, port, sock, > CLIENT_MULTI_STATEMENTS | CLIENT_REMEMBER_OPTIONS)) > { > > === modified file 'include/mysql.h' > --- a/include/mysql.h 2013-07-18 14:46:57 +0000 > +++ b/include/mysql.h 2013-10-02 13:54:55 +0000 > @@ -557,6 +557,8 @@ int STDCALL mysql_list_proce > int status); > int STDCALL mysql_options(MYSQL *mysql,enum mysql_option option, > const void *arg); > +int STDCALL mysql_options4(MYSQL *mysql,enum mysql_option option, > + const void *arg1, const void *arg2); This is pretty silly, I'd rather have mysql_optionv(MYSQL *mysql, enum mysql_option option, ...) or even (redefining the existing function): mysql_option(MYSQL *mysql, enum mysql_option option, ...) but we're copying MySQL here, so we don't have much of a choice > void STDCALL mysql_free_result(MYSQL_RES *result); > int STDCALL mysql_free_result_start(MYSQL_RES *result); > int STDCALL mysql_free_result_cont(MYSQL_RES *result, int status); > > === modified file 'sql/sql_acl.cc' > --- a/sql/sql_acl.cc 2013-09-21 08:14:42 +0000 > +++ b/sql/sql_acl.cc 2013-10-02 13:54:55 +0000 > @@ -8570,7 +8628,13 @@ static ulong parse_client_handshake_pack > mpvio->acl_user->plugin= old_password_plugin_name; > } > } > - > + > + if (db && Why do you check for db being not NULL? > + (thd->client_capabilities & CLIENT_CONNECT_ATTRS) && > + read_client_connect_attrs(&next_field, ((char *)net->read_pos) + pkt_len, > + mpvio->thd->charset())) > + return packet_error; > + > /* > if the acl_user needs a different plugin to authenticate > (specified in GRANT ... AUTHENTICATED VIA plugin_name ..) > > === modified file 'sql/sys_vars.cc' > --- a/sql/sys_vars.cc 2013-09-21 08:14:42 +0000 > +++ b/sql/sys_vars.cc 2013-10-02 13:54:55 +0000 > @@ -71,6 +71,7 @@ > #define export /* not static */ > > #ifdef WITH_PERFSCHEMA_STORAGE_ENGINE > +#ifndef EMBEDDED_LIBRARY Why is this? > > static Sys_var_mybool Sys_pfs_enabled( > "performance_schema", Regards, Sergei

2 3

[Maria-developers] Adding versions to documentation
by Daniel Bartholomew 03 Oct '13

03 Oct '13

All, Whenever we document new features in the Knowledge Base, one thing we need to do is say when it was (or will be) introduced. For example, if I'm documenting a feature that was introduced in MariaDB 10.0.4, I would add something similar to the following to the top of the page: <<product mariadb from=10.0.4>> This feature was introduced in MariaDB 10.0.4 <</product>> If you're in a hurry and can't remember the <<product>> macro syntax, just use a sentence. We'll wrap it in a product macro later. Also, if you come across an article that doesn't have this information, please add it if you can, or send me an email so that I know it's missing. Thanks! -- Daniel Bartholomew, Sr Tech Writer / Sys Admin MariaDB

1 0

[Maria-developers] [Gsoc]Regexp enhancements License
by Sudheera Palihakkara 03 Oct '13

03 Oct '13

Hi all, This is to inform you that the code that I created for the task "Regexp enhancements" during "Google Summer Of Code 2013" is available to to MariaDB Foundation under terms of the New BSD License. thank you. -- *Sudheera Palihakkara.* Undergraduate Department of *Computer Science and Engineering, *Faculty of Engineering, *University of Moratuwa*, Sri Lanka.

1 0

[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] Galera and aria
by Roberto Spadim 01 Oct '13

01 Oct '13

Hi again guys! =) I have a doubt, i was using a diff -u galera-5.33a mariadb-5.33a | grep -i "myisam" and found some lines... well could we add the aria storage to 'work' like myisam with galera cluster? i don't know if it's the right way (changing "== MYISAM_ENGINE" to something like (== myisam_engine OR == maria_engine)) if yes... i could make this patch and send? should i create a mdev to this task (allow aria engine with galera cluster)? today it's not a trasational engine, but it's better than myisam with the crash safe feature that's all :) thanks guys -- Roberto Spadim

2 4

[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