- developers - lists.mariadb.org

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

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

21 May '15

Hi Jan, 1) rotating encrypted => unencrypted is definitely supported, in my latest version the TODO is removed...(hope you have a recent enough version) 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...) /Jonas On Fri, May 15, 2015 at 1:23 PM, Jan Lindström <jan.lindstrom(a)mariadb.com> wrote: > Hi, > > At fil0crypt.cc there is > > fil_crypt_needs_rotation(uint key_version, const key_state_t *key_state) > { > // TODO(jonaso): Add support for rotating encrypted => unencrypted > > if (key_version == 0 && key_state->key_version != 0) { > /* this is rotation unencrypted => encrypted > * ignore rotate_key_age */ > return true; > } > > Thus to me it is not clear is the support for rotating encrypted => > unencrypted really missing or not and furthermore, see attached test case > for this, > > encrypted + insert + grep : ok > encrypted => unencrypted + grep: ok > unencrypted => encrypted + grep: not ok > > R: Jan >

1 0

Re: [Maria-developers] [Commits] b2db8e8: MDEV-7949: Item_field::used_tables() takes 0.29% in OLTP RO
by Sergey Vojtovich 21 May '15

21 May '15

Hi Sanja, this patch looks Ok, just one minor suggestion inline. Though effect for given workload is quite moderate: Item_field::used_tables 0.21% -> 0.20% Item_basic_constant::used_tables 0.11% -> 0.11% Item_func::used_tables 0.05% -> 0.05% Item_sum::used_tables 0.00% -> 0.00% On Wed, May 13, 2015 at 04:17:26PM +0200, sanja(a)mariadb.com wrote: > revision-id: b2db8e85422a455a10cd28ef7e44977182528f70 > parent(s): b22959903b89e798f8804ec9a815c88f75915cd9 > committer: Oleksandr Byelkin > branch nick: server > timestamp: 2015-05-13 16:17:22 +0200 > message: > > MDEV-7949: Item_field::used_tables() takes 0.29% in OLTP RO > > small sixes of used_tables() usage > ...skip... > diff --git a/sql/item_func.h b/sql/item_func.h > index 0d57c2b..c48cd84 100644 > --- a/sql/item_func.h > +++ b/sql/item_func.h > @@ -1337,7 +1337,7 @@ class Item_func_sleep :public Item_int_func > const char *func_name() const { return "sleep"; } > table_map used_tables() const > { > - return Item_int_func::used_tables() | RAND_TABLE_BIT; > + return used_tables_cache | RAND_TABLE_BIT; > } > bool is_expensive() { return 1; } > longlong val_int(); > @@ -1591,7 +1591,7 @@ class Item_func_get_lock :public Item_int_func > void fix_length_and_dec() { max_length=1; maybe_null=1;} > table_map used_tables() const > { > - return Item_int_func::used_tables() | RAND_TABLE_BIT; > + return used_tables_cache | RAND_TABLE_BIT; > } > bool const_item() const { return 0; } > bool is_expensive() { return 1; } > @@ -1611,7 +1611,7 @@ class Item_func_release_lock :public Item_int_func > void fix_length_and_dec() { max_length= 1; maybe_null= 1;} > table_map used_tables() const > { > - return Item_int_func::used_tables() | RAND_TABLE_BIT; > + return used_tables_cache | RAND_TABLE_BIT; > } > bool const_item() const { return 0; } > bool is_expensive() { return 1; } > @@ -1724,7 +1724,7 @@ class Item_func_set_user_var :public Item_func > void fix_length_and_dec(); > table_map used_tables() const > { > - return Item_func::used_tables() | RAND_TABLE_BIT; > + return used_tables_cache | RAND_TABLE_BIT; > } > bool const_item() const { return 0; } > bool is_expensive() { return 1; } Was there any reason for this change? Compiler should be clever enough to inline used_tables() of base class here. Regards, Sergey

3 2

[Maria-developers] Misnamed libmariadb-dev packages on aptitude
by pcrews 19 May '15

19 May '15

Hello, Today, I was experimenting with building mariadb images and encountered this issue: E: Unable to locate package libmariadb-dev pcrews@erlking-dev:~/git/dibtest$ aptitude search libmariadb p libmariadbd-dev - MariaDB embedded database development files p libmariadbd-dev:i386 - MariaDB embedded database development files Note how these are listed as libmariadbd vs. libmariadb. I am writing to see if this can be fixed or if this might be some change in naming (which would require some updating of other tools' references, etc). Thank you, Patrick Crews

1 0