- developers - lists.mariadb.org

Re: [Maria-developers] [Commits] 6f03ea7: MDEV-8202 - st_select_lex::master_unit() takes 0.17% in OLTP RO
by Sergei Golubchik 28 May '15

28 May '15

Hi, Sergey! On May 21, svoj(a)mariadb.org wrote: > revision-id: 6f03ea78aa4a70b9a9f2f096f1d7a6c282ca9575 > parent(s): 79372cf58e426eb60acb9f0abf862a1f0fe0e4e3 > committer: Sergey Vojtovich > branch nick: mariadb > timestamp: 2015-05-21 19:35:49 +0400 > message: > > MDEV-8202 - st_select_lex::master_unit() takes 0.17% in OLTP RO > > To make st_select_lex::master_unit() inlinable: > - moved it's definition to sql_lex.h > - removed base class virtual master_unit() declaration since this method is > specific to st_select_lex ok to push Regards, Sergei

1 0

[Maria-developers] MDEV-7950 Item_func::type() takes 0.26% in OLTP RO
by Alexander Barkov 28 May '15

28 May '15

Hello Sergey, Please review the next step for MDEV-7950. It does the following: - Adds specific implementations of virtual method get_mm_tree() into Item_func_like, Item_bool_rowready_func2, Item_func_spatial_rel - Gets rid of two virtual calls: select_optimize() and have_rev_func(). - Removes virtual methods Item_func::select_optimize(), Item_func::have_rev_func(), as well as enum Item_func::optimize_type, because they are not needed any more. Thanks.

2 1

Re: [Maria-developers] [Commits] 8a595a9: cleanup: LOAD DATA replication support in IO_CACHE
by Sergey Vojtovich 28 May '15

28 May '15

Hi Sergei, a few thoughts/questions inline. On Tue, May 26, 2015 at 03:37:36PM +0200, serg(a)mariadb.org wrote: > revision-id: 8a595a9825de286ad073f6f043db4c0dce88a2ea > parent(s): 3e55ef26d49a900782d2c2bb2c03470faed6ec9d > committer: Sergei Golubchik > branch nick: maria > timestamp: 2015-05-26 15:09:25 +0200 > message: > > cleanup: LOAD DATA replication support in IO_CACHE > > remove some 14-year old code that added support for > LOAD DATA replication to IO_CACHE: > * three callbacks, of which only two were actually used and that > were only needed for LOAD DATA replication but were > tested in every IO_CACHE instance > * an additional opaque void * argument in IO_CACHE, also only > used for LOAD DATA replication, but present everywhere > * the code to close IO_CACHE prematurely in LOAD DATA to have > these callbacks called in the correct order and a long > comment explaining what will happen if IO_CACHE is not > closed prematurely > * a variable to track whether IO_CACHE was closed prematurely > (to avoid double-closing it) > > this also fixes a bug with replication of LOAD DATA LOCAL > (IO_CACHE of READ_NET type was not calling these callbacks at all) I couldn't track this down. That is GET/my_b_get/pre_read was called in both cases as well as end_io_cache/pre_close. Could you elaborate? ...skip... > diff --git a/sql/sql_load.cc b/sql/sql_load.cc > index 18982c5..62b6e17 100644 > --- a/sql/sql_load.cc > +++ b/sql/sql_load.cc > @@ -1410,20 +1372,15 @@ READ_INFO::READ_INFO(File file_par, uint tot_length, CHARSET_INFO *cs, > } > else > { > - /* > - init_io_cache() will not initialize read_function member > - if the cache is READ_NET. So we work around the problem with a > - manual assignment > - */ > - need_end_io_cache = 1; > - > #ifndef EMBEDDED_LIBRARY > if (get_it_from_net) > cache.read_function = _my_b_net_read; > > if (mysql_bin_log.is_open()) > - cache.pre_read = cache.pre_close = > - (IO_CACHE_CALLBACK) log_loaded_block; > + { > + cache.real_read_function= cache.read_function; > + cache.read_function= log_loaded_block; > + } > #endif > } > } Will this compile with replication disabled? OTOH I guess it didn't compile even before: log_loaded_block() seem to be unavailable when replication is disabled. IOCACHE::read_function may change, e.g. on error in _my_b_async_read(). Shouldn't we handle this? > @@ -1432,8 +1389,7 @@ READ_INFO::READ_INFO(File file_par, uint tot_length, CHARSET_INFO *cs, > > READ_INFO::~READ_INFO() > { > - if (need_end_io_cache) > - ::end_io_cache(&cache); > + ::end_io_cache(&cache); > my_free(buffer); > List_iterator<XML_TAG> xmlit(taglist); > XML_TAG *t; Hmm... I may miss something, but shouldn't we call log_loaded_block() for last successfully read block? Thanks, Sergey

