Re: Protect syscache from bloating with negative cache entries

At Tue, 12 Feb 2019 18:33:46 +0100, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote in
<d3b291ff-d993-78d1-8d28-61bcf72793d6@2ndquadrant.com>
> > "catalog_cache_prune_min_age", "catalog_cache_memory_target", (if
> > exists) "catalog_cache_entry_limit" and
> > "catalog_cache_prune_ratio" make sense?
> > 
> 
> I think "catalog_cache" sounds about right, although my point was simply
> that there's a discrepancy between sgml docs and code.

system_catalog_cache is too long for parameter names. So I named
parameters "catalog_cache_*" and "system catalog cache" or
"catalog cache" in documentation.

> >> 2) "cache_entry_limit" is not mentioned in sgml docs at all, and it's
> >> defined three times in guc.c for some reason.
> > 
> > It is just PoC, added to show how it looks. (The multiple
> > instances must bex a result of a convulsion of my fingers..) I
> > think this is not useful unless it can be specfied per-relation
> > or per-cache basis. I'll remove the GUC and add reloptions for
> > the purpose. (But it won't work for pg_class and pg_attribute
> > for now).
> > 
> 
> OK, although I'd just keep it as simple as possible. TBH I can't really
> imagine users tuning limits for individual caches in any meaningful way.

I also fee like so, but anyway (:p), in v15, it is evoleved into
a feature that limits cache size with the total size based on
global LRU list.

> > I didin't consider planning that happen within a function. If
> > 5min is the default for catalog_cache_prune_min_age, 10% of it
> > (30s) seems enough and gettieofday() with such intervals wouldn't
> > affect forground jobs. I'd choose catalog_c_p_m_age/10 rather
> > than fixed value 30s and 1s as the minimal.
> > 
> 
> Actually, I see CatCacheCleanupOldEntries contains this comment:
> 
> /*
>  * Calculate the duration from the time of the last access to the
>  * "current" time. Since catcacheclock is not advanced within a
>  * transaction, the entries that are accessed within the current
>  * transaction won't be pruned.
>  */
> 
> which I think is pretty much what I've been saying ... But the question
> is whether we need to do something about it.

As I wrote in the messages just replied to Tsunakawa-san, it just
a bogus comment. The corrent one is the following. I'll replace
it in the next version.

> * Calculate the duration from the time from the last access to
> * the "current" time. catcacheclock is updated per-statement
> * basis and additionaly udpated periodically during a long
> * running query.

> > I obeserved significant degradation by setting up timer at every
> > statement start. The patch is doing the followings to get rid of
> > the degradation.
> > 
> > (1) Every statement updates the catcache timestamp as currently
> >     does.  (SetCatCacheClock)
> > 
> > (2) The timestamp is also updated periodically using timer
> >    separately from (1). The timer starts if not yet at the time
> >    of (1).  (SetCatCacheClock, UpdateCatCacheClock)
> > 
> > (3) Statement end and transaction end don't stop the timer, to
> >    avoid overhead of setting up a timer. (
> > 
> > (4) But it stops by error. I choosed not to change the thing in
> >     PostgresMain that it kills all timers on error.
> > 
> > (5) Also changing the GUC catalog_cache_prune_min_age kills the
> >    timer, in order to reflect the change quickly especially when
> >    it is shortened.
> > 
> 
> Interesting. What was the frequency of the timer / how often was it
> executed? Can you share the code somehow?

Please find it in v14 [1] or v15 [2], which contain the same code
for teh purpose.

[1] https://www.postgresql.org/message-id/20190212.203628.118792892.horiguchi.kyotaro@lab.ntt.co.jp

[2] https://www.postgresql.org/message-id/20190213.153114.239737674.horiguchi.kyotaro%40lab.ntt.co.jp

regarsd.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center