3 Feb
2014
3 Feb
'14
8:11 p.m.
Hi Sergei,
On Mon, Feb 03, 2014 at 05:22:50PM +0100, Sergei Golubchik wrote:
> Hi, Sergey!
>
> On Jan 31, Sergey Vojtovich wrote:
> > >
> > > I'm sorry, I don't understand that at all.
> > >
> > > Why my_refs is only incremented in debug builds? In non-debug builds you
> > > wait until all tables but one are closed, in debug builds you may keep
> > > more than one table opened.
> > See above, this patch removes #ifndef DBUG_OFF, so my_refs are counted properly
> > in both builds.
>
> Right. Sorry. My mistake.
>
> > > You wait here on a condition, how do you know there's FLUSH TABLES (or
> > > anything at all) running that will signal this condition?
> > 1. we're protected by exclusive metadata lock, so the only possible statement
> > that may cause concurrency issues should be FLUSH TABLES (in fact there are
> > more cases, like ha_table_exists()).
> > 2. share->tdc.all_tables holds only tables owned by current thread (or empty),
> > there are assertions for this above.
> > 3. if concurrent FLUSH TABLES removed some TABLE instances from tdc.all_tables,
> > but didn't close them yet, share ref_count reflects that.
> > 4. this FLUSH TABLES must call intern_close_table(), which will eventually call
> > tdc_release_share()/mysql_cond_broadcast().
> >
> > That's it.
>
> Okay, thanks. Got it.
> What did you mean "like ha_table_exists()" ?
FWICS ha_table_exists() may be called without prior acquisition of MDL, e.g. by
fill_schema_table_names() (SHOW TABLES). It does the following:
<code>
TABLE_SHARE *share= tdc_lock_share(db, table_name); // <LOCK_table_share>ref_count++</LOCK_table_share>
if (share)
{
if (hton)
*hton= share->db_type();
tdc_unlock_share(share); // <LOCK_table_share>ref_count--</LOCK_table_share>
DBUG_RETURN(TRUE);
}
</code>
Note that it locks and then immediately unlocks table share, in other words
condition signal is guaranteed. In fact we don't need to touch ref_count
here - all we need is hash lookup (there is no API for that yet). I'll add
this idea to MDEV-5471.
Regards,
Sergey