Re: Protect syscache from bloating with negative cache entries

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, andres(at)anarazel(dot)de
Cc: robertmhaas(at)gmail(dot)com, ideriha(dot)takeshi(at)jp(dot)fujitsu(dot)com, tomas(dot)vondra(at)2ndquadrant(dot)com, tgl(at)sss(dot)pgh(dot)pa(dot)us, tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com, alvherre(at)2ndquadrant(dot)com, bruce(at)momjian(dot)us, pgsql-hackers(at)lists(dot)postgresql(dot)org, michael(dot)paquier(at)gmail(dot)com, david(at)pgmasters(dot)net, craig(at)2ndquadrant(dot)com
Subject: Re: Protect syscache from bloating with negative cache entries
Date: 2021-01-27 11:11:55
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 27/01/2021 03:13, Kyotaro Horiguchi wrote:
> At Thu, 14 Jan 2021 17:32:27 +0900 (JST), Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> wrote in
>> The commit 4656e3d668 (debug_invalidate_system_caches_always)
>> conflicted with this patch. Rebased.
> At Wed, 27 Jan 2021 10:07:47 +0900 (JST), Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> wrote in
>> (I found a bug in a benchmark-aid function
>> (CatalogCacheFlushCatalog2), I repost an updated version soon.)
> I noticed that a catcachebench-aid function
> CatalogCacheFlushCatalog2() allocates bucked array wrongly in the
> current memory context, which leads to a crash.
> This is a fixed it then rebased version.

Thanks, with the scripts you provided, I was able to run the performance
tests on my laptop, and got very similar results as you did.

The impact of v7-0002-Remove-dead-flag-from-catcache-tuple.patch is very
small. I think I could see it in the tests, but only barely. And the
tests did nothing else than do syscache lookups; in any real world
scenario, it would be lost in noise. I think we can put that aside for
now, and focus on v6-0001-CatCache-expiration-feature.patch:

The pruning is still pretty lethargic:

- Entries created in the same transactions are never pruned away

- The size of the hash table is never shrunk. So even though the patch
puts a backstop to the hash table growing indefinitely, if you run one
transaction that bloats the cache, it's bloated for the rest of the session.

I think that's OK. We might want to be more aggressive in the future,
but for now it seems reasonable to lean towards the current behavior
where nothing is pruned. Although I wonder if we should try to set
'catcacheclock' more aggressively. I think we could set it whenever
GetCurrentTimestamp() is called, for example.

Given how unaggressive this mechanism is, I think it should be safe to
enable it by default. What would be a suitable default for
catalog_cache_prune_min_age? 30 seconds?

Documentation needs to be updated for the new GUC.

Attached is a version with a few little cleanups:
- use TimestampTz instead of uint64 for the timestamps
- remove assign_catalog_cache_prune_min_age(). All it did was convert
the GUC's value from seconds to microseconds, and stored it in a
separate variable. Multiplication is cheap, so we can just do it when we
use the GUC's value instead.

- Heikki

Attachment Content-Type Size
v8-0001-CatCache-expiration-feature.patch text/x-patch 7.6 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2021-01-27 11:12:17 Re: Support ALTER SUBSCRIPTION ... ADD/DROP PUBLICATION ... syntax
Previous Message Dean Rasheed 2021-01-27 11:02:51 Re: PoC/WIP: Extended statistics on expressions