From: Sergey Vojtovich
Date: November 19 2010 5:36pm
Subject: Re: bzr commit into mysql-next-mr-bugfixing branch
 (magne.mahre:3328) Bug#49177
List-Archive: http://lists.mysql.com/commits/124503
Message-Id: <20101119173613.GA11073@june>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Transfer-Encoding: 8bit

Magne,

first of all it is a question. Not request, not even a
suggestion.

(heh, nice rhyme)

When I looked at this patch for the first time I wasn't
aware of WL#4305. At that time your patch was the only
viable fix. Now, I think we should consider WL#4305.

The core problem you're fixing is performance bottleneck on
mi_open().

Said the above, what I mean when I say "less intrusive":
- keep list;
- search hash only on open;
- on open: add new hash entry;
- on close: remove hash entry;
- on panic: free hash;
- probably failover to list if hash is broken;
- probably consider hash broken if myisam is used not by the server.

Mine arguments for less intrusive patch are: it should be safer, it
should be easier to remove when we do WL#4305 based solution.

Mine arguments against less intrusive patch are: current patch needs
to be adjusted (probably 1-2 days of coding).

Regards,
Sergey

On Fri, Nov 19, 2010 at 05:52:24PM +0100, Magne Mæhre wrote:
> On 11/19/2010 05:22 PM, Sergey Vojtovich wrote:
> > Magne,
> > 
> > ok, do you think it is worth to consider that we may rework
> > this part sooner or later and make less intrusive patch?
> 
> Hi Sergey / Mattias
> 
> I'm not entirely sure I follow you.   What the patch does is
> simply to exchange one data structure with another.  Since the
> container classes in mysql doesn't provide a common interface,
> there is a need to change some code.  Most of the code change
> is in peripheral functions (mi_panic, debugging, etc.), where
> there is a need to traverse all stored objects.  I also had to
> do some extra initialization, since the MyISAM code is also used
> directly from supporting tools, which does not use the
> handlerton.
> 
> I feel that this solution is fairly good, as a trade-off
> between speed, complexity and implementation risk.  It is
> entirely possible to get an even faster lookup mechanism
> than this, but I feel it's not worth the extra effort.  The
> benchmarking we did, show a tremendous(!) improvement over the
> old implementation (see my _private_ entry on the bug report
> page, dated Sep. 30th)
> 
> Since WL#4305 isn't, to my knowledge, started on yet, it means
> the impact on that work is minimal.  What is absolutely certain
> is that we won't be using the old iterate-and-compare-name-string
> approach in that worklog, so I believe something like my patch
> will naturally find its place in that code.
> 
> Of course, I'm open to suggestions on how to improve the code,
> and other approaches.  I am, however, reluctant to have code
> living in my sandbox, while waiting for a worklog that has
> unknown priority, and might even be stopped (not that I
> have any indications on that happening :)
> 
> So - to sum it up --  I'm, of course, very eager to get this
> patch in.  I'm, on the other hand, ready to accept it if it is
> thought to be a bad idea :)
> 
> --Magne

-- 
Sergey Vojtovich <svoj@stripped>
MySQL AB, Software Engineer
Izhevsk, Russia, www.mysql.com