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

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.

> 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...).

> 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 :-)
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