maria-developers team mailing list archive

[Sergey Vojtovich <svoj@xxxxxxxxxxx>]

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