2 2

Re: [Maria-developers] [Commits] 1cd9319: myisam/aria: don't mess with IO_CACHE::file
by Sergei Golubchik 28 May '15

28 May '15

Hi, Sergey! On May 27, Sergey Vojtovich wrote: > Hi Sergei, > > I'm lost now. In Aria you did change very similar code and kept > init_io_cache(-1) while init_io_cache(info->dfile) was moved down > and replaced with init_io_cache(new_file). This sounds reasonable. > FWICS MyISAM has very same code. > > Isn't "temp" in fact info->dfile and not -1? Ok, the original problem. init_io_cache(-1) means that IO_CACHE uses a temporary file as a backing store. That is, when in-memory buffer is full, IO_CACHE creates a temporary file to keep the data. MyISAM in a couple of places was creating IO_CACHE with init_io_cache(-1) and later was replacing IO_CACHE::file. That is, IO_CACHE was created as if it was a temporary storage, but later changed to use a real file. Which is not a good idea now, as I (in a later changeset) detect temporary IO_CACHE's in init_io_cache. So, I've fixed MyISAM/Aria not to access internal IO_CACHE fields and create IO_CACHE to have the correct file descriptior from the start. Regards, Sergei

2 1

Re: [Maria-developers] [Commits] 1cd9319: myisam/aria: don't mess with IO_CACHE::file
by Sergei Golubchik 27 May '15

27 May '15

Hi, Sergey! On May 27, Sergey Vojtovich wrote: > Hi Sergei, > > looks good. Ok to push, a few minor suggestions inline. > > > diff --git a/storage/maria/ma_check.c b/storage/maria/ma_check.c > > index 8c012d2..d50528e 100644 > > --- a/storage/maria/ma_check.c > > +++ b/storage/maria/ma_check.c > > @@ -4267,20 +4267,22 @@ int maria_repair_parallel(HA_CHECK *param, register MARIA_HA *info, > > > > if (!(sort_info.key_block= > > alloc_key_blocks(param, (uint) param->sort_key_blocks, > > - share->base.max_key_block_length)) || > > - init_io_cache(&param->read_cache, info->dfile.file, > > + share->base.max_key_block_length))) > > + goto err; > > + > > + if (init_io_cache(&param->read_cache, info->dfile.file, > > (uint) param->read_buffer_length, > > - READ_CACHE, share->pack.header_length, 1, MYF(MY_WME)) || > > - (!rep_quick && > > - (init_io_cache(&info->rec_cache, info->dfile.file, > > - (uint) param->write_buffer_length, > > - WRITE_CACHE, new_header_length, 1, > > - MYF(MY_WME | MY_WAIT_IF_FULL) & param->myf_rw) || > > - init_io_cache(&new_data_cache, -1, > > - (uint) param->write_buffer_length, > > - READ_CACHE, new_header_length, 1, > > - MYF(MY_WME | MY_DONT_CHECK_FILESIZE))))) > > + READ_CACHE, share->pack.header_length, 1, MYF(MY_WME))) > > goto err; > > + > > + if (!rep_quick) > > + { > > + if (init_io_cache(&new_data_cache, -1, > > + (uint) param->write_buffer_length, > > + READ_CACHE, new_header_length, 1, > > + MYF(MY_WME | MY_DONT_CHECK_FILESIZE))) > > + goto err; > > + } > Why not to move new_data_cache initialization down along with rec_cache? I've simply kept it where it was, the goal was to avoid io_cache.file=new_file_descriptor; for example, there was info->rec_cache.file=new_file; and I moved info->rec_cache initialization down where new_file was known and could be passed as an argument to init_io_cache(). There was no such problem with new_data_cache. Regards, Sergei

