maria-developers team mailing list archive

[Sergey Vojtovich <svoj@xxxxxxxxxxx>]

Hi Sergei,

thanks for looking into this patch. Frankly speaking I find it a bit
questionable too. Below are links that should answer your questions...
What problem do I attempt to solve: https://lists.launchpad.net/maria-developers/msg06118.html
How do I attempt to solve it: https://mariadb.atlassian.net/browse/MDEV-4956

32 connections issue simple SELECT against one table. Server has 4 CPU
(32 cores + 32 HyperThreads).

For every statment we acquire table from table cache and then release table back
to the cache. That involves update of 3 lists: unused_tables, per-share
used_tables and free_tables. These lists are protected by LOCK_open
(see tc_acquire_table() and tc_release_table()).

Every time we update global pointer, corresponding cache lines of sibling CPUs
have to be invalidated. This is causing expensive memory reads while LOCK_open
is held.

Oracle solved this problem by partitioning table cache, allowing emulation of
something like per-CPU lists.

We attempted to split LOCK_open logically, and succeeded at everything but these
3 lists. I attempted lock-free list for free_tables, but TPS rate didn't improve.

What we need is to reduce number of these expensive memory reads, and there
are two solutions: partition these lists or get rid of them. As we agreed not
to partition, I'm trying the latter solution.

Why I find this patch questionable? It reduces LOCK_open wait time by 30%, to
get close to Oracle wait time, we need to reduce wait time by 90%. We could
remove unused_tables as well, but it will be 60% not 90%.

Thanks,
Sergey

On Mon, Sep 09, 2013 at 06:14:13PM +0200, Sergei Golubchik wrote:
> Hi, Sergey!
> 
> On Aug 27, Sergey Vojtovich wrote:
> > At lp:maria/10.0
> > 
> > ------------------------------------------------------------
> > revno: 3807
> > revision-id: svoj@xxxxxxxxxxx-20130827121233-xh1uyhgfwbhedqyf
> > parent: jplindst@xxxxxxxxxxx-20130823060357-pww92qxla7o8iir7
> > committer: Sergey Vojtovich <svoj@xxxxxxxxxxx>
> > branch nick: 10.0
> > timestamp: Tue 2013-08-27 16:12:33 +0400
> > message:
> >   MDEV-4956 - Reduce usage of LOCK_open: TABLE_SHARE::tdc.used_tables
> >   
> >   - tc_acquire_table and tc_release_table do not access
> >     TABLE_SHARE::tdc.used_tables anymore
> >   - in tc_acquire_table(): release LOCK_tdc after we relase LOCK_open
> >     (saves a few CPU cycles in critical section)
> >   - in tc_release_table(): if we reached table cache threshold, evict
> >     to-be-released table without moving it to unused_tables. unused_tables
> >     must be empty at this point (added assertion).
> 
> I don't understand what you're doing here, could you elaborate?
> E.g. explain in a changeset comment what you've done, why you introduced
> a new list for tables (all_share), what is its semantics, etc.
> 
> Regards,
> Sergei