December 2014 - developers - lists.mariadb.org

[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(… [View More]&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. [View Less]

4 16

Re: [Maria-developers] Row-based replication of old DECIMAL to new DECIMAL
by Sergei Golubchik 13 Feb '15

13 Feb '15

Hi, Alexander! On Dec 22, Alexander Barkov wrote: > Hi Sergei, Kristian, > > I'm looking at: > > MDEV-7268 Column of table cannot be converted from type 'decimal(0,?)' > to type ' 'decimal(10,7)' > > This bug cannot be fixed in general case, because the old DECIMAL data > type does not write its metadata into the binary log. > > I suggest we don't fix replication of old DECIMAL > (neither binlogging, nor slave-side). I think that's reasonable. But then … [View More]

7 15

[Maria-developers] Dynamic columns support in libmaria client library?
by Peter Laursen 28 Jan '15

28 Jan '15

This page https://mariadb.com/kb/en/mariadb/documentation/nosql/dynamic-columns-api/ does not mention anything about it. it just says *The API is a part of libmysql C client library. In order to use it, one needs to include this header file* *#include <mysql/ma_dyncol.h>* *and link against libmysql.* Now can ma_dyncol.h be linked against libmaria as well? There are 2 questions actually 1) technical: will it work with libmaria? 2) legal: can a client program using libmaria under LGPL … [View More]

3 5

[Maria-developers] MDEV-7288 USER/ROLE: CREATE OR REPLACE, CREATE IF NOT EXISTS, DROP IF EXISTS
by Alexander Barkov 09 Jan '15

09 Jan '15

Hi Sergei, Please have a look into the patch for MDEV-7288. It consists of the USER/ROLE related part of the big GSoC patch from Sriram that you reviewed earlier, with review suggestions addressed. You earlier had the following suggestions/questions: >> diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc >> index 5f077c2..1e156be 100644 >> --- a/sql/sql_acl.cc >> +++ b/sql/sql_acl.cc >> ... >> @@ -9215,10 +9252,26 @@ bool mysql_create_user(THD *thd, List … [View More]

3 3

[Maria-developers] New buildbot? - debpkg101-sid from Otto's github
by Otto Kekäläinen 02 Jan '15

02 Jan '15

Hello Daniel! Thanks for setting up the current buildbots that build my branches of 5.5 and 10.0. Could you now please also set up one the build from my 10.1 branch as I have started working on it too? The buildbot should be a copy of http://buildbot.askmonty.org/buildbot/builders/debpkg-sid/ but so that it pulls in the branch 10.1 from the repository https://github.com/ottok/mariadb Thanks! 2014-09-01 11:25 GMT+03:00 Otto Kekäläinen <otto(a)seravo.fi>: > Hello! > > 2014-… [View More]

1 2

Re: [Maria-developers] [Commits] b2f45ba: MDEV-7324 - Lock-free hash for table definition cache
by Sergei Golubchik 28 Dec '14

28 Dec '14

Hi, Sergey! On Dec 22, svoj(a)mariadb.org wrote: > revision-id: b2f45ba8280c97f8dc8b1adb3234cf2e28e48982 > parent(s): 4a469b0ed68bb341e1a1e1ba540a45a3b6ff3ef9 > committer: Sergey Vojtovich > branch nick: 10.1 > timestamp: 2014-12-22 19:56:48 +0400 > message: > > MDEV-7324 - Lock-free hash for table definition cache That looked good to me. I didn't find anything obviously wrong. There are mostly questions below. Some are style suggestions (like, renaming of methods, … [View More]etc). The only place I'm not sure about is early lf_hash_search_unpin() in tdc_acquire_share(). > diff --git a/mysys/lf_hash.c b/mysys/lf_hash.c > --- a/mysys/lf_hash.c > +++ b/mysys/lf_hash.c > @@ -122,7 +122,7 @@ static int lfind(LF_SLIST * volatile *head, CHARSET_INFO *cs, uint32 hashnr, > { > if (unlikely(callback)) > { > - if (callback(cursor->curr + 1, (void*)key)) > + if (cur_hashnr & 1 && callback(cursor->curr + 1, (void*)key)) Sorry for this. > return 1; > } > else if (cur_hashnr >= hashnr) > diff --git a/sql/sql_class.cc b/sql/sql_class.cc > --- a/sql/sql_class.cc > +++ b/sql/sql_class.cc > @@ -912,17 +912,17 @@ bool Drop_table_error_handler::handle_condition(THD *thd, > #endif /* defined(ENABLED_DEBUG_SYNC) */ > wait_for_commit_ptr(0), > main_da(0, false, false), > - m_stmt_da(&main_da) > + m_stmt_da(&main_da), > #ifdef WITH_WSREP > - , > wsrep_applier(is_wsrep_applier), > wsrep_applier_closing(false), > wsrep_client_thread(false), > wsrep_apply_toi(false), > wsrep_po_handle(WSREP_PO_INITIALIZER), > wsrep_po_cnt(0), > - wsrep_apply_format(0) > + wsrep_apply_format(0), > #endif > + tdc_hash_pins(0) Eh... I actually wanted wsrep related data to the at the end of the THD. Just to keep the rest of the THD layout identical for wsrep and non-wsrep builds. In case a wsrep-enabled plugin is loaded into non-wsrep server or visa versa. That was an early change, though, I've done more cleanups after that and moved wsrep into a service, so perhaps (even, most probably) this is not needed anymore. But I'd better stay on the safe side and keep wsrep at the end of THD. > { > ulong tmp; > > @@ -1698,6 +1698,8 @@ void THD::cleanup(void) > > free_root(&main_mem_root, MYF(0)); > main_da.free_memory(); > + if (tdc_hash_pins) > + lf_hash_put_pins(tdc_hash_pins); no tdc_hash_pins=0 here? > if (status_var.memory_used != 0) > { > DBUG_PRINT("error", ("memory_used: %lld", status_var.memory_used)); > diff --git a/sql/table.h b/sql/table.h > --- a/sql/table.h > +++ b/sql/table.h > @@ -611,32 +612,7 @@ struct TABLE_SHARE > mysql_mutex_t LOCK_ha_data; /* To protect access to ha_data */ > mysql_mutex_t LOCK_share; /* To protect TABLE_SHARE */ > > - typedef I_P_List <TABLE, TABLE_share> TABLE_list; > - typedef I_P_List <TABLE, All_share_tables> All_share_tables_list; > - struct > - { > - /** > - Protects ref_count, m_flush_tickets, all_tables, free_tables, flushed, > - all_tables_refs. > - */ > - mysql_mutex_t LOCK_table_share; > - mysql_cond_t COND_release; > - TABLE_SHARE *next, **prev; /* Link to unused shares */ > - uint ref_count; /* How many TABLE objects uses this */ > - uint all_tables_refs; /* Number of refs to all_tables */ > - /** > - List of tickets representing threads waiting for the share to be flushed. > - */ > - Wait_for_flush_list m_flush_tickets; > - /* > - Doubly-linked (back-linked) lists of used and unused TABLE objects > - for this share. > - */ > - All_share_tables_list all_tables; > - TABLE_list free_tables; > - ulong version; > - bool flushed; > - } tdc; > + TDC_element *tdc; Hmm. Ok. I think I like this change - TABLE_SHARE now contains table metadata and everything related to tdc is moved to TDC_element. Right? > > LEX_CUSTRING tabledef_version; > > diff --git a/sql/table.cc b/sql/table.cc > --- a/sql/table.cc > +++ b/sql/table.cc > @@ -3845,11 +3844,11 @@ bool TABLE_SHARE::visit_subgraph(Wait_for_flush *wait_for_flush, > because we won't try to acquire tdc.LOCK_table_share while > holding a write-lock on MDL_lock::m_rwlock. > */ > - mysql_mutex_lock(&tdc.LOCK_table_share); > - tdc.all_tables_refs++; > - mysql_mutex_unlock(&tdc.LOCK_table_share); > + mysql_mutex_lock(&tdc->LOCK_table_share); > + tdc->all_tables_refs++; > + mysql_mutex_unlock(&tdc->LOCK_table_share); Do I understand it correctly that a TDC_element can never be deleted from LF_HASH as long as at least one TABLE_SHARE references it? > > - All_share_tables_list::Iterator tables_it(tdc.all_tables); > + TDC_element::All_share_tables_list::Iterator tables_it(tdc->all_tables); > > /* > In case of multiple searches running in parallel, avoid going > @@ -3946,21 +3945,10 @@ bool TABLE_SHARE::wait_for_old_version(THD *thd, struct timespec *abstime, > > mdl_context->done_waiting_for(); > > - mysql_mutex_lock(&tdc.LOCK_table_share); > - > - tdc.m_flush_tickets.remove(&ticket); > - > - if (tdc.m_flush_tickets.is_empty() && tdc.ref_count == 0) > - { > - /* > - If our thread was the last one using the share, > - we must destroy it here. > - */ > - mysql_mutex_unlock(&tdc.LOCK_table_share); > - destroy(); > - } > - else > - mysql_mutex_unlock(&tdc.LOCK_table_share); > + mysql_mutex_lock(&tdc->LOCK_table_share); > + tdc->m_flush_tickets.remove(&ticket); > + mysql_cond_broadcast(&tdc->COND_release); > + mysql_mutex_unlock(&tdc->LOCK_table_share); Why is the logic here different? > > > /* > diff --git a/sql/table_cache.h b/sql/table_cache.h > --- a/sql/table_cache.h > +++ b/sql/table_cache.h > @@ -16,6 +16,171 @@ > Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */ > > > +extern PSI_mutex_key key_TABLE_SHARE_LOCK_table_share; > +extern PSI_cond_key key_TABLE_SHARE_COND_release; > + > +class TDC_element > +{ > +public: > + uchar m_key[NAME_LEN + 1 + NAME_LEN + 1]; > + uint m_key_length; > + ulong version; > + bool flushed; > + TABLE_SHARE *share; > + > + typedef I_P_List <TABLE, TABLE_share> TABLE_list; > + typedef I_P_List <TABLE, All_share_tables> All_share_tables_list; > + /** > + Protects ref_count, m_flush_tickets, all_tables, free_tables, flushed, > + all_tables_refs. > + */ > + mysql_mutex_t LOCK_table_share; > + mysql_cond_t COND_release; > + TDC_element *next, **prev; /* Link to unused shares */ > + uint ref_count; /* How many TABLE objects uses this */ > + uint all_tables_refs; /* Number of refs to all_tables */ > + /** > + List of tickets representing threads waiting for the share to be flushed. > + */ > + Wait_for_flush_list m_flush_tickets; > + /* > + Doubly-linked (back-linked) lists of used and unused TABLE objects > + for this share. > + */ > + All_share_tables_list all_tables; > + TABLE_list free_tables; > + > + TDC_element() {} > + > + TDC_element(const char *key, uint key_length) : m_key_length(key_length) > + { > + memcpy(m_key, key, key_length); > + } > + > + > + void dbug_release_assertions() Somewhat confusing name. If I hadn't see the code below I'd thought this function releases some assertions (how? why?) may be assert_correctness_at_release() would be less ambiguous? > + { > + DBUG_ASSERT(share == 0); > + DBUG_ASSERT(ref_count == 0); > + DBUG_ASSERT(m_flush_tickets.is_empty()); > + DBUG_ASSERT(all_tables.is_empty()); > + DBUG_ASSERT(free_tables.is_empty()); > + DBUG_ASSERT(all_tables_refs == 0); > + DBUG_ASSERT(next == 0); > + DBUG_ASSERT(prev == 0); > + } > + > + > + /** > + Acquire TABLE object from table cache. > + > + @pre share must be protected against removal. > + > + Acquired object cannot be evicted or acquired again. > + > + While locked: > + - pop object from TABLE_SHARE::tdc.free_tables > + > + While unlocked: > + - mark object used by thd "While locked" / "While unlocked" - what does that mean? > + > + @return TABLE object, or NULL if no unused objects. > + */ > + > + TABLE *acquire_table(THD *thd) > + { > + TABLE *table; > + > + mysql_mutex_lock(&LOCK_table_share); > + table= free_tables.pop_front(); > + if (table) > + { > + DBUG_ASSERT(!table->in_use); lots of asserts - good! > + table->in_use= thd; > + /* The ex-unused table must be fully functional. */ > + DBUG_ASSERT(table->db_stat && table->file); > + /* The children must be detached from the table. */ > + DBUG_ASSERT(!table->file->extra(HA_EXTRA_IS_ATTACHED_CHILDREN)); I don't even think of asking why it's HA_EXTRA_IS_ATTACHED_CHILDREN and not HA_EXTRA_HAS_ATTACHED_CHILDREN or, at least, HA_EXTRA_CHILDREN_ARE_ATTACHED. Ah, whatever... Horrors of the English language hidden in old MySQL code :) > + } > + mysql_mutex_unlock(&LOCK_table_share); > + return table; > + } > + > + > + /** > + Get last element of free_tables. > + */ > + > + TABLE *free_tables_back() Eh, I've thought for a few seconds what could it mean to "free tables back" until I read the function comment. You see, "free" is so often used as a verb that one absolutely doesn't see it that here it means "free_tables' back" even though it's also a common naming pattern (xxx_back for the list xxx). Rename the method, may be? > + { > + TABLE_list::Iterator it(share->tdc->free_tables); > + TABLE *entry, *last= 0; > + while ((entry= it++)) > + last= entry; > + return last; > + } > + > + > + /** > + Wait for MDL deadlock detector to complete traversing tdc.all_tables. > + > + Must be called before updating TABLE_SHARE::tdc.all_tables. > + */ > + > + void wait_for_mdl_deadlock_detector() > + { > + while (all_tables_refs) > + mysql_cond_wait(&COND_release, &LOCK_table_share); > + } > + > + > + /** > + Prepeare table share for use with table definition cache. > + */ > + > + static void lf_alloc_constructor(uchar *arg) > + { > + TDC_element *element= (TDC_element*) (arg + LF_HASH_OVERHEAD); > + DBUG_ENTER("tdc_init_share"); intentionally? or forgot to update DBUG_ENTER? > + mysql_mutex_init(key_TABLE_SHARE_LOCK_table_share, > + &element->LOCK_table_share, MY_MUTEX_INIT_FAST); > + mysql_cond_init(key_TABLE_SHARE_COND_release, &element->COND_release, 0); > + element->m_flush_tickets.empty(); > + element->all_tables.empty(); > + element->free_tables.empty(); > + element->all_tables_refs= 0; > + element->share= 0; > + element->ref_count= 0; > + element->next= 0; > + element->prev= 0; > + DBUG_VOID_RETURN; > + } > + > + > + /** > + Release table definition cache specific resources of table share. > + */ > + > + static void lf_alloc_destructor(uchar *arg) > + { > + TDC_element *element= (TDC_element*) (arg + LF_HASH_OVERHEAD); > + DBUG_ENTER("tdc_deinit_share"); intentionally? or forgot to update DBUG_ENTER? > + element->dbug_release_assertions(); > + mysql_cond_destroy(&element->COND_release); > + mysql_mutex_destroy(&element->LOCK_table_share); > + DBUG_VOID_RETURN; > + } > + > + > + static uchar *key(const TDC_element *element, size_t *length, > + my_bool not_used __attribute__((unused))) > + { > + *length= element->m_key_length; > + return (uchar*) element->m_key; > + } > +}; > + > + > enum enum_tdc_remove_table_type > { > TDC_RT_REMOVE_ALL, > diff --git a/sql/sql_base.cc b/sql/sql_base.cc > --- a/sql/sql_base.cc > +++ b/sql/sql_base.cc > @@ -270,58 +270,75 @@ uint get_table_def_key(const TABLE_LIST *table_list, const char **key) > # Pointer to list of names of open tables. > */ > > -OPEN_TABLE_LIST *list_open_tables(THD *thd, const char *db, const char *wild) > +struct list_open_tables_arg > { > + THD *thd; > + const char *db; > + const char *wild; > TABLE_LIST table_list; > OPEN_TABLE_LIST **start_list, *open_list; > - TABLE_SHARE *share; > - TDC_iterator tdc_it; > - DBUG_ENTER("list_open_tables"); > +}; > > - bzero((char*) &table_list,sizeof(table_list)); > - start_list= &open_list; > - open_list=0; > > - tdc_it.init(); > - while ((share= tdc_it.next())) > +static my_bool list_open_tables_callback(TDC_element *element, > + list_open_tables_arg *arg) > { > - if (db && my_strcasecmp(system_charset_info, db, share->db.str)) > - continue; > - if (wild && wild_compare(share->table_name.str, wild, 0)) > - continue; > + char *db= (char*) element->m_key; > + char *table_name= (char*) element->m_key + strlen((char*) element->m_key) + 1; > > + if (arg->db && my_strcasecmp(system_charset_info, arg->db, db)) > + return FALSE; > + if (arg->wild && wild_compare(table_name, arg->wild, 0)) > + return FALSE; > + > /* Check if user has SELECT privilege for any column in the table */ > - table_list.db= share->db.str; > - table_list.table_name= share->table_name.str; > - table_list.grant.privilege=0; > + arg->table_list.db= db; > + arg->table_list.table_name= table_name; > + arg->table_list.grant.privilege= 0; > > - if (check_table_access(thd,SELECT_ACL,&table_list, TRUE, 1, TRUE)) > - continue; > + if (check_table_access(arg->thd, SELECT_ACL, &arg->table_list, TRUE, 1, TRUE)) > + return FALSE; > > - if (!(*start_list = (OPEN_TABLE_LIST *) > - sql_alloc(sizeof(**start_list)+share->table_cache_key.length))) > - { > - open_list=0; // Out of memory > - break; > - } > + if (!(*arg->start_list= (OPEN_TABLE_LIST *) sql_alloc( when you know the thd, prefer thd->alloc() over sql_alloc() (which is, basically, current_thd->alloc()) > + sizeof(**arg->start_list) + element->m_key_length))) > + return TRUE; > + > - strmov((*start_list)->table= > - strmov(((*start_list)->db= (char*) ((*start_list)+1)), > - share->db.str)+1, > - share->table_name.str); > + strmov((*arg->start_list)->table= > + strmov(((*arg->start_list)->db= (char*) ((*arg->start_list) + 1)), > + db) + 1, table_name); > - (*start_list)->in_use= 0; > + (*arg->start_list)->in_use= 0; > - mysql_mutex_lock(&share->tdc.LOCK_table_share); > - TABLE_SHARE::All_share_tables_list::Iterator it(share->tdc.all_tables); > + > + mysql_mutex_lock(&element->LOCK_table_share); > + TDC_element::All_share_tables_list::Iterator it(element->all_tables); > TABLE *table; > while ((table= it++)) > if (table->in_use) > - ++(*start_list)->in_use; > + ++(*arg->start_list)->in_use; > - mysql_mutex_unlock(&share->tdc.LOCK_table_share); > + mysql_mutex_unlock(&element->LOCK_table_share); > - (*start_list)->locked= 0; /* Obsolete. */ > + (*arg->start_list)->locked= 0; /* Obsolete. */ > - start_list= &(*start_list)->next; > + arg->start_list= &(*arg->start_list)->next; > - *start_list=0; > + *arg->start_list= 0; > + return FALSE; > } > - tdc_it.deinit(); > - DBUG_RETURN(open_list); > + > + > +OPEN_TABLE_LIST *list_open_tables(THD *thd, const char *db, const char *wild) > +{ > + list_open_tables_arg argument; > + DBUG_ENTER("list_open_tables"); > + > + argument.thd= thd; > + argument.db= db; > + argument.wild= wild; > + bzero((char*) &argument.table_list, sizeof(argument.table_list)); > + argument.start_list= &argument.open_list; > + argument.open_list= 0; > + > + if (tdc_iterate(thd, (my_hash_walk_action) list_open_tables_callback, > + &argument, true)) > + DBUG_RETURN(0); > + > + DBUG_RETURN(argument.open_list); > } > > /***************************************************************************** > diff --git a/sql/table_cache.cc b/sql/table_cache.cc > --- a/sql/table_cache.cc > +++ b/sql/table_cache.cc > @@ -72,32 +76,23 @@ > /** > Protects unused shares list. > > - TABLE_SHARE::tdc.prev > - TABLE_SHARE::tdc.next > - oldest_unused_share > - end_of_unused_share > + TDC_element::prev > + TDC_element::next > + unused_shares > */ > > static mysql_mutex_t LOCK_unused_shares; > -static mysql_rwlock_t LOCK_tdc; /**< Protects tdc_hash. */ > my_atomic_rwlock_t LOCK_tdc_atomics; /**< Protects tdc_version. */ > > #ifdef HAVE_PSI_INTERFACE > -static PSI_mutex_key key_LOCK_unused_shares, key_TABLE_SHARE_LOCK_table_share; > +PSI_mutex_key key_LOCK_unused_shares, key_TABLE_SHARE_LOCK_table_share; really? not static? > static PSI_mutex_info all_tc_mutexes[]= > { > { &key_LOCK_unused_shares, "LOCK_unused_shares", PSI_FLAG_GLOBAL }, > { &key_TABLE_SHARE_LOCK_table_share, "TABLE_SHARE::tdc.LOCK_table_share", 0 } > }; > > -static PSI_rwlock_key key_rwlock_LOCK_tdc; > -static PSI_rwlock_info all_tc_rwlocks[]= > -{ > - { &key_rwlock_LOCK_tdc, "LOCK_tdc", PSI_FLAG_GLOBAL } > -}; > - > - > -static PSI_cond_key key_TABLE_SHARE_COND_release; > +PSI_cond_key key_TABLE_SHARE_COND_release; and here > static PSI_cond_info all_tc_conds[]= > { > { &key_TABLE_SHARE_COND_release, "TABLE_SHARE::tdc.COND_release", 0 } > @@ -265,85 +269,41 @@ void tc_add_table(THD *thd, TABLE *table) > > if (need_purge) > { > - TABLE_SHARE *purge_share= 0; > - TABLE_SHARE *share; > - TABLE *entry; > - ulonglong UNINIT_VAR(purge_time); > - TDC_iterator tdc_it; > + tc_add_table_arg argument; > + argument.purge_time= ULONGLONG_MAX; > + tdc_iterate(thd, (my_hash_walk_action) tc_add_table_callback, &argument); > > - tdc_it.init(); > - while ((share= tdc_it.next())) > + if (argument.purge_time != ULONGLONG_MAX) > { > - mysql_mutex_lock(&share->tdc.LOCK_table_share); > - if ((entry= tc_free_tables_back(share)) && > - (!purge_share || entry->tc_time < purge_time)) > + TDC_element *element= (TDC_element*) lf_hash_search(&tdc_hash, > + thd->tdc_hash_pins, > + argument.key, > + argument.key_length); > + if (element) > { > - purge_share= share; > - purge_time= entry->tc_time; > - } > - mysql_mutex_unlock(&share->tdc.LOCK_table_share); > - } > + TABLE *entry; > + mysql_mutex_lock(&element->LOCK_table_share); > + lf_hash_search_unpin(thd->tdc_hash_pins); > + element->wait_for_mdl_deadlock_detector(); you wait under a mutex? hmm... > > - if (purge_share) > - { > - mysql_mutex_lock(&purge_share->tdc.LOCK_table_share); > - tc_wait_for_mdl_deadlock_detector(purge_share); > - tdc_it.deinit(); > /* > It may happen that oldest table was acquired meanwhile. In this case > just go ahead, number of objects in table cache will normalize > eventually. > */ > - if ((entry= tc_free_tables_back(purge_share)) && > - entry->tc_time == purge_time) > + if ((entry= element->free_tables_back()) && > + entry->tc_time == argument.purge_time) > { > - entry->s->tdc.free_tables.remove(entry); > + element->free_tables.remove(entry); > tc_remove_table(entry); > - mysql_mutex_unlock(&purge_share->tdc.LOCK_table_share); > + mysql_mutex_unlock(&element->LOCK_table_share); > intern_close_table(entry); > } > else > - mysql_mutex_unlock(&purge_share->tdc.LOCK_table_share); > + mysql_mutex_unlock(&element->LOCK_table_share); > } > - else > - tdc_it.deinit(); > } > } > - > - > -/** > - Acquire TABLE object from table cache. > - > - @pre share must be protected against removal. > - > - Acquired object cannot be evicted or acquired again. > - > - While locked: > - - pop object from TABLE_SHARE::tdc.free_tables > - > - While unlocked: > - - mark object used by thd > - > - @return TABLE object, or NULL if no unused objects. > -*/ > - > -static TABLE *tc_acquire_table(THD *thd, TABLE_SHARE *share) > -{ > - TABLE *table; > - > - mysql_mutex_lock(&share->tdc.LOCK_table_share); > - table= share->tdc.free_tables.pop_front(); > - if (table) > - { > - DBUG_ASSERT(!table->in_use); > - table->in_use= thd; > - /* The ex-unused table must be fully functional. */ > - DBUG_ASSERT(table->db_stat && table->file); > - /* The children must be detached from the table. */ > - DBUG_ASSERT(!table->file->extra(HA_EXTRA_IS_ATTACHED_CHILDREN)); > - } ok, you've moved it to table_cache.h... > - mysql_mutex_unlock(&share->tdc.LOCK_table_share); > - return table; > } > > > @@ -397,88 +357,76 @@ bool tc_release_table(TABLE *table) > */ > table->in_use= 0; > /* Add table to the list of unused TABLE objects for this share. */ > - table->s->tdc.free_tables.push_front(table); > - mysql_mutex_unlock(&table->s->tdc.LOCK_table_share); > + table->s->tdc->free_tables.push_front(table); > + mysql_mutex_unlock(&table->s->tdc->LOCK_table_share); > return false; > > purge: > - tc_wait_for_mdl_deadlock_detector(table->s); > + table->s->tdc->wait_for_mdl_deadlock_detector(); > tc_remove_table(table); > - mysql_mutex_unlock(&table->s->tdc.LOCK_table_share); > + mysql_mutex_unlock(&table->s->tdc->LOCK_table_share); > table->in_use= 0; > intern_close_table(table); > return true; > } > > > -extern "C" uchar *tdc_key(const uchar *record, size_t *length, > - my_bool not_used __attribute__((unused))) > -{ > - TABLE_SHARE *entry= (TABLE_SHARE*) record; > - *length= entry->table_cache_key.length; > - return (uchar*) entry->table_cache_key.str; > -} > - > - > /** > Delete share from hash and free share object. > - > - @return > - @retval 0 Success > - @retval 1 Share is referenced > */ > > -static int tdc_delete_share_from_hash(TABLE_SHARE *share) > +static void tdc_delete_share_from_hash(TDC_element *element) > { > + THD *thd= current_thd; > + LF_PINS *pins; > + TABLE_SHARE *share; > DBUG_ENTER("tdc_delete_share_from_hash"); > - mysql_rwlock_wrlock(&LOCK_tdc); > - mysql_mutex_lock(&share->tdc.LOCK_table_share); > - if (--share->tdc.ref_count) > - { > - mysql_cond_broadcast(&share->tdc.COND_release); > - mysql_mutex_unlock(&share->tdc.LOCK_table_share); > - mysql_rwlock_unlock(&LOCK_tdc); > - DBUG_RETURN(1); > - } > - my_hash_delete(&tdc_hash, (uchar*) share); > + > + share= element->share; > + DBUG_ASSERT(share); > + element->share= 0; > /* Notify PFS early, while still locked. */ "while still locked" what? > PSI_CALL_release_table_share(share->m_psi); > share->m_psi= 0; > - mysql_rwlock_unlock(&LOCK_tdc); > > - if (share->tdc.m_flush_tickets.is_empty()) > + if (!element->m_flush_tickets.is_empty()) > { > - /* No threads are waiting for this share to be flushed, destroy it. */ > - mysql_mutex_unlock(&share->tdc.LOCK_table_share); > - free_table_share(share); > - } > - else > - { > - Wait_for_flush_list::Iterator it(share->tdc.m_flush_tickets); > + Wait_for_flush_list::Iterator it(element->m_flush_tickets); > Wait_for_flush *ticket; > while ((ticket= it++)) > (void) ticket->get_ctx()->m_wait.set_status(MDL_wait::GRANTED); > - /* > - If there are threads waiting for this share to be flushed, > - the last one to receive the notification will destroy the > - share. At this point the share is removed from the table > - definition cache, so is OK to proceed here without waiting > - for this thread to do the work. > - */ > - mysql_mutex_unlock(&share->tdc.LOCK_table_share); > + > + do > + { > + mysql_cond_wait(&element->COND_release, &element->LOCK_table_share); > + } while (!element->m_flush_tickets.is_empty()); > } > - DBUG_RETURN(0); > + > + mysql_mutex_unlock(&element->LOCK_table_share); Ok, so the caller must lock LOCK_table_share? please put mysql_mutex_assert_owner() in the beginning of the function > + > + if (thd) When thd is NULL here? On shutdown? > + { > + fix_thd_pins(thd); > + pins= thd->tdc_hash_pins; > } > + else > + pins= lf_hash_get_pins(&tdc_hash); > > + DBUG_ASSERT(pins); // What can we do about it? > + element->dbug_release_assertions(); > + lf_hash_delete(&tdc_hash, pins, element->m_key, element->m_key_length); > + if (!thd) > + lf_hash_put_pins(pins); > + free_table_share(share); > + DBUG_VOID_RETURN; > +} > > + > /** > Initialize table definition cache. > - > - @retval 0 Success > - @retval !0 Error > */ > > -int tdc_init(void) > +void tdc_init(void) > { > DBUG_ENTER("tdc_init"); > #ifdef HAVE_PSI_INTERFACE > @@ -642,26 +548,34 @@ void tdc_deinit_share(TABLE_SHARE *share) > Caller is expected to unlock table share with tdc_unlock_share(). > > @retval 0 Share not found > - @retval !0 Pointer to locked table share > + @retval MY_ERRPTR OOM > + @retval ptr Pointer to locked table share > */ > > -TABLE_SHARE *tdc_lock_share(const char *db, const char *table_name) > +TDC_element *tdc_lock_share(THD *thd, const char *db, const char *table_name) > { > + TDC_element *element; > char key[MAX_DBKEY_LENGTH]; > - uint key_length; > > DBUG_ENTER("tdc_lock_share"); > - key_length= tdc_create_key(key, db, table_name); > + if (fix_thd_pins(thd)) > + DBUG_RETURN((TDC_element*) MY_ERRPTR); I suppose you can do it in THD:init, if you'd like. A THD almost always will need to access table definition cache, so it'll need these pins. Then there will be no need to return MY_ERRPTR here and handle it in the caller > > - mysql_rwlock_rdlock(&LOCK_tdc); > - TABLE_SHARE* share= (TABLE_SHARE*) my_hash_search(&tdc_hash, > - (uchar*) key, key_length); > - if (share && !share->error) > - mysql_mutex_lock(&share->tdc.LOCK_table_share); > - else > - share= 0; > - mysql_rwlock_unlock(&LOCK_tdc); > - DBUG_RETURN(share); > + element= (TDC_element *) lf_hash_search(&tdc_hash, thd->tdc_hash_pins, > + (uchar*) key, > + tdc_create_key(key, db, table_name)); > + if (element) > + { > + mysql_mutex_lock(&element->LOCK_table_share); > + if (!element->share || element->share->error) > + { > + mysql_mutex_unlock(&element->LOCK_table_share); > + element= 0; > + } > + lf_hash_search_unpin(thd->tdc_hash_pins); > + } > + > + DBUG_RETURN(element); Looks good to me > } > > > @@ -702,60 +616,60 @@ TABLE_SHARE *tdc_acquire_share(THD *thd, const char *db, const char *table_name, > TABLE **out_table) > { > TABLE_SHARE *share; > + TDC_element *element; > bool was_unused; > DBUG_ENTER("tdc_acquire_share"); > > - mysql_rwlock_rdlock(&LOCK_tdc); > - share= (TABLE_SHARE*) my_hash_search_using_hash_value(&tdc_hash, hash_value, > - (uchar*) key, > - key_length); > - if (!share) > + if (fix_thd_pins(thd)) > + DBUG_RETURN(0); > + > +retry: > + while (!(element= (TDC_element*) lf_hash_search_using_hash_value(&tdc_hash, Was lf_hash_search_using_hash_value added in a separate patch? > + thd->tdc_hash_pins, hash_value, (uchar*) key, key_length))) > { > - TABLE_SHARE *new_share; > - mysql_rwlock_unlock(&LOCK_tdc); > + TDC_element tmp(key, key_length); > + int res= lf_hash_insert(&tdc_hash, thd->tdc_hash_pins, (uchar*) &tmp); > > - if (!(new_share= alloc_table_share(db, table_name, key, key_length))) > + if (res == -1) > DBUG_RETURN(0); > - new_share->error= OPEN_FRM_OPEN_ERROR; > + else if (res == 1) > + continue; > > - mysql_rwlock_wrlock(&LOCK_tdc); > - share= (TABLE_SHARE*) my_hash_search_using_hash_value(&tdc_hash, hash_value, > - (uchar*) key, > - key_length); > - if (!share) > - { > - bool need_purge; > + element= (TDC_element*) lf_hash_search_using_hash_value(&tdc_hash, > + thd->tdc_hash_pins, hash_value, (uchar*) key, key_length); > + lf_hash_search_unpin(thd->tdc_hash_pins); How can you be sure that nobody frees your element after you unpin it? > + DBUG_ASSERT(element); > + element->dbug_release_assertions(); > > - share= new_share; > - mysql_mutex_lock(&share->tdc.LOCK_table_share); > - if (my_hash_insert(&tdc_hash, (uchar*) share)) > + if (!(share= alloc_table_share(db, table_name, key, key_length))) > { > - mysql_mutex_unlock(&share->tdc.LOCK_table_share); > - mysql_rwlock_unlock(&LOCK_tdc); > - free_table_share(share); > + lf_hash_delete(&tdc_hash, thd->tdc_hash_pins, key, key_length); > DBUG_RETURN(0); > } > - need_purge= tdc_hash.records > tdc_size; > - mysql_rwlock_unlock(&LOCK_tdc); > > /* note that tdc_acquire_share() *always* uses discovery */ > open_table_def(thd, share, flags | GTS_USE_DISCOVERY); > - share->tdc.ref_count++; > - mysql_mutex_unlock(&share->tdc.LOCK_table_share); > > if (share->error) > { > - tdc_delete_share_from_hash(share); > + free_table_share(share); > + lf_hash_delete(&tdc_hash, thd->tdc_hash_pins, key, key_length); > DBUG_RETURN(0); > } > - else if (need_purge) > + > + mysql_mutex_lock(&element->LOCK_table_share); > + element->share= share; > + share->tdc= element; > + element->ref_count++; > + element->version= tdc_refresh_version(); > + element->flushed= false; > + mysql_mutex_unlock(&element->LOCK_table_share); > + > tdc_purge(false); > if (out_table) > *out_table= 0; > share->m_psi= PSI_CALL_get_table_share(false, share); > goto end; > - } > - free_table_share(new_share); > } > > /* cannot force discovery of a cached share */ > @@ -763,18 +677,25 @@ TABLE_SHARE *tdc_acquire_share(THD *thd, const char *db, const char *table_name, > > if (out_table && (flags & GTS_TABLE)) > { > - if ((*out_table= tc_acquire_table(thd, share))) > + if ((*out_table= element->acquire_table(thd))) > { > - mysql_rwlock_unlock(&LOCK_tdc); > + lf_hash_search_unpin(thd->tdc_hash_pins); what this lf_hash_search_unpin corresponds to? > DBUG_ASSERT(!(flags & GTS_NOLOCK)); > - DBUG_ASSERT(!share->error); > - DBUG_ASSERT(!share->is_view); > - DBUG_RETURN(share); > + DBUG_ASSERT(element->share); > + DBUG_ASSERT(!element->share->error); > + DBUG_ASSERT(!element->share->is_view); > + DBUG_RETURN(element->share); > } > } > > - mysql_mutex_lock(&share->tdc.LOCK_table_share); > - mysql_rwlock_unlock(&LOCK_tdc); > + mysql_mutex_lock(&element->LOCK_table_share); > + if (!(share= element->share)) > + { > + mysql_mutex_unlock(&element->LOCK_table_share); > + lf_hash_search_unpin(thd->tdc_hash_pins); and here > + goto retry; > + } > + lf_hash_search_unpin(thd->tdc_hash_pins); may be that first lf_hash_search_unpin was wrong? the one that goes directly after lf_hash_search. > > /* > We found an existing table definition. Return it if we didn't get > @@ -983,13 +864,36 @@ bool tdc_remove_table(THD *thd, enum_tdc_remove_table_type remove_type, > thd->mdl_context.is_lock_owner(MDL_key::TABLE, db, table_name, > MDL_EXCLUSIVE)); > > - if ((share= tdc_delete_share(db, table_name))) > + > + mysql_mutex_lock(&LOCK_unused_shares); > + if (!(element= tdc_lock_share(thd, db, table_name))) > { > - I_P_List <TABLE, TABLE_share> purge_tables; > - uint my_refs= 1; > + mysql_mutex_unlock(&LOCK_unused_shares); > + DBUG_ASSERT(remove_type != TDC_RT_REMOVE_NOT_OWN_KEEP_SHARE); > + DBUG_RETURN(false); > + } > > - mysql_mutex_lock(&share->tdc.LOCK_table_share); > - tc_wait_for_mdl_deadlock_detector(share); > + DBUG_ASSERT(element != MY_ERRPTR); // What can we do about it? allocate thd->tdc_hash_pins in THD::init > + > + if (!element->ref_count) > + { > + if (element->prev) > + { > + unused_shares.remove(element); > + /* Concurrent thread may start using share again, reset prev and next. */ What concurrent thread? element->ref_count is 0, you removed the the share from the unused_shares list. There can be no concurrent threads, as far as I understand. And indeed, you do remove the share from the hash (which does free_table_share) immediately below, which also proves that there can be no concurrent threads. > + element->prev= 0; > + element->next= 0; > + } > + mysql_mutex_unlock(&LOCK_unused_shares); > + > + tdc_delete_share_from_hash(element); > + DBUG_RETURN(true); > + } > + mysql_mutex_unlock(&LOCK_unused_shares); > + > + element->ref_count++; > + > + element->wait_for_mdl_deadlock_detector(); > /* > Mark share flushed in order to ensure that it gets > automatically deleted once it is no longer referenced. Regards, Sergei [View Less]

2 4

[Maria-developers] ccache with mariadb?
by Otto Kekäläinen 28 Dec '14

28 Dec '14

Hello! Has somebody managed to configure ccache correctly so that it would work with MariaDB compilation? https://ccache.samba.org/ I don't seem to get it working as the build always seems to pick up the real gcc binary instead of any of the ccache symlink wrappers I've manually put in place to replace all of gcc, cc1, cc1plus etc.

2 3

Re: [Maria-developers] MDEV-7273 - 10.1 fails to start up during tc_log initializations on PPC64
by Sergei Golubchik 26 Dec '14

26 Dec '14

Hi, Sergey! On Dec 26, svoj(a)mariadb.org wrote: > revision-id: f65901eef28728212a350f61569dc884c747bffc > parent(s): 8c616cd347aa7b3fcc2d8defb5e172ad316f432c > committer: Sergey Vojtovich > branch nick: 10.1 > timestamp: 2014-12-26 23:38:45 +0400 > message: > > MDEV-7273 - 10.1 fails to start up during tc_log initializations on PPC64 > > log-tc-size is 24K by default. Page size is 64K on PPC64. But log-tc-size > must be at least 3 x page size. This is … [View More]

2 1

Re: [Maria-developers] MDEV-7273 - 10.1 fails to start up during tc_log initializations on PPC64
by Sergei Golubchik 26 Dec '14

26 Dec '14

Hi, Sergey! Yes, that's what I had in mind, thanks! There are few rough edges, see below. Ok to push after you fix that, there's no need for another review round. On Dec 26, svoj(a)mariadb.org wrote: > revision-id: 72e58dc2b1a0bb859e918de0ff820d5a2a92dc32 > parent(s): db89dd3a8f7b0d868946d25ba98c6f88612d309a > committer: Sergey Vojtovich > branch nick: 10.1 > timestamp: 2014-12-26 17:03:45 +0400 > message: > > MDEV-7273 - 10.1 fails to start up during tc_log … [View More]initializations on PPC64 > > log-tc-size is 24K by default. Page size is 64K on PPC64. But log-tc-size > must be at least 3 x page size. This is enforced by TC_LOG_MMAP::open() > with a comment: to guarantee non-empty pool. > > This all makes server not startable in default configuration on PPC64. > > Autosize log-tc-size, so that it's min value= page size * 6, default > value= page size * 6, block size= page size. > > --- > mysql-test/r/mysqld--help.result | 1 - > mysql-test/suite/sys_vars/r/log_tc_size_basic.result | 4 ++++ > .../suite/sys_vars/r/sysvars_server_notembedded.result | 14 ++++++++++++++ > mysql-test/suite/sys_vars/t/log_tc_size_basic.test | 5 +++++ > mysql-test/t/mysqld--help.test | 2 +- > sql/log.cc | 5 ++--- > sql/log.h | 3 +-- > sql/mysqld.cc | 7 +------ > sql/sys_vars.cc | 11 +++++++++++ > 9 files changed, 39 insertions(+), 13 deletions(-) > > diff --git a/mysql-test/suite/sys_vars/t/log_tc_size_basic.test b/mysql-test/suite/sys_vars/t/log_tc_size_basic.test > new file mode 100644 > index 0000000..1fce212 > --- /dev/null > +++ b/mysql-test/suite/sys_vars/t/log_tc_size_basic.test > @@ -0,0 +1,5 @@ > +--error ER_INCORRECT_GLOBAL_LOCAL_VAR,ER_UNKNOWN_SYSTEM_VARIABLE > +SET GLOBAL log_tc_size=1; > + > +--error ER_INCORRECT_GLOBAL_LOCAL_VAR,ER_UNKNOWN_SYSTEM_VARIABLE do we ever build on platforms without mmap? if the answer is "no" - you don't need ER_UNKNOWN_SYSTEM_VARIABLE here > +SET SESSION log_tc_size=1; > diff --git a/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result b/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result > index 006b9a9..5491169 100644 > --- a/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result > +++ b/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result > @@ -1757,6 +1757,20 @@ NUMERIC_BLOCK_SIZE NULL > ENUM_VALUE_LIST innodb,query_plan,explain > READ_ONLY NO > COMMAND_LINE_ARGUMENT REQUIRED > +VARIABLE_NAME LOG_TC_SIZE > +SESSION_VALUE NULL > +GLOBAL_VALUE 24576 > +GLOBAL_VALUE_ORIGIN AUTO > +DEFAULT_VALUE 24576 > +VARIABLE_SCOPE GLOBAL > +VARIABLE_TYPE BIGINT UNSIGNED > +VARIABLE_COMMENT Size of transaction coordinator log. > +NUMERIC_MIN_VALUE 24576 > +NUMERIC_MAX_VALUE 18446744073709551615 > +NUMERIC_BLOCK_SIZE 4096 > +ENUM_VALUE_LIST NULL > +READ_ONLY YES > +COMMAND_LINE_ARGUMENT REQUIRED if the answer is "yes" - this test will fail there. even if the answer is "no", the variable value is architecture dependent. But it's good to see all stable variable metadata (e.g. that GLOBAL_VALUE_ORIGIN is AUTO). > VARIABLE_NAME LOG_WARNINGS > SESSION_VALUE 1 > GLOBAL_VALUE 1 > diff --git a/sql/log.cc b/sql/log.cc > index 66e1426..6d6512e 100644 > --- a/sql/log.cc > +++ b/sql/log.cc > @@ -8793,7 +8792,7 @@ int TC_LOG_MMAP::unlog(ulong cookie, my_xid xid) > mysql_mutex_lock(&LOCK_pending_checkpoint); > if (pending_checkpoint == NULL) > { > - uint32 size= sizeof(*pending_checkpoint); > + uint32 size= sizeof(*pending_checkpoint) + tc_log_page_size / sizeof(my_xid); that's not enough. There are further sizeof's, like if (pending_checkpoint->count == sizeof(pending_checkpoint->cookies) / sizeof(pending_checkpoint->cookies[0])) and for (i= 0; i < sizeof(pending->cookies)/sizeof(pending->cookies[0]); ++i) I've found these two by looking for "sizeof" in the Kristian's patch that introduced pending_checkpoint. There can be more sizeofs added later. > if (!(pending_checkpoint= > (pending_cookies *)my_malloc(size, MYF(MY_ZEROFILL)))) > { > diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc > index fb11377..bfaf7d7 100644 > --- a/sql/sys_vars.cc > +++ b/sql/sys_vars.cc > @@ -5202,3 +5202,14 @@ static Sys_var_mybool Sys_strict_password_validation( > "that cannot be validated (passwords specified as a hash)", > GLOBAL_VAR(strict_password_validation), > CMD_LINE(OPT_ARG), DEFAULT(TRUE), NO_MUTEX_GUARD, NOT_IN_BINLOG); > + > +#ifdef HAVE_MMAP > +static Sys_var_ulong Sys_log_tc_size( > + "log_tc_size", > + "Size of transaction coordinator log.", > + READ_ONLY GLOBAL_VAR(opt_tc_log_size), > + CMD_LINE(REQUIRED_ARG), > + VALID_RANGE(my_getpagesize() * 6, ULONG_MAX), > + DEFAULT(my_getpagesize() * 6), > + BLOCK_SIZE(my_getpagesize())); I think, min value could be my_getpagesize()*3 ? > +#endif Regards, Sergei [View Less]

2 1

Re: [Maria-developers] MDEV-7273 - 10.1 fails to start up during tc_log initializations on PPC64
by Sergei Golubchik 25 Dec '14

25 Dec '14

Hi, Sergey! On Dec 17, svoj(a)mariadb.org wrote: > revision-id: 4a469b0ed68bb341e1a1e1ba540a45a3b6ff3ef9 > parent(s): ea8862aa9ffa1cfac9a99bbe0fe9b539f12b9a19 > committer: Sergey Vojtovich > branch nick: 10.1 > timestamp: 2014-12-17 13:55:01 +0400 > message: > > MDEV-7273 - 10.1 fails to start up during tc_log initializations on PPC64 > > log-tc-size is 24K by default. Page size is 64K on PPC64. But log-tc-size > must be at least 3 x page size. This is … [View More]

1 0