2 2

[Maria-developers] step7 (cleanup) MDEV-7950 Item_func::type() takes 0.26% in OLTP RO
by Alexander Barkov 27 May '15

27 May '15

Hi Sergey, please review the next step for MDEV-7950. It breaks get_mm_parts() into a virtual method in Item, so - replaces one virtual call item->type() to another virtual call, item->get_mm_tree(). - and also removes this virtual call for functype(), which used to distinguish between COND_AND_FUNC and COND_OR_FUNC: >- if (((Item_cond*) cond)->functype() == Item_func::COND_AND_FUNC) The next patches will: 1. Split the code in Item::get_mm_tree() responsible for functions into methods in Item_func and its descendants: Item_func Item_func_like, to get rid of the select_optimize() call Item_func_between Item_func_in Item_equal 2. Remove virtual method Item_func::select_optimize(), as well as enum Item_func::optimize_type, because they won't be needed any more. Thanks.

2 3

[Maria-developers] Memory barrier problem in InnoDB/XtraDB mutex implementation causing hangs
by Kristian Nielsen 27 May '15

27 May '15

I have been debugging some test failures seen in Buildbot: https://mariadb.atlassian.net/browse/MDEV-7026 After some debugging, this is what I think is going on. I will refer to MariaDB 5.5 sources, though the problem is also in 10.0. InnoDB/XtraDB mutexes are taken in mutex_enter_func() and unlocked in mutex_exit_func(). Simplifying for clarity, here are the relevant parts of the logic: ENTER: # First spin wait, hoping to get the mutex without yielding. os_compare_and_swap_ulint(&mutex->waiters, 0, 1); if (os_atomic_test_and_set_byte(&mutex->lock_word, 1)) sync_array_wait_event() RELEASE: os_atomic_lock_release_byte(&mutex->lock_word); os_rmb; waiters = *(volatile *)&mutex->waiters; if (waiters) mutex_signal_object(mutex); The interesting point here is how to ensure that we do not lose the wakeup of a waiting thread when the holding thread releases the mutex. The idea is that the waiting thread first sets mutex->waiters=1. Then it checks if the mutex is free - if not, it goes to sleep. The thread that releases the lock first marks the mutex free. Then it checks if there are any waiters, and if so wakes them up. For this to work, we need to ensure that the setting of waiters=1 has completed before the load of lock_word can start - a full memory barrier. Likewise, we need the setting of lock_word to 0 to complete before the load of mutex->waiters - again a full memory barrier. Otherwise we can get something like this: ENTER RELEASE Start store to mutex->waiters Start store to lock_word Load mutex->lock_word Store to lock_word becomes visible Load mutex->waiters Store to waiters becomes visible Check lock word==1 Check waiters==0, skip wakeup Sleep (forever) Or this: ENTER RELEASE Start store to lock_word Start store to mutex->waiters Load mutex->waiters Store to waiters becomes visible Load mutex->lock_word Store to lock_word becomes visible Check lock word==1 Sleep (forever) Check waiters==0, skip wakeup (So both full barriers are needed, the one in ENTER to prevent the first example, the one in RELEASE to prevent the second). It looks like for x86, the full barriers were broken in MariaDB 5.5.40 and 10.0.13. os_compare_and_swap_ulint() is defined as either __sync_bool_compare_and_swap() or atomic_cas_ulong(), both of which implies a full barrier. So ENTER looks ok. os_atomic_lock_release_byte() however is now defined as __sync_lock_release(). This is _not_ a full barrier, just a release barrier. So the RELEASE case seems to be incorrect, allowing the second deadlock example to occur. Previously, RELEASE used os_atomic_test_and_set_byte(), which is __sync_lock_test_and_set(). This is defined in GCC docs as an "aquire" barrier only, which is also wrong, obviously. But it happens to generate an XCHG instruction on x86, which is a full barrier. This is probably why it works on x86 (and probably why it did not work on PowerPC). Here is a small test program, and the assembly from GCC 5.7 on amd64: void test(unsigned *p, unsigned *q) { while (__sync_lock_test_and_set(q, 1)) { } *p++; __sync_lock_release(q); } test: movl $1, %edx .L2: movl %edx, %eax xchgl (%rsi), %eax testl %eax, %eax jne .L2 movl $0, (%rsi) ret As can be seen, the __sync_lock_test_and_set() generates a full barrier, but the __sync_lock_release() generates nothing (as this is sufficient for release semantics for the rather strong memory order on x86). So the InnoDB mutexes look completely broken from MariaDB 5.5.40 and 10.0.13 :-( This is consistent with the observations in MDEV-7026. A thread is waiting for wakeup on a mutex that is already marked free. Apparently this is not a new problem. This comment in mutex_exit_func() makes me want to cry: /* A problem: we assume that mutex_reset_lock word is a memory barrier, that is when we read the waiters field next, the read must be serialized in memory after the reset. A speculative processor might perform the read first, which could leave a waiting thread hanging indefinitely. Our current solution call every second sync_arr_wake_threads_if_sema_free() to wake up possible hanging threads if they are missed in mutex_signal_object. */ So someone found the mutex implementation broken, but instead of fixing it, they implemented a horrible workaround to try and break any deadlocks that might occur. (The reason that the deadlocks are not broken in the MDEV-7026 seems to be that it occurs during startup, before starting the srv_error_monitor_thread() that does the sync_arr_wake_threads_if_sema_free()). Now, what is also interesting is that this seems to explain very nicely the many server lockups we have seen for users that upgrade to recent MariaDB versions. I was doing a very detailed investigation of one such case where the user was able to supply unusually detailed information. I found a thread that was stuck waiting on a mutex, but no other threads seemed to be holding that mutex. This is exactly what would be seen in case of a lost wakeup due to this issue. Furthermore, the sync_arr_wake_threads_if_sema_free() mechanism, as well as the normal InnoDB "long semaphore wait", was itself blocked on that same mutex wait, which explains why they were not effective for the user in that particular case. So it looks like we need to revert the incorrect patch and release new 5.5.40a and 10.0.15 releases ASAP, to solve these hangs for users. And do a proper patch for PowerPC with proper full memory barriers. (Incidentally, the os_rmb in RELEASE seems to also be part of some PowerPC patch, it also looks completely wrong). However, more importantly, this issue clearly shows some fundamental problems in our development procedures that we need to fix ASAP. Just look at it. We have users (and customers!) that suffer from strange hangs. This is their production systems that are down. Support and developers are unable to help them due to the difficulty of debugging rare race conditions in production systems. And all the time the bug was staring us in the face, easily reproducable in our own Buildbot! But we deliberately chose to ignore those failures. The only thing that matters is "when can you push", and getting more releases out. Never mind what kind of crap is then pushed out to users. I am deeply ashamed of treating our users like this. And extremely unhappy. I even wrote about this (the importance of not ignoring Buildbot failures) just a few weeks ago. This was completely ignored, I got zero replies, and the weekly calls continued with mindless juggling of Jira numbers and buggy releases. As a result, our users suffer. We need to stop this. Please? - Kristian.

