Re: bug in zcatalog queryplan causing very slow first query after startup

On 17/10/2013, at 7:39 PM, Hanno Schlichting <[email protected]> wrote:

> Dylan Jay <djay@...> writes:
>> Discovered a nasty-ish bug in the zcatalog the other day which I need some
>> advice fixing. During startup zope will work out it's query plan including
>> evaluating each index to see which is VALUE_INDEX. It's looking for indexes
>> with uniquevalues of < 10. Unfortunately if you have a large primary key
>> type index of say 10k values then this will do a lot of network calls and
>> take over an hour. The bug is that it really doesn't need all the keys,
>> just the number of unique ones.
>
> Nice catch! I wonder why nobody else ran into this before. It should be a
> common problem.

True. Maybe same reason we worked it out late. Our production Db grew
and it wasn't till we moved the Db back to uat that we noticed. We
also use pretaweb.healthcheck to prime our site.
I also found some references on the net to preloading queryplans so
maybe others just worked around it like us.

>
>> My options are to either
>> 1. Change the UnIndex implementation to be lazy so the sequence returned
>> can be used to determine the length of the keys without returning all the
>> keys. This would only fix the problem for UnIndex implementations
>> but I think that would cover most cases.
>
> This is the approach I tried on ZCatalog master.
>
> The uniqueValues interface description is pretty vague. While looking at the
> different indexes, I noticed the PathIndex returning a generator for this
> method, while others where returning tuples or lists. I took this as enough
> evidence to suggest using generators for all the other indexes would also
> be in-line with the interface description.
>
> I clarified the description and changed the other indexes to generators.
>
> On the queryplan itself, I changed the logic to avoid calling `len()` and
> instead just try to iterate over the results, until I exceed
> MAX_DISTINCT_VALUES (default: 10) items. If there are at least 11 entries,
> than the index is no longer considered a value index. It doesn't matter if
> its 12 or 10000 entries.
>
> I think this approach should work and the changes are likely backwards
> compatible with most code out there. I had to adjust some tests, as calling
> `len(index.uniqueValues())` is no longer possible. One has to cast the
> generator to a list or tuple before being able to get a length. To minimize
> the risk, I've changed the `uniqueValuesFor` method on the Catalog class,
> to do the tuple-casting. This method is a more public API and more likely
> to be used in projects. It should be rare for anyone to call a method directly
> on one of the index classes.
>
> I'd appreciate some feedback on the approach and whether you think this is
> backwards compatible enough to merge it into the 2.13 maintenance branch
> (someone could run the Plone tests with this...).

Awesome. That sounds like a sensible approach. I also noticed the
description of the interface was vague but no idea on compatibility.
Thanks for the fix and I hope it gets merged soon.

>
>> I would also like to make a fix to make queryplans persistent into the ZODB
>> which would further help to prevent slow queries on startup. Does anyone
>> know this wasn't done in the originally?
>
> To provide some context: SQL databases have background worker processes which
> maintain separate statistics tables, primarily gathering information about
> index presence and data length statistics. This process happens outside the
> normal select/insert/delete work and thus doesn't impact app performance much
> and on the other hand can do some more heavy handed work in the background.
>
> For Zope/Plone this poses two problems. For one there is no standard way to
> have a single background process run on a per database basis. The best we can
> do is per-instance background threads like the one used in plone.cachepurging.
> Unless one introduces a hard dependency on a task queue / async worker system.
> I didn't want to either to Zope itself.
>
> And even if one would do this, this process would have to load enormous data
> sets from the server to determine the data lengths, as BTrees don't provide
> length information about themselves, unless one reads them completely on the
> client side. One could maintain additional BTrees.Length counters for all
> BTrees as part of the normal insert/delete work, but this would increase
> conflict errors and thus quickly have a negative impact.
>
> The second problem is that this assumes a correlation between data lengths
> and query runtime. For SQL servers where all the work happens on the server
> side this is a good model.
>
> But the ZCatalog actually does almost all the work on the ZEO client side.
> The dominant factor for query performance isn't the length of any of the
> involved data sets, but the ratio between how much of that data has to be
> loaded from the server and how much is already in the connection or ZEO
> caches.
>
> One could assume all connection caches to be filled uniformly, equally
> sized and all Zope instances running on equal hardware / software
> configurations. If that were true, than the query plan for any instance would
> be a good fit for all the other ones. Unfortunately those aren't valid
> assumptions in most mildly complex setups. And in trivial setups the query
> plan doesn't really matter as things are fast enough.
>
> In more complex setups people choose to dedicate instances to special
> functions like serving anonymous traffic vs. logged-in users, serving theme
> resources or serving web crawlers. And even if this isn't done, than each
> cache will be different depending on what kind of requests it has served in
> the past.
>
> So while in SQL databases global server-side statistics make a lot of sense,
> for the catalog per connection/app thread statistics are a much better fit.
> And these stats aren't just dependent on the data itself, but much more so on
> outside factors, like cache sizes, load balancer settings, current web traffic
> patterns and client-side hardware/software environments.
>
> With all that said, maintaining query plans across instance restarts has much
> less value than one would assume. The best one can do is to create some
> warm-up process to prefill the connection cache before allowing any newly
> started instance to serve web traffic.
>
> Hope this is remotely comprehensible :-)

Yes I see what your saying, different threads end up with different
query plans so its not clear which to save or if its a good idea.
On the other hand the work that has to be done now to preload a
queryplan when you want by creating and deploying an egg is PITA. One
option is a manual button to save the plan and maybe edit it. However
at the moment the queryplan us global. Even without edit this is a
security concern since a manager of one site can know information of
all sites on a zope platform. We always run Plone multi site so this
is something we'd like to change.




> Hanno
>
>
> ------------------------------------------------------------------------------
> October Webinars: Code for Performance
> Free Intel webinars can help you accelerate application performance.
> Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from
> the latest Intel processors and coprocessors. See abstracts and register >
> http://pubads.g.doubleclick.net/gampad/clk?id=60135031&iu=/4140/ostg.clktrk
> _______________________________________________
> Plone-developers mailing list
> Plone-developers-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/plone-developers

------------------------------------------------------------------------------
October Webinars: Code for Performance
Free Intel webinars can help you accelerate application performance.
Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from 
the latest Intel processors and coprocessors. See abstracts and register >
http://pubads.g.doubleclick.net/gampad/clk?id=60135031&iu=/4140/ostg.clktrk