← Back to team overview

maria-developers team mailing list archive

Re: [Commits] b2f45ba: MDEV-7324 - Lock-free hash for table definition cache

 

Hi Sergei,

On Sun, Dec 28, 2014 at 06:08:07PM +0100, Sergei Golubchik wrote:
...skip...

> > > >  #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?
> > These need to be visible by TDC_element::lf_alloc_constructor. Their static
> > status is to be restored later.
> 
> And if you make them static they won't be visible from
> TDC_element::lf_alloc_constructor?
Yes, just tried that.

> 
> > > > @@ -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?
> > Element found, but it may be to-be-deleted element or not-yet-open element.
> > In both cases free_tables must be empty. This is also guarded by the following
> > assert: DBUG_RETURN(element->share).
> 
> I mean, what lf_hash_search invocation does that unpin corresponds to.
> I didn't see any lf_hash_search that wouldn't have a matching unpin.
> So this one (and other below) look "homeless"
There're 2 searches and 4 unpins in tdc_acquire_share. Let's simplify it a bit:

retry:
  while (!search1())
  {
    insert();
    search2();
    unpin2();
    return;
  }

  if (fastpath_suceeded())
  {
    unpin1(); // This is unpin in question, corresponds to search1
    return;
  }

  if (deleted)
  {
    unpin1();
    goto retry;
  }
  unpin1();

Thanks,
Sergey


Follow ups

References