4 16

Re: [Maria-developers] Rotating from encry
by Jonas Oreland 22 May '15

22 May '15

8043 I tried using a search engine and googled for "jira mariadb innodb encryption jonas" :) /Jonas On Thu, May 21, 2015 at 4:36 PM, Jan Lindström <jan.lindstrom(a)mariadb.com> wrote: > Hi, > > Can you give the MDEV, I do not seem to be able to find it. > > R: Jan > > On Thu, May 21, 2015 at 2:51 PM, Jonas Oreland <jonaso(a)google.com> wrote: > >> i made a JIRA entry with latest innodb encryption... >> >> /Jonas >> >> On Thu, May 21, 2015 at 12:30 PM, Jan Lindström < >> jan.lindstrom(a)mariadb.com> wrote: >> >>> Hi, >>> >>> On Thu, May 21, 2015 at 11:33 AM, Jonas Oreland <jonaso(a)google.com> >>> wrote: >>> >>>> Hi Jan, >>>> >>>> 1) rotating encrypted => unencrypted is definitely supported, >>>> in my latest version the TODO is removed...(hope you have a recent >>>> enough version) >>>> >>> >>> Based on this, I do not have the latest version. Where, I could get >>> that, it does not seem to be on branch? >>> >>>> >>>> >>>> 2) Thanks! for testcase, it did indeed reveal a bug with the "encrypted >>>> => unencrypted => encrypted" sequence >>>> >>>> attaching fix. >>>> >>>> Let me know it fixes your version of testcase (i've used a modified >>>> version since e.g we don't have per table settings...) >>>> >>> >>> Yes, this fix is good. Thanks. >>> >>> R: Jan >>> >> >> >

