|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|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
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.
|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|