- developers - lists.mariadb.org

[Maria-developers] Error building mariadb-10.0 with Clang 3.8: Unqualified lookup in templates
by Luke Benes 13 Oct '16

13 Oct '16

Building mariadb-10.0 with Clang results in: graph_concepts.hpp:93:17: error: call to function 'out_edges' that is neither visible in the template definition nor found by argument-dependent lookup Full log: http://clang.debian.net/logs/2016-08-30/mariadb-10.0_10.0.26-3_unstable_cla…

2 1

Re: [Maria-developers] [MariaDB/server] Add new scheduling algorithm for reducing tail latencies (#245)
by Kristian Nielsen 12 Oct '16

12 Oct '16

Jiamin Huang <notifications(a)github.com> writes: > This branch introduces a new scheduling algorithm > (Variance-Aware-Transaction-Scheduling, VATS) for the record lock manager > of InnoDB and XtraDB. Instead of using First-Come-First-Served (FCFS), the > newly introduced algorithm uses an Eldest-Transaction-First (ETF) > heuristic, which prefers older transactions over new ones. It could be interesting to extend this to also understand commit order for in-order parallel replication. For in-order parallel replication, the commit order of transactions is fixed from the start. Suppose transactions Tm and Tn are both requesting the same record lock, and that Tm goes before Tn. If Tn gets the lock, it will be immediately killed since Tm must be allowed to go first. But there is no guarantee that Tm will be the older transaction. So prefering Tm over Tn here based on the commit order can potentially avoid some transaction rollback/retry. The function thd_deadlock_victim_preference() seems appropriate for overriding the age-of-transaction preference. If this function returns a prefered deadlock victim, give the lock to the other transaction. If not, the VATS algorithm can be applied. Especially in aggressive parallel replication, there can be a lot of conflicting transactions (one user reported 20% of transactions conflicting, while still getting good speedup from parallel replication). So it might be a worthwhile thing to do. - Kristian.

1 0

Re: [Maria-developers] Optimistic parallel replication for RocksDB
by Kristian Nielsen 11 Oct '16

11 Oct '16

Yoshinori Matsunobu <yoshinori(a)fb.com> writes: > About transaction ids, it's not visible from MyRocks yet. We're currently working on > RocksDB to add an API to get transaction id, and making it available via MyRocks. Ok, I see. Optimistic parallel replication needs the ability to somehow find the THD that is holding the row lock that is blocking another THD. If we have T1 followed by T2, T2 will only commit after T1 has. So if there is a conflicting row lock, we need a way to identify T2 so that the conflict can be resolved. Lock wait timeout is not sufficient here because of the requirement of in-order commit. > Rows are normally released at transaction commit or rollback, but there are some exceptions. > - Auto-increment id allocation is implemented as std::atomic<longlong> and > the lock is released earlier than statement/transaction, like > InnoDB. I hope this doesn't matter for > parallel replication, since auto-inc ids are always given on slaves > (either RBR image, or insert_id with SBR). Agree, it does not matter, MyRocks just should not report these lock conflicts with thd_rpl_deadlock_check() (and since this is using a different mechanism, there is no reason it would). > - MyRocks has data dictionary > (https://github.com/facebook/mysql-5.6/wiki/MyRocks-data-dictionary-format) > and data dictionary operations' transaction scope is different from > applications'. For example, internal index > id allocation is done (and committed) immediately. There is no SQL > statements to directly manipulate data dictionary, > so I assume this won't matter for replication either. Agree, it shouldn't. Optimistic parallel replication handles DDL pessimistically anyway - DDL is not run in parallel with any other statements. Thanks, - Kristian.

1 0

[Maria-developers] Optimistic parallel replication for RocksDB
by Kristian Nielsen 10 Oct '16

10 Oct '16

Sergey, Yoshinori, it was great talking to you about MyRocks in Amsterdam. I took a first look at how to extend MyRocks to work with optimistic parallel replication. It looks conceptually quite simple. Sergey, I understand you have more pressing priorities right now (like getting a tree to build :), so let us revisit this in more detail when you get to it. It looks like the fix is conceptually as simple as this patch, which calls thd_rpl_deadlock_check() whenever a transaction is blocked on a row lock: ----------------------------------------------------------------------- diff --git a/utilities/transactions/transaction_lock_mgr.cc b/utilities/transactions/transaction_lock_mgr.cc index 28e8598..5ff291f 100644 --- a/utilities/transactions/transaction_lock_mgr.cc +++ b/utilities/transactions/transaction_lock_mgr.cc @@ -317,6 +317,8 @@ Status TransactionLockMgr::AcquireWithTimeout(LockMap* lock_map, return result; } +extern "C" int thd_rpl_deadlock_check(MYSQL_THD thd, MYSQL_THD other_thd); + // Try to lock this key after we have acquired the mutex. // Sets *expire_time to the expiration time in microseconds // or 0 if no expiration. @@ -340,6 +342,9 @@ Status TransactionLockMgr::AcquireLocked(LockMap* lock_map, lock_info.expiration_time = txn_lock_info.expiration_time; // lock_cnt does not change } else { + THD *blocked_thd = getTHD(txn_lock_info.txn_id); + THD *bloking_thd = getTHD(lock_info.txn_id); + thd_rpl_deadlock_check(blocked_thd, blocking_thd); result = Status::TimedOut(Status::SubCode::kLockTimeout); } } ----------------------------------------------------------------------- A real patch will need some plumbing to put the code in the right place and have the right information available. Ie. probably the thd_rpl_deadlock_check() call will go into an overridden virtual method in ha_rocksdb.cc. I also did not check if/how one can get from txn_id to THD (what is called getTHD() above), I assume it can be implemented reasonably easy if it is not already there? Hints will be appreciated here as I am new to the MyRocks and RocksDB codebases. When are row locks released? I am interested in whether row locks can be released earlier than at transaction commit time. If so, the simple patch above will give false positives, and it might be worth it to investigate ways to not report locks that are released earlier than commit. Eg. in InnoDB, auto-increment locks are released earlier than commit, and thus are not reported. Once something like this is in place, I think optimistic parallel replication should work. In case of a conflict between transactions T1 and T2, thd_rpl_deadlock_check(T1, T2) will be called and will cause T2 to be killed so that T1 can proceed and T2 be re-tried afterwards. So things look good now; let us revisit this when there is a tree to work on. - Kristian.

1 0

Re: [Maria-developers] 8cd65cd: MDEV-9143 JSON_xxx functions.
by Sergei Golubchik 08 Oct '16

08 Oct '16

Hi, Alexey! On Oct 08, Alexey Botchkov wrote: > > Other comments, that you didn't reply to, does it mean you agree with > > them (like adding tests with values that need json-escaping)? Okay. > > Right, it means that I just agreed with the comment. > > I don't think we're going to need more unittest files so yep, will > move it to strings/ Will check for missing functions from MySQL, try > running MySQL tests with us, then push. > > After that we'll still have one inconsistency with MySQL - that they > have that JSON type for fields and values. JSON-values let the user > to compare values as 'json'-s. Not sure anybody ever uses it. But > JSON fields are probably good to import from MySQL, maybe in read-only > mode. Just to let people switch to MariaDB easily over these JSON > tables. Yes... I think, out of missing features, JSON_TABLE is the most interesting, but also the comlexity is a big unknown. The second one would be GeoJSON (and it's fairly predictable), then goes JSON comparison and JSON data type. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] 8cd65cd: MDEV-9143 JSON_xxx functions.
by Sergei Golubchik 08 Oct '16

08 Oct '16

Hi, Alexey! On Sep 21, Alexey Botchkov wrote: > > > > + In the worst case one character from the 'a' string > > > + turns into '\uXXXX\uXXXX' which is 12. > > > how comes? add an example, please. Like "for example, character x'1234' > > in the charset A becomes '\u1234\u5678' if JSON string is in the charset B" > > For instance the 'SMILING FACE WITH SUNGLASSES' character, > which is utf32(0001F60E) should be represented as \uD83D\uDE0E. I hope you put this in the comment in the code... Other comments, that you didn't reply to, does it mean you agree with them (like adding tests with values that need json-escaping)? Okay. Thanks for unit tests. There weren't many, but if you think they cover all the library functionality, then it's fine. Just one thought - do you expect any more json_lib related test *files* in the future? If not, you could move json_lib-t.cc into unittest/strings. And why the test is .cc? The file looked pure C to me. Anyway, I think it's ok to push after resolving these minor issues. Thanks! Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] [Commits] 1c9da8d: MDEV-9312: storage engine not enforced during galera cluster replication
by Jan Lindström 29 Sep '16

29 Sep '16

Hi Nirbhay, This looks ok but one question (no need to change now): On Wed, Sep 28, 2016 at 7:36 PM, Nirbhay Choubey <nirbhay(a)mariadb.com> wrote: > > + Since some wsrep threads (THDs) are create before plugins are > + initialized, LOCK_plugin mutex needs to be initialized here. > + */ > Is there some fundamental reason why we can't create wsrep threads after other plugins and related global system variables (that are really needed for wsrep) are initialized ? R: Jan

2 1

Re: [Maria-developers] [Commits] a8162d4: MDEV-9312: storage engine not enforced during galera cluster replication
by Sergei Golubchik 28 Sep '16

28 Sep '16

Hi, Nirbhay! I don't understand, why do you need to create a dummy plugin here? On Sep 27, Nirbhay Choubey wrote: > revision-id: a8162d4a8737cff67889390fad0153acc175391d (mariadb-10.1.17-22-ga8162d4) > parent(s): 6a6b253a6ecbd4d3dd254044d12ec64475453275 > author: Nirbhay Choubey > committer: Nirbhay Choubey > timestamp: 2016-09-27 09:03:26 -0400 > message: > > MDEV-9312: storage engine not enforced during galera cluster replication > > Perform a post initialization of plugin-related variables > of wsrep threads after their global counterparts have been > initialized. ... > +#ifdef WITH_WSREP > + > +/* > + Placeholder for global_system_variables.table_plugin required during > + initialization of startup wsrep threads. > +*/ > +static st_plugin_int *wsrep_dummy_plugin; > + > +/* > + Initialize wsrep_dummy_plugin and assign it to > + global_system_variables.table_plugin. > +*/ > +void wsrep_plugins_pre_init() > +{ > + wsrep_dummy_plugin= > + (st_plugin_int *) my_malloc(sizeof(st_plugin_int), MYF(0)); > + wsrep_dummy_plugin->state= PLUGIN_IS_DISABLED; > + global_system_variables.table_plugin= plugin_int_to_ref(wsrep_dummy_plugin); > +} > + Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 2

Re: [Maria-developers] 12ffe70: MDEV-9416: MariaDB galera got signal 11 when altering table add unique index
by Sergei Golubchik 28 Sep '16

28 Sep '16

Hi, Nirbhay! On Sep 24, Nirbhay Choubey wrote: > revision-id: 12ffe70831c9c69c3d6ef83431443696455c332b (mariadb-10.1.17-23-g12ffe70) > parent(s): 8ffded0a78c0a4912b32acac4c3f58f04c3bcd87 > author: Nirbhay Choubey > committer: Nirbhay Choubey > timestamp: 2016-09-24 00:27:38 -0400 > message: > > MDEV-9416: MariaDB galera got signal 11 when altering table add unique index > > When a BF thread attempts to abort a victim thread's transaction, > the victim thread is not locked and thus its not safe to rely on > its data structures like htons registered for the trx. > > So, instead of getting the registered htons from victim, innodb's > hton can be looked up directly from installed_htons[] and used to > abort the transaction. (Same technique is used in older versions) > > diff --git a/sql/handler.cc b/sql/handler.cc > index 4e4c8fa..6a32fec 100644 > --- a/sql/handler.cc > +++ b/sql/handler.cc > @@ -6108,29 +6108,16 @@ int ha_abort_transaction(THD *bf_thd, THD *victim_thd, my_bool signal) > DBUG_RETURN(0); > } > > - /* Try statement transaction if standard one is not set. */ > - THD_TRANS *trans= (victim_thd->transaction.all.ha_list) ? > - &victim_thd->transaction.all : &victim_thd->transaction.stmt; > - > - Ha_trx_info *ha_info= trans->ha_list, *ha_info_next; > - > - for (; ha_info; ha_info= ha_info_next) > + handlerton *hton= installed_htons[DB_TYPE_INNODB]; > + if (hton && hton->abort_transaction) > { > - handlerton *hton= ha_info->ht(); > - if (!hton->abort_transaction) > - { > - /* Skip warning for binlog & wsrep. */ > - if (hton->db_type != DB_TYPE_BINLOG && hton != wsrep_hton) > - { > - WSREP_WARN("Cannot abort transaction."); > - } > - } > - else > - { > - hton->abort_transaction(hton, bf_thd, victim_thd, signal); > - } > - ha_info_next= ha_info->next(); > + hton->abort_transaction(hton, bf_thd, victim_thd, signal); Okay, but please add a comment that it's safe to abort from one thread (bf_thd) the transaction, running in another thread (victim_thd), because innodb's lock_sys and trx_mutex guarantee the necessary protection. And that it's not safe to access victim_thd->transaction, because it's not protected from concurrent accesses. And it's an overkill to take LOCK_plugin and iterate the whole installed_htons[] array every time. Then ok to push > } > + else > + { > + WSREP_WARN("Cannot abort InnoDB transaction"); > + } > + > DBUG_RETURN(0); > } Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] [Commits] f2aea43: MDEV-10649: Optimizer sometimes use "index" instead of "range" access for UPDATE
by Jan Lindström 28 Sep '16

28 Sep '16

On Tue, Sep 6, 2016 at 8:37 PM, Sergei Petrunia <psergey(a)askmonty.org> wrote: > revision-id: f2aea435df7e92fcf8f09f8f6c160161168c5bed > parent(s): a14f61ef749ad9f9ab2b0f5badf6754ba7443c9e > committer: Sergei Petrunia > branch nick: 10.0 > timestamp: 2016-09-06 20:37:21 +0300 > message: > > MDEV-10649: Optimizer sometimes use "index" instead of "range" access for > UPDATE > > (XtraDB variant only, for now) > > Re-opening a TABLE object (after e.g. FLUSH TABLES or open table cache > eviction) causes ha_innobase to call > dict_stats_update(DICT_STATS_FETCH_ONLY_IF_NOT_IN_MEMORY). > > Inside this call, the following is done: > dict_stats_empty_table(table); > dict_stats_copy(table, t); > > On the other hand, commands like UPDATE make this call to get the "rows in > table" statistics in table->stats.records: > > ha_innobase->info(HA_STATUS_VARIABLE|HA_STATUS_NO_LOCK) > > note the HA_STATUS_NO_LOCK parameter. It means, no locks are taken by > ::info() If the ::info() call happens between dict_stats_empty_table > and dict_stats_copy calls, the UPDATE's optimizer will get an estimate > of table->stats.records=1, which causes it to pick a full table scan, > which in turn will take a lot of row locks and cause other bad > consequences. > > --- > storage/xtradb/dict/dict0stats.cc | 29 +++++++++++++++++++---------- > 1 file changed, 19 insertions(+), 10 deletions(-) > > diff --git a/storage/xtradb/dict/dict0stats.cc b/storage/xtradb/dict/ > dict0stats.cc > index b073398..a4aa436 100644 > --- a/storage/xtradb/dict/dict0stats.cc > +++ b/storage/xtradb/dict/dict0stats.cc > @@ -673,7 +673,10 @@ Write all zeros (or 1 where it makes sense) into a > table and its indexes' > dict_stats_copy( > /*============*/ > dict_table_t* dst, /*!< in/out: destination table */ > - const dict_table_t* src) /*!< in: source table */ > + const dict_table_t* src, /*!< in: source table */ > + bool reset_ignored_indexes) /*!< in: if true, set ignored > indexes > + to have the same statistics > as if > + the table was empty */ > { > dst->stats_last_recalc = src->stats_last_recalc; > dst->stat_n_rows = src->stat_n_rows; > @@ -692,7 +695,16 @@ Write all zeros (or 1 where it makes sense) into a > table and its indexes' > && (src_idx = dict_table_get_next_index(src_idx)))) { > > if (dict_stats_should_ignore_index(dst_idx)) { > - continue; > + if (reset_ignored_indexes) { > + /* Reset index statistics for all ignored > indexes, > + unless they are FT indexes (these have no > statistics)*/ > + if (dst_idx->type & DICT_FTS) { > + continue; > + } > + dict_stats_empty_index(dst_idx); > Does this really help? Yes, we hold dict_sys mutex here so dict_stats_empty_index here is safe for all readers using same mutex. However, as you pointed up above, info() uses no locking method. Thus, we really do not know what it will return, all values before dict_stats_copy(), some of the indexes with new stats, all indexes with new stats. > > @@ -3240,13 +3252,10 @@ N*AVG(Ui). In each call it searches for the > currently fetched index into > > dict_table_stats_lock(table, RW_X_LATCH); > > - /* Initialize all stats to dummy values before > - copying because dict_stats_table_clone_create() > does > - skip corrupted indexes so our dummy object 't' may > - have less indexes than the real object 'table'. */ > - dict_stats_empty_table(table); > - > - dict_stats_copy(table, t); > + /* Pass reset_ignored_indexes=true as parameter > + to dict_stats_copy. This will cause statictics > + for corrupted indexes to be set to empty values */ > + dict_stats_copy(table, t, true); > This is better solution than the original but as noted above, is this enough ? R: Jan

2 2