1 0

[Maria-developers] How do I read information_schema.global_status not using query.
by kyung nam Kim 22 May '15

22 May '15

Hi, I hava a trouble with getting a data from informaition_schema.global_status table not using query. I wrote below code and it's possible to get data from some table in performance_schema . ** reader.cc =============================================================================================== my_thread_init(); ... MEM_ROOT mem_field; READ_RECORD read_record_info; tables.init_one_table(STRING_WITH_LEN("performance_schema"), STRING_WITH_LEN("events_statements_current"), bool result = open_and_lock_tables(ctx->thd, &tables, FALSE, MYSQL_LOCK_IGNORE_TIMEOUT); if (result) { return false; } if(table = tables.table) /* if only table exists */ { init_sql_alloc(&mem_field, 1024, 0, MYF(0)); ... while (!(error= read_record_info.read_record(&read_record_info))) { char* value = get_field(&mem_field, table->field[0]); // It's OK. .... } } ... ==================================================================================== It's possible to read data from performance_schema tables using low-level function . But I tried to read data from information_schema.global_status table as same way(just changed the name of database and table) , an error occurred in open_and_lock_tables(). Is there a way to read data from information_schema.global_status not using query? Regards, Kim kyungnam.

1 0

Re: [Maria-developers] Rotating from encry
by Jonas Oreland 21 May '15

21 May '15

i made a JIRA entry with latest innodb encryption... /Jonas On Thu, May 21, 2015 at 12:30 PM, Jan Lindström <jan.lindstrom(a)mariadb.com> wrote: > Hi, > > On Thu, May 21, 2015 at 11:33 AM, Jonas Oreland <jonaso(a)google.com> wrote: > >> Hi Jan, >> >> 1) rotating encrypted => unencrypted is definitely supported, >> in my latest version the TODO is removed...(hope you have a recent enough >> version) >> > > Based on this, I do not have the latest version. Where, I could get that, > it does not seem to be on branch? > >> >> >> 2) Thanks! for testcase, it did indeed reveal a bug with the "encrypted >> => unencrypted => encrypted" sequence >> >> attaching fix. >> >> Let me know it fixes your version of testcase (i've used a modified >> version since e.g we don't have per table settings...) >> > > Yes, this fix is good. Thanks. > > R: Jan >

1 0