Re: [HACKERS] Protect syscache from bloating with negative cache entries
This is a rebased version of the patch. At Fri, 17 Mar 2017 14:23:13 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI wrote in <20170317.142313.232290068.horiguchi.kyot...@lab.ntt.co.jp> > At Tue, 7 Mar 2017 19:23:14 -0800, David Steele wrote > in <3b7b7f90-db46-8c37-c4f7-443330c3a...@pgmasters.net> > > On 3/3/17 4:54 PM, David Steele wrote: > > > > > On 2/1/17 1:25 AM, Kyotaro HORIGUCHI wrote: > > >> Hello, thank you for moving this to the next CF. > > >> > > >> At Wed, 1 Feb 2017 13:09:51 +0900, Michael Paquier > > >> wrote in > > >> > > >>> On Tue, Jan 24, 2017 at 4:58 PM, Kyotaro HORIGUCHI > > >>> wrote: > > Six new syscaches in 665d1fa was conflicted and 3-way merge > > worked correctly. The new syscaches don't seem to be targets of > > this patch. > > >>> To be honest, I am not completely sure what to think about this patch. > > >>> Moved to next CF as there is a new version, and no new reviews to make > > >>> the discussion perhaps move on. > > >> I'm thinking the following is the status of this topic. > > >> > > >> - The patch stll is not getting conflicted. > > >> > > >> - This is not a hollistic measure for memory leak but surely > > >>saves some existing cases. > > >> > > >> - Shared catcache is another discussion (and won't really > > >>proposed in a short time due to the issue on locking.) > > >> > > >> - As I mentioned, a patch that caps the number of negative > > >>entries is avaiable (in first-created - first-delete manner) > > >>but it is having a loose end of how to determine the > > >>limitation. > > > While preventing bloat in the syscache is a worthwhile goal, it > > > appears > > > there are a number of loose ends here and a new patch has not been > > > provided. regards, -- Kyotaro Horiguchi NTT Open Source Software Center >From 9f2c81dbc9bc344cafd6995dfc5969d55a8457d9 Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi Date: Mon, 28 Aug 2017 11:36:21 +0900 Subject: [PATCH 1/2] Cleanup negative cache of pg_statistic when dropping a relation. Accessing columns that don't have statistics leaves negative entries in catcache for pg_statstic, but there's no chance to remove them. Especially when repeatedly creating then dropping temporary tables bloats catcache so much that memory pressure becomes significant. This patch removes negative entries in STATRELATTINH, ATTNAME and ATTNUM when corresponding relation is dropped. --- src/backend/utils/cache/catcache.c | 58 ++- src/backend/utils/cache/syscache.c | 302 +++-- src/include/utils/catcache.h | 3 + src/include/utils/syscache.h | 2 + 4 files changed, 282 insertions(+), 83 deletions(-) diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c index 95a0742..bd303f3 100644 --- a/src/backend/utils/cache/catcache.c +++ b/src/backend/utils/cache/catcache.c @@ -423,10 +423,11 @@ CatCachePrintStats(int code, Datum arg) if (cache->cc_ntup == 0 && cache->cc_searches == 0) continue; /* don't print unused caches */ - elog(DEBUG2, "catcache %s/%u: %d tup, %ld srch, %ld+%ld=%ld hits, %ld+%ld=%ld loads, %ld invals, %ld lsrch, %ld lhits", + elog(DEBUG2, "catcache %s/%u: %d tup, %d negtup, %ld srch, %ld+%ld=%ld hits, %ld+%ld=%ld loads, %ld invals, %ld lsrch, %ld lhits", cache->cc_relname, cache->cc_indexoid, cache->cc_ntup, + cache->cc_nnegtup, cache->cc_searches, cache->cc_hits, cache->cc_neg_hits, @@ -495,8 +496,11 @@ CatCacheRemoveCTup(CatCache *cache, CatCTup *ct) * point into tuple, allocated together with the CatCTup. */ if (ct->negative) + { CatCacheFreeKeys(cache->cc_tupdesc, cache->cc_nkeys, cache->cc_keyno, ct->keys); + --cache->cc_nnegtup; + } pfree(ct); @@ -697,6 +701,51 @@ ResetCatalogCache(CatCache *cache) } /* + * CleanupCatCacheNegEntries + * + * Remove negative cache tuples matching a partial key. + * + */ +void +CleanupCatCacheNegEntries(CatCache *cache, ScanKeyData *skey) +{ + int i; + + /* If this cache has no negative entries, nothing to do */ + if (cache->cc_nnegtup == 0) + return; + + /* searching with a partial key means scanning the whole cache */ + for (i = 0; i < cache->cc_nbuckets; i++) + { + dlist_head *bucket = &cache->cc_bucket[i]; + dlist_mutable_iter iter; + + dlist_foreach_modify(iter, bucket) + { + const CCFastEqualFN *cc_fastequal = cache->cc_fastequal; + CatCTup*ct = dlist_container(CatCTup, cache_elem, iter.cur); + int oid_attnum = skey->sk_attno - 1; + + if (!ct->negative) +continue; + + /* Compare the OIDs */ + if (!(cc_fastequal[oid_attnum])(ct->keys[oid_attnum], + skey[0].sk_argument)) +continue; + + /* + * the negative cache entries can no longer be referenced, so we + * can remove it unconditionally + */ + CatCacheRemoveCTup(cache, ct); + } + } +} + + +/* * ResetCatalogCaches * * Reset all caches when a shared cache inval event forces it @@ -845,
Re: [HACKERS] Protect syscache from bloating with negative cache entries
Thank you for the comment. At Mon, 28 Aug 2017 21:31:58 -0400, Robert Haas wrote in > On Mon, Aug 28, 2017 at 5:24 AM, Kyotaro HORIGUCHI > wrote: > > This patch have had interferences from several commits after the > > last submission. I amended this patch to follow them (up to > > f97c55c), removed an unnecessary branch and edited some comments. > > I think the core problem for this patch is that there's no consensus > on what approach to take. Until that somehow gets sorted out, I think > this isn't going to make any progress. Unfortunately, I don't have a > clear idea what sort of solution everybody could tolerate. > > I still think that some kind of slow-expire behavior -- like a clock > hand that hits each backend every 10 minutes and expires entries not > used since the last hit -- is actually pretty sensible. It ensures > that idle or long-running backends don't accumulate infinite bloat > while still allowing the cache to grow large enough for good > performance when all entries are being regularly used. But Tom > doesn't like it. Other approaches were also discussed; none of them > seem like an obvious slam-dunk. I suppose that it slows intermittent lookup of non-existent objects. I have tried a slight different thing. Removing entries by 'age', preserving specified number (or ratio to live entries) of younger negative entries. The problem of that approach was I didn't find how to determine the number of entries to preserve, or I didn't want to offer additional knobs for them. Finally I proposed the patch upthread since it doesn't need any assumption on usage. Though I can make another patch that does the same thing based on LRU, the same how-many-to-preserve problem ought to be resolved in order to avoid slowing the inermittent lookup. > Turning to the patch itself, I don't know how we decide whether the > patch is worth it. Scanning the whole (potentially large) cache to > remove negative entries has a cost, mostly in CPU cycles; keeping > those negative entries around for a long time also has a cost, mostly > in memory. I don't know how to decide whether these patches will help > more people than it hurts, or the other way around -- and it's not > clear that anyone else has a good idea about that either. Scanning a hash on invalidation of several catalogs (hopefully slightly) slows certain percentage of inavlidations on maybe most of workloads. Holding no-longer-lookedup entries surely kills a backend under certain workloads sooner or later. This doesn't save the pg_proc cases, but saves pg_statistic and pg_class cases. I'm not sure what other catalogs can bloat. I could reduce the complexity of this. Inval mechanism conveys only a hash value so this scans the whole of a cache for the target OIDs (with possible spurious targets). This will be resolved by letting inval mechanism convey an OID. (but this may need additional members in an inval entry.) Still, the full scan perfomed in CleanupCatCacheNegEntries doesn't seem easily avoidable. Separating the hash by OID of key or provide special dlist that points tuples in buckets will introduce another complexity. > Typos: funciton, paritial. Thanks. ispell told me of additional typos corresnpond, belive and undistinguisable. regards, -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
Thank you for reviewing this. At Sat, 2 Sep 2017 12:12:47 +1200, Thomas Munro wrote in > On Mon, Aug 28, 2017 at 9:24 PM, Kyotaro HORIGUCHI > wrote: > > This patch have had interferences from several commits after the > > last submission. I amended this patch to follow them (up to > > f97c55c), removed an unnecessary branch and edited some comments. > > Hi Kyotaro-san, > > This applies but several regression tests fail for me. Here is a > sample backtrace: Sorry for the silly mistake. STAEXTNAMENSP and STATRELATTINH was missing additional elements in their definitions. Somehow I've removed them. The attached patch doesn't crash by regression test. And fixed some typos pointed by Robert and found by myself. regards. -- Kyotaro Horiguchi NTT Open Source Software Center >From 215530f562eaa514437e811686f6e3e419fdd950 Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi Date: Mon, 28 Aug 2017 11:36:21 +0900 Subject: [PATCH 1/2] Cleanup negative cache of pg_statistic when dropping a relation. Accessing columns that don't have statistics leaves negative entries in catcache for pg_statstic, but there's no chance to remove them. Especially when repeatedly creating then dropping temporary tables bloats catcache so much that memory pressure becomes significant. This patch removes negative entries in STATRELATTINH, ATTNAME and ATTNUM when corresponding relation is dropped. --- src/backend/utils/cache/catcache.c | 57 ++- src/backend/utils/cache/syscache.c | 302 +++-- src/include/utils/catcache.h | 3 + src/include/utils/syscache.h | 2 + 4 files changed, 281 insertions(+), 83 deletions(-) diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c index e092801..73e7a44 100644 --- a/src/backend/utils/cache/catcache.c +++ b/src/backend/utils/cache/catcache.c @@ -304,10 +304,11 @@ CatCachePrintStats(int code, Datum arg) if (cache->cc_ntup == 0 && cache->cc_searches == 0) continue; /* don't print unused caches */ - elog(DEBUG2, "catcache %s/%u: %d tup, %ld srch, %ld+%ld=%ld hits, %ld+%ld=%ld loads, %ld invals, %ld lsrch, %ld lhits", + elog(DEBUG2, "catcache %s/%u: %d tup, %d negtup, %ld srch, %ld+%ld=%ld hits, %ld+%ld=%ld loads, %ld invals, %ld lsrch, %ld lhits", cache->cc_relname, cache->cc_indexoid, cache->cc_ntup, + cache->cc_nnegtup, cache->cc_searches, cache->cc_hits, cache->cc_neg_hits, @@ -374,6 +375,10 @@ CatCacheRemoveCTup(CatCache *cache, CatCTup *ct) /* free associated tuple data */ if (ct->tuple.t_data != NULL) pfree(ct->tuple.t_data); + + if (ct->negative) + --cache->cc_nnegtup; + pfree(ct); --cache->cc_ntup; @@ -572,6 +577,49 @@ ResetCatalogCache(CatCache *cache) } /* + * CleanupCatCacheNegEntries + * + * Remove negative cache tuples matching a partial key. + * + */ +void +CleanupCatCacheNegEntries(CatCache *cache, ScanKeyData *skey) +{ + int i; + + /* If this cache has no negative entries, nothing to do */ + if (cache->cc_nnegtup == 0) + return; + + /* searching with a partial key means scanning the whole cache */ + for (i = 0; i < cache->cc_nbuckets; i++) + { + dlist_head *bucket = &cache->cc_bucket[i]; + dlist_mutable_iter iter; + + dlist_foreach_modify(iter, bucket) + { + CatCTup*ct = dlist_container(CatCTup, cache_elem, iter.cur); + bool res; + + if (!ct->negative) +continue; + + HeapKeyTest(&ct->tuple, cache->cc_tupdesc, 1, skey, res); + if (!res) +continue; + + /* + * the negative cache entries can no longer be referenced, so we + * can remove it unconditionally + */ + CatCacheRemoveCTup(cache, ct); + } + } +} + + +/* * ResetCatalogCaches * * Reset all caches when a shared cache inval event forces it @@ -718,6 +766,7 @@ InitCatCache(int id, cp->cc_relisshared = false; /* temporary */ cp->cc_tupdesc = (TupleDesc) NULL; cp->cc_ntup = 0; + cp->cc_nnegtup = 0; cp->cc_nbuckets = nbuckets; cp->cc_nkeys = nkeys; for (i = 0; i < nkeys; ++i) @@ -1215,8 +1264,8 @@ SearchCatCache(CatCache *cache, CACHE4_elog(DEBUG2, "SearchCatCache(%s): Contains %d/%d tuples", cache->cc_relname, cache->cc_ntup, CacheHdr->ch_ntup); - CACHE3_elog(DEBUG2, "SearchCatCache(%s): put neg entry in bucket %d", - cache->cc_relname, hashIndex); + CACHE4_elog(DEBUG2, "SearchCatCache(%s): put neg entry in bucket %d, total %d", + cache->cc_relname, hashIndex, cache->cc_nnegtup); /* * We are not returning the negative entry to the caller, so leave its @@ -1667,6 +1716,8 @@ CatalogCacheCreateEntry(CatCache *cache, HeapTuple ntp, cache->cc_ntup++; CacheHdr->ch_ntup++; + if (negative) + cache->cc_nnegtup++; /* * If the hash table has become too full, enlarge the buckets array. Quite diff --git a/src/backend/utils/cache/syscache.c b/src/backend/utils/cache/syscache.c index 607fe9d..1faed74 100644 --- a/src/backend/utils/cache/syscache.c +++ b/src/backend/utils/cache/syscache.c @@ -75,6 +7
Re: [HACKERS] Protect syscache from bloating with negative cache entries
On Mon, Aug 28, 2017 at 9:24 PM, Kyotaro HORIGUCHI wrote: > This patch have had interferences from several commits after the > last submission. I amended this patch to follow them (up to > f97c55c), removed an unnecessary branch and edited some comments. Hi Kyotaro-san, This applies but several regression tests fail for me. Here is a sample backtrace: frame #3: 0x00010f0614c0 postgres`ExceptionalCondition(conditionName="!(attnum < 0 ? attnum == (-2) : cache->cc_tupdesc->attrs[attnum].atttypid == 26)", errorType="FailedAssertion", fileName="catcache.c", lineNumber=1384) + 128 at assert.c:54 frame #4: 0x00010f03b5fd postgres`CollectOIDsForHashValue(cache=0x7fe273821268, hashValue=994410284, attnum=0) + 253 at catcache.c:1383 frame #5: 0x00010f055e8e postgres`SysCacheSysCacheInvalCallback(arg=140610577303984, cacheid=0, hashValue=994410284) + 94 at syscache.c:1692 frame #6: 0x00010f03fbbb postgres`CallSyscacheCallbacks(cacheid=0, hashvalue=994410284) + 219 at inval.c:1468 frame #7: 0x00010f03f878 postgres`LocalExecuteInvalidationMessage(msg=0x7fff51213ff8) + 88 at inval.c:566 frame #8: 0x00010ee7a3f2 postgres`ReceiveSharedInvalidMessages(invalFunction=(postgres`LocalExecuteInvalidationMessage at inval.c:555), resetFunction=(postgres`InvalidateSystemCaches at inval.c:647)) + 354 at sinval.c:121 frame #9: 0x00010f03fcb7 postgres`AcceptInvalidationMessages + 23 at inval.c:686 frame #10: 0x00010eade609 postgres`AtStart_Cache + 9 at xact.c:987 frame #11: 0x00010ead8c2f postgres`StartTransaction + 655 at xact.c:1921 frame #12: 0x00010ead8896 postgres`StartTransactionCommand + 70 at xact.c:2691 frame #13: 0x00010eea9746 postgres`start_xact_command + 22 at postgres.c:2438 frame #14: 0x00010eea722e postgres`exec_simple_query(query_string="RESET SESSION AUTHORIZATION;") + 126 at postgres.c:913 frame #15: 0x00010eea68d7 postgres`PostgresMain(argc=1, argv=0x7fe2738036a8, dbname="regression", username="munro") + 2375 at postgres.c:4090 frame #16: 0x00010eded40e postgres`BackendRun(port=0x7fe2716001a0) + 654 at postmaster.c:4357 frame #17: 0x00010edec793 postgres`BackendStartup(port=0x7fe2716001a0) + 483 at postmaster.c:4029 frame #18: 0x00010edeb785 postgres`ServerLoop + 597 at postmaster.c:1753 frame #19: 0x00010ede8f71 postgres`PostmasterMain(argc=8, argv=0x7fe271403860) + 5553 at postmaster.c:1361 frame #20: 0x00010ed0ccd9 postgres`main(argc=8, argv=0x7fe271403860) + 761 at main.c:228 frame #21: 0x7fff8333a5ad libdyld.dylib`start + 1 -- Thomas Munro http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
On Mon, Aug 28, 2017 at 5:24 AM, Kyotaro HORIGUCHI wrote: > This patch have had interferences from several commits after the > last submission. I amended this patch to follow them (up to > f97c55c), removed an unnecessary branch and edited some comments. I think the core problem for this patch is that there's no consensus on what approach to take. Until that somehow gets sorted out, I think this isn't going to make any progress. Unfortunately, I don't have a clear idea what sort of solution everybody could tolerate. I still think that some kind of slow-expire behavior -- like a clock hand that hits each backend every 10 minutes and expires entries not used since the last hit -- is actually pretty sensible. It ensures that idle or long-running backends don't accumulate infinite bloat while still allowing the cache to grow large enough for good performance when all entries are being regularly used. But Tom doesn't like it. Other approaches were also discussed; none of them seem like an obvious slam-dunk. Turning to the patch itself, I don't know how we decide whether the patch is worth it. Scanning the whole (potentially large) cache to remove negative entries has a cost, mostly in CPU cycles; keeping those negative entries around for a long time also has a cost, mostly in memory. I don't know how to decide whether these patches will help more people than it hurts, or the other way around -- and it's not clear that anyone else has a good idea about that either. Typos: funciton, paritial. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
Thank you for your attention. At Mon, 14 Aug 2017 17:33:48 -0400, Peter Eisentraut wrote in <09fa011f-4536-b05d-0625-11f3625d8...@2ndquadrant.com> > On 1/24/17 02:58, Kyotaro HORIGUCHI wrote: > >> BTW, if you set a slightly larger > >> context size on the patch you might be able to avoid rebases; right > >> now the patch doesn't include enough context to uniquely identify the > >> chunks against cacheinfo[]. > > git format-patch -U5 fuses all hunks on cacheinfo[] together. I'm > > not sure that such a hunk can avoid rebases. Is this what you > > suggested? -U4 added an identifiable forward context line for > > some elements so the attached patch is made with four context > > lines. > > This patch needs another rebase for the upcoming commit fest. This patch have had interferences from several commits after the last submission. I amended this patch to follow them (up to f97c55c), removed an unnecessary branch and edited some comments. regards, -- Kyotaro Horiguchi NTT Open Source Software Center >From 4887f7d178b41a1a4729931a12bd396b9a8e8ee0 Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi Date: Mon, 28 Aug 2017 11:36:21 +0900 Subject: [PATCH 1/2] Cleanup negative cache of pg_statistic when dropping a relation. Accessing columns that don't have statistics leaves negative entries in catcache for pg_statstic, but there's no chance to remove them. Especially when repeatedly creating then dropping temporary tables bloats catcache so much that memory pressure becomes significant. This patch removes negative entries in STATRELATTINH, ATTNAME and ATTNUM when corresponding relation is dropped. --- src/backend/utils/cache/catcache.c | 57 ++- src/backend/utils/cache/syscache.c | 299 +++-- src/include/utils/catcache.h | 3 + src/include/utils/syscache.h | 2 + 4 files changed, 279 insertions(+), 82 deletions(-) diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c index e092801..e50c997 100644 --- a/src/backend/utils/cache/catcache.c +++ b/src/backend/utils/cache/catcache.c @@ -304,10 +304,11 @@ CatCachePrintStats(int code, Datum arg) if (cache->cc_ntup == 0 && cache->cc_searches == 0) continue; /* don't print unused caches */ - elog(DEBUG2, "catcache %s/%u: %d tup, %ld srch, %ld+%ld=%ld hits, %ld+%ld=%ld loads, %ld invals, %ld lsrch, %ld lhits", + elog(DEBUG2, "catcache %s/%u: %d tup, %d negtup, %ld srch, %ld+%ld=%ld hits, %ld+%ld=%ld loads, %ld invals, %ld lsrch, %ld lhits", cache->cc_relname, cache->cc_indexoid, cache->cc_ntup, + cache->cc_nnegtup, cache->cc_searches, cache->cc_hits, cache->cc_neg_hits, @@ -374,6 +375,10 @@ CatCacheRemoveCTup(CatCache *cache, CatCTup *ct) /* free associated tuple data */ if (ct->tuple.t_data != NULL) pfree(ct->tuple.t_data); + + if (ct->negative) + --cache->cc_nnegtup; + pfree(ct); --cache->cc_ntup; @@ -572,6 +577,49 @@ ResetCatalogCache(CatCache *cache) } /* + * CleanupCatCacheNegEntries + * + * Remove negative cache tuples matching a partial key. + * + */ +void +CleanupCatCacheNegEntries(CatCache *cache, ScanKeyData *skey) +{ + int i; + + /* If this cache has no negative entries, nothing to do */ + if (cache->cc_nnegtup == 0) + return; + + /* searching with a paritial key means scanning the whole cache */ + for (i = 0; i < cache->cc_nbuckets; i++) + { + dlist_head *bucket = &cache->cc_bucket[i]; + dlist_mutable_iter iter; + + dlist_foreach_modify(iter, bucket) + { + CatCTup*ct = dlist_container(CatCTup, cache_elem, iter.cur); + bool res; + + if (!ct->negative) +continue; + + HeapKeyTest(&ct->tuple, cache->cc_tupdesc, 1, skey, res); + if (!res) +continue; + + /* + * the negative cache entries can no longer be referenced, so we + * can remove it unconditionally + */ + CatCacheRemoveCTup(cache, ct); + } + } +} + + +/* * ResetCatalogCaches * * Reset all caches when a shared cache inval event forces it @@ -718,6 +766,7 @@ InitCatCache(int id, cp->cc_relisshared = false; /* temporary */ cp->cc_tupdesc = (TupleDesc) NULL; cp->cc_ntup = 0; + cp->cc_nnegtup = 0; cp->cc_nbuckets = nbuckets; cp->cc_nkeys = nkeys; for (i = 0; i < nkeys; ++i) @@ -1215,8 +1264,8 @@ SearchCatCache(CatCache *cache, CACHE4_elog(DEBUG2, "SearchCatCache(%s): Contains %d/%d tuples", cache->cc_relname, cache->cc_ntup, CacheHdr->ch_ntup); - CACHE3_elog(DEBUG2, "SearchCatCache(%s): put neg entry in bucket %d", - cache->cc_relname, hashIndex); + CACHE4_elog(DEBUG2, "SearchCatCache(%s): put neg entry in bucket %d, total %d", + cache->cc_relname, hashIndex, cache->cc_nnegtup); /* * We are not returning the negative entry to the caller, so leave its @@ -1667,6 +1716,8 @@ CatalogCacheCreateEntry(CatCache *cache, HeapTuple ntp, cache->cc_ntup++; CacheHdr->ch_ntup++; + if (negative) + cache->cc_nnegtup++; /* * If the hash table has become too full, enlarge the
Re: [HACKERS] Protect syscache from bloating with negative cache entries
On 1/24/17 02:58, Kyotaro HORIGUCHI wrote: >> BTW, if you set a slightly larger >> context size on the patch you might be able to avoid rebases; right >> now the patch doesn't include enough context to uniquely identify the >> chunks against cacheinfo[]. > git format-patch -U5 fuses all hunks on cacheinfo[] together. I'm > not sure that such a hunk can avoid rebases. Is this what you > suggested? -U4 added an identifiable forward context line for > some elements so the attached patch is made with four context > lines. This patch needs another rebase for the upcoming commit fest. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
At Tue, 7 Mar 2017 19:23:14 -0800, David Steele wrote in <3b7b7f90-db46-8c37-c4f7-443330c3a...@pgmasters.net> > On 3/3/17 4:54 PM, David Steele wrote: > > > On 2/1/17 1:25 AM, Kyotaro HORIGUCHI wrote: > >> Hello, thank you for moving this to the next CF. > >> > >> At Wed, 1 Feb 2017 13:09:51 +0900, Michael Paquier > >> wrote in > >> > >>> On Tue, Jan 24, 2017 at 4:58 PM, Kyotaro HORIGUCHI > >>> wrote: > Six new syscaches in 665d1fa was conflicted and 3-way merge > worked correctly. The new syscaches don't seem to be targets of > this patch. > >>> To be honest, I am not completely sure what to think about this patch. > >>> Moved to next CF as there is a new version, and no new reviews to make > >>> the discussion perhaps move on. > >> I'm thinking the following is the status of this topic. > >> > >> - The patch stll is not getting conflicted. > >> > >> - This is not a hollistic measure for memory leak but surely > >>saves some existing cases. > >> > >> - Shared catcache is another discussion (and won't really > >>proposed in a short time due to the issue on locking.) > >> > >> - As I mentioned, a patch that caps the number of negative > >>entries is avaiable (in first-created - first-delete manner) > >>but it is having a loose end of how to determine the > >>limitation. > > While preventing bloat in the syscache is a worthwhile goal, it > > appears > > there are a number of loose ends here and a new patch has not been > > provided. > > > > It's a pretty major change so I recommend moving this patch to the > > 2017-07 CF. > > Not hearing any opinions pro or con, I'm moving this patch to the > 2017-07 CF. Ah. Yes, I agree on this. Thanks. -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
On 3/3/17 4:54 PM, David Steele wrote: On 2/1/17 1:25 AM, Kyotaro HORIGUCHI wrote: Hello, thank you for moving this to the next CF. At Wed, 1 Feb 2017 13:09:51 +0900, Michael Paquier wrote in On Tue, Jan 24, 2017 at 4:58 PM, Kyotaro HORIGUCHI wrote: Six new syscaches in 665d1fa was conflicted and 3-way merge worked correctly. The new syscaches don't seem to be targets of this patch. To be honest, I am not completely sure what to think about this patch. Moved to next CF as there is a new version, and no new reviews to make the discussion perhaps move on. I'm thinking the following is the status of this topic. - The patch stll is not getting conflicted. - This is not a hollistic measure for memory leak but surely saves some existing cases. - Shared catcache is another discussion (and won't really proposed in a short time due to the issue on locking.) - As I mentioned, a patch that caps the number of negative entries is avaiable (in first-created - first-delete manner) but it is having a loose end of how to determine the limitation. While preventing bloat in the syscache is a worthwhile goal, it appears there are a number of loose ends here and a new patch has not been provided. It's a pretty major change so I recommend moving this patch to the 2017-07 CF. Not hearing any opinions pro or con, I'm moving this patch to the 2017-07 CF. -- -David da...@pgmasters.net -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
On 2/1/17 1:25 AM, Kyotaro HORIGUCHI wrote: > Hello, thank you for moving this to the next CF. > > At Wed, 1 Feb 2017 13:09:51 +0900, Michael Paquier > wrote in > >> On Tue, Jan 24, 2017 at 4:58 PM, Kyotaro HORIGUCHI >> wrote: >>> Six new syscaches in 665d1fa was conflicted and 3-way merge >>> worked correctly. The new syscaches don't seem to be targets of >>> this patch. >> >> To be honest, I am not completely sure what to think about this patch. >> Moved to next CF as there is a new version, and no new reviews to make >> the discussion perhaps move on. > > I'm thinking the following is the status of this topic. > > - The patch stll is not getting conflicted. > > - This is not a hollistic measure for memory leak but surely > saves some existing cases. > > - Shared catcache is another discussion (and won't really > proposed in a short time due to the issue on locking.) > > - As I mentioned, a patch that caps the number of negative > entries is avaiable (in first-created - first-delete manner) > but it is having a loose end of how to determine the > limitation. While preventing bloat in the syscache is a worthwhile goal, it appears there are a number of loose ends here and a new patch has not been provided. It's a pretty major change so I recommend moving this patch to the 2017-07 CF. -- -David da...@pgmasters.net -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
Hello, thank you for moving this to the next CF. At Wed, 1 Feb 2017 13:09:51 +0900, Michael Paquier wrote in > On Tue, Jan 24, 2017 at 4:58 PM, Kyotaro HORIGUCHI > wrote: > > Six new syscaches in 665d1fa was conflicted and 3-way merge > > worked correctly. The new syscaches don't seem to be targets of > > this patch. > > To be honest, I am not completely sure what to think about this patch. > Moved to next CF as there is a new version, and no new reviews to make > the discussion perhaps move on. I'm thinking the following is the status of this topic. - The patch stll is not getting conflicted. - This is not a hollistic measure for memory leak but surely saves some existing cases. - Shared catcache is another discussion (and won't really proposed in a short time due to the issue on locking.) - As I mentioned, a patch that caps the number of negative entries is avaiable (in first-created - first-delete manner) but it is having a loose end of how to determine the limitation. regards, -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
On Tue, Jan 24, 2017 at 4:58 PM, Kyotaro HORIGUCHI wrote: > Six new syscaches in 665d1fa was conflicted and 3-way merge > worked correctly. The new syscaches don't seem to be targets of > this patch. To be honest, I am not completely sure what to think about this patch. Moved to next CF as there is a new version, and no new reviews to make the discussion perhaps move on. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
Hello, I have tried to cap the number of negative entries for myself (by removing negative entries in least recentrly created first order) but the ceils cannot be reasonably determined both absolutely or relatively to positive entries. Apparently it differs widely among caches and applications. At Mon, 23 Jan 2017 08:16:49 -0600, Jim Nasby wrote in <6519b7ad-0aa6-c9f4-8869-20691107f...@bluetreble.com> > On 1/22/17 5:03 PM, Tom Lane wrote: > >> Ok, after reading the code I see I only partly understood what you > >> were > >> saying. In any case, it might still be useful to do some testing with > >> CATCACHE_STATS defined to see if there's caches that don't accumulate > >> a > >> lot of negative entries. > > There definitely are, according to my testing, but by the same token > > it's not clear that a shutoff check would save anything. > > Currently they wouldn't, but there's concerns about the performance of > some of the other ideas in this thread. Getting rid of negative > entries that don't really help could reduce some of those concerns. Or > perhaps the original complaint about STATRELATTINH could be solved by > just disabling negative entries on that cache. As for STATRELATTINH, planning involving small temporary tables that frequently accessed willget benefit from negative entries, but it might ignorably small. ATTNAME, ATTNUM and RENAMENSP also might not get so much from negative entries. If these are true, the whole stuff this patch adds can be replaced with just a boolean in cachedesc that inhibits negatvie entries. Anyway this patch don't save the case of the cache bloat relaed to function reference. I'm not sure how that could be reproduced, though. regards, -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
Hello, thank you for lookin this. At Mon, 23 Jan 2017 16:54:36 -0600, Jim Nasby wrote in <21803f50-a823-c444-ee2b-9a153114f...@bluetreble.com> > On 1/21/17 6:42 PM, Jim Nasby wrote: > > On 12/26/16 2:31 AM, Kyotaro HORIGUCHI wrote: > >> The points of discussion are the following, I think. > >> > >> 1. The first patch seems working well. It costs the time to scan > >>the whole of a catcache that have negative entries for other > >>reloids. However, such negative entries are created by rather > >>unusual usages. Accesing to undefined columns, and accessing > >>columns on which no statistics have created. The > >>whole-catcache scan occurs on ATTNAME, ATTNUM and > >>STATRELATTINH for every invalidation of a relcache entry. > > > > I took a look at this. It looks sane, though I've got a few minor > > comment tweaks: > > > > + *Remove negative cache tuples maching a partial key. > > s/maching/matching/ > > > > +/* searching with a paritial key needs scanning the whole cache */ > > > > s/needs/means/ > > > > + * a negative cache entry cannot be referenced so we can remove > > > > s/referenced/referenced,/ > > > > I was wondering if there's a way to test the performance impact of > > deleting negative entries. Thanks for the pointing out. These are addressed. > I did a make installcheck run with CATCACHE_STATS to see how often we > get negative entries in the 3 caches affected by this patch. The > caches on pg_attribute get almost no negative entries. pg_statistic > gets a good amount of negative entries, presumably because we start > off with no entries in there. On a stable system that presumably won't > be an issue, but if temporary tables are in use and being analyzed I'd > think there could be a moderate amount of inval traffic on that > cache. I'll leave it to a committer to decide if they thing that's an > issue, but you might want to try and quantify how big a hit that is. I > think it'd also be useful to know how much bloat you were seeing in > the field. > > The patch is currently conflicting against master though, due to some > caches being added. Can you rebase? Six new syscaches in 665d1fa was conflicted and 3-way merge worked correctly. The new syscaches don't seem to be targets of this patch. > BTW, if you set a slightly larger > context size on the patch you might be able to avoid rebases; right > now the patch doesn't include enough context to uniquely identify the > chunks against cacheinfo[]. git format-patch -U5 fuses all hunks on cacheinfo[] together. I'm not sure that such a hunk can avoid rebases. Is this what you suggested? -U4 added an identifiable forward context line for some elements so the attached patch is made with four context lines. regards, -- Kyotaro Horiguchi NTT Open Source Software Center >From 3e9932cff6a2db1d081821269f1bab8b39b0f4f0 Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi Date: Thu, 15 Dec 2016 17:43:03 +0900 Subject: [PATCH 1/2] Cleanup negative cache of pg_statistic when dropping a relation. Accessing columns that don't have statistics causes leaves negative entries in catcache for pg_statstic, but there's no chance to remove them. Especially when repeatedly creating then dropping temporary tables bloats catcache so much that memory pressure can be significant. This patch removes negative entries in STATRELATTINH, ATTNAME and ATTNUM when corresponding relation is dropped. --- src/backend/utils/cache/catcache.c | 57 ++- src/backend/utils/cache/syscache.c | 295 +++-- src/include/utils/catcache.h | 3 + src/include/utils/syscache.h | 2 + 4 files changed, 277 insertions(+), 80 deletions(-) diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c index c27186f..6ff2c5e 100644 --- a/src/backend/utils/cache/catcache.c +++ b/src/backend/utils/cache/catcache.c @@ -303,12 +303,13 @@ CatCachePrintStats(int code, Datum arg) CatCache *cache = slist_container(CatCache, cc_next, iter.cur); if (cache->cc_ntup == 0 && cache->cc_searches == 0) continue; /* don't print unused caches */ - elog(DEBUG2, "catcache %s/%u: %d tup, %ld srch, %ld+%ld=%ld hits, %ld+%ld=%ld loads, %ld invals, %ld lsrch, %ld lhits", + elog(DEBUG2, "catcache %s/%u: %d tup, %d negtup, %ld srch, %ld+%ld=%ld hits, %ld+%ld=%ld loads, %ld invals, %ld lsrch, %ld lhits", cache->cc_relname, cache->cc_indexoid, cache->cc_ntup, + cache->cc_nnegtup, cache->cc_searches, cache->cc_hits, cache->cc_neg_hits, cache->cc_hits + cache->cc_neg_hits, @@ -373,8 +374,12 @@ CatCacheRemoveCTup(CatCache *cache, CatCTup *ct) /* free associated tuple data */ if (ct->tuple.t_data != NULL) pfree(ct->tuple.t_data); + + if (ct->negative) + --cache->cc_nnegtup; + pfree(ct); --cache->cc_ntup; --CacheHdr->ch_ntup; @@ -636,8 +641,51 @@ ResetCatalogCache(CatCache *cache) } } /* + * CleanupCatCacheNegEntries + * + * Remove negative cache tup
Re: [HACKERS] Protect syscache from bloating with negative cache entries
On 1/21/17 6:42 PM, Jim Nasby wrote: On 12/26/16 2:31 AM, Kyotaro HORIGUCHI wrote: The points of discussion are the following, I think. 1. The first patch seems working well. It costs the time to scan the whole of a catcache that have negative entries for other reloids. However, such negative entries are created by rather unusual usages. Accesing to undefined columns, and accessing columns on which no statistics have created. The whole-catcache scan occurs on ATTNAME, ATTNUM and STATRELATTINH for every invalidation of a relcache entry. I took a look at this. It looks sane, though I've got a few minor comment tweaks: + *Remove negative cache tuples maching a partial key. s/maching/matching/ +/* searching with a paritial key needs scanning the whole cache */ s/needs/means/ + * a negative cache entry cannot be referenced so we can remove s/referenced/referenced,/ I was wondering if there's a way to test the performance impact of deleting negative entries. I did a make installcheck run with CATCACHE_STATS to see how often we get negative entries in the 3 caches affected by this patch. The caches on pg_attribute get almost no negative entries. pg_statistic gets a good amount of negative entries, presumably because we start off with no entries in there. On a stable system that presumably won't be an issue, but if temporary tables are in use and being analyzed I'd think there could be a moderate amount of inval traffic on that cache. I'll leave it to a committer to decide if they thing that's an issue, but you might want to try and quantify how big a hit that is. I think it'd also be useful to know how much bloat you were seeing in the field. The patch is currently conflicting against master though, due to some caches being added. Can you rebase? BTW, if you set a slightly larger context size on the patch you might be able to avoid rebases; right now the patch doesn't include enough context to uniquely identify the chunks against cacheinfo[]. -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Experts in Analytics, Data Architecture and PostgreSQL Data in Trouble? Get it in Treble! http://BlueTreble.com 855-TREBLE2 (855-873-2532) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
On 1/22/17 5:03 PM, Tom Lane wrote: Ok, after reading the code I see I only partly understood what you were saying. In any case, it might still be useful to do some testing with CATCACHE_STATS defined to see if there's caches that don't accumulate a lot of negative entries. There definitely are, according to my testing, but by the same token it's not clear that a shutoff check would save anything. Currently they wouldn't, but there's concerns about the performance of some of the other ideas in this thread. Getting rid of negative entries that don't really help could reduce some of those concerns. Or perhaps the original complaint about STATRELATTINH could be solved by just disabling negative entries on that cache. -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Experts in Analytics, Data Architecture and PostgreSQL Data in Trouble? Get it in Treble! http://BlueTreble.com 855-TREBLE2 (855-873-2532) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
Jim Nasby writes: >> Ahh, I hadn't considered that. So one idea would be to only track >> negative entries on caches where we know they're actually useful. That >> might make the performance hit of some of the other ideas more >> tolerable. Presumably you're much less likely to pollute the namespace >> cache than some of the others. > Ok, after reading the code I see I only partly understood what you were > saying. In any case, it might still be useful to do some testing with > CATCACHE_STATS defined to see if there's caches that don't accumulate a > lot of negative entries. There definitely are, according to my testing, but by the same token it's not clear that a shutoff check would save anything. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
On 1/22/17 4:41 PM, Jim Nasby wrote: On 1/21/17 8:54 PM, Tom Lane wrote: Jim Nasby writes: The other (possibly naive) question I have is how useful negative entries really are? Will Postgres regularly incur negative lookups, or will these only happen due to user activity? It varies depending on the particular syscache, but in at least some of them, negative cache entries are critical for performance. See for example RelnameGetRelid(), which basically does a RELNAMENSP cache lookup for each schema down the search path until it finds a match. Ahh, I hadn't considered that. So one idea would be to only track negative entries on caches where we know they're actually useful. That might make the performance hit of some of the other ideas more tolerable. Presumably you're much less likely to pollute the namespace cache than some of the others. Ok, after reading the code I see I only partly understood what you were saying. In any case, it might still be useful to do some testing with CATCACHE_STATS defined to see if there's caches that don't accumulate a lot of negative entries. Attached is a patch that tries to document some of this. -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Experts in Analytics, Data Architecture and PostgreSQL Data in Trouble? Get it in Treble! http://BlueTreble.com 855-TREBLE2 (855-873-2532) diff --git a/src/include/utils/catcache.h b/src/include/utils/catcache.h index 253c7b53ed..d718d82d80 100644 --- a/src/include/utils/catcache.h +++ b/src/include/utils/catcache.h @@ -101,7 +101,12 @@ typedef struct catctup * * A negative cache entry is an assertion that there is no tuple matching * a particular key. This is just as useful as a normal entry so far as -* avoiding catalog searches is concerned. Management of positive and +* avoiding catalog searches is concerned. In particular, caching negative +* entries is critical for performance of some caches. For example, current +* code will produce a negative RELNAMENSP entry for every un-qualified +* table looking with the default search_path that puts the pg_catalog +* schema first. This effect can be obverved by defining CATCACHE_STATS and +* observing the log at backend exit. Management of positive and * negative entries is identical. */ int refcount; /* number of active references */ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
On 1/21/17 8:54 PM, Tom Lane wrote: Jim Nasby writes: The other (possibly naive) question I have is how useful negative entries really are? Will Postgres regularly incur negative lookups, or will these only happen due to user activity? It varies depending on the particular syscache, but in at least some of them, negative cache entries are critical for performance. See for example RelnameGetRelid(), which basically does a RELNAMENSP cache lookup for each schema down the search path until it finds a match. Ahh, I hadn't considered that. So one idea would be to only track negative entries on caches where we know they're actually useful. That might make the performance hit of some of the other ideas more tolerable. Presumably you're much less likely to pollute the namespace cache than some of the others. -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Experts in Analytics, Data Architecture and PostgreSQL Data in Trouble? Get it in Treble! http://BlueTreble.com 855-TREBLE2 (855-873-2532) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
Jim Nasby writes: > The other (possibly naive) question I have is how useful negative > entries really are? Will Postgres regularly incur negative lookups, or > will these only happen due to user activity? It varies depending on the particular syscache, but in at least some of them, negative cache entries are critical for performance. See for example RelnameGetRelid(), which basically does a RELNAMENSP cache lookup for each schema down the search path until it finds a match. For any user table name with the standard search_path, there's a guaranteed failure in pg_catalog before you can hope to find a match. If we don't have negative cache entries, then *every invocation of this function has to go to disk* (or at least to shared buffers). It's possible that we could revise all our lookup patterns to avoid this sort of thing. But I don't have much faith in that always being possible, and exactly none that we won't introduce new lookup patterns that need it in future. I spent some time, for instance, wondering if RelnameGetRelid could use a SearchSysCacheList lookup instead, doing the lookup on table name only and then inspecting the whole list to see which entry is frontmost according to the current search path. But that has performance failure modes of its own, for example if you have identical table names in a boatload of different schemas. We do it that way for some other cases such as function lookups, but I think it's much less likely that people have identical function names in N schemas than that they have identical table names in N schemas. If you want to poke into this for particular test scenarios, building with CATCACHE_STATS defined will yield a bunch of numbers dumped to the postmaster log at each backend exit. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
On 12/26/16 2:31 AM, Kyotaro HORIGUCHI wrote: The points of discussion are the following, I think. 1. The first patch seems working well. It costs the time to scan the whole of a catcache that have negative entries for other reloids. However, such negative entries are created by rather unusual usages. Accesing to undefined columns, and accessing columns on which no statistics have created. The whole-catcache scan occurs on ATTNAME, ATTNUM and STATRELATTINH for every invalidation of a relcache entry. I took a look at this. It looks sane, though I've got a few minor comment tweaks: + * Remove negative cache tuples maching a partial key. s/maching/matching/ +/* searching with a paritial key needs scanning the whole cache */ s/needs/means/ + * a negative cache entry cannot be referenced so we can remove s/referenced/referenced,/ I was wondering if there's a way to test the performance impact of deleting negative entries. 2. The second patch also works, but flushing negative entries by hash values is inefficient. It scans the bucket corresponding to given hash value for OIDs, then flushing negative entries iterating over all the collected OIDs. So this costs more time than 1 and flushes involving entries that is not necessary to be removed. If this feature is valuable but such side effects are not acceptable, new invalidation category based on cacheid-oid pair would be needed. I glanced at this and it looks sane. Didn't go any farther since this one's pretty up in the air. ISTM it'd be better to do some kind of aging instead of patch 2. The other (possibly naive) question I have is how useful negative entries really are? Will Postgres regularly incur negative lookups, or will these only happen due to user activity? I can't think of a case where an app would need to depend on fast negative lookup (in other words, it should be considered a bug in the app). I can see where getting rid of them completely might be problematic, but maybe we can just keep a relatively small number of them around. I'm thinking a simple LRU list of X number of negative entries; when that fills you reuse the oldest one. You'd have to pay the LRU maintenance cost on every negative hit, but if those shouldn't be that common it shouldn't be bad. That might well necessitate another GUC, but it seems a lot simpler than most of the other ideas. -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Experts in Analytics, Data Architecture and PostgreSQL Data in Trouble? Get it in Treble! http://BlueTreble.com 855-TREBLE2 (855-873-2532) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
On Sat, Jan 14, 2017 at 9:36 AM, Tomas Vondra wrote: > On 01/14/2017 12:06 AM, Andres Freund wrote: >> On 2017-01-13 17:58:41 -0500, Tom Lane wrote: >>> >>> But, again, the catcache isn't the only source of per-process bloat >>> and I'm not even sure it's the main one. A more holistic approach >>> might be called for. >> >> It'd be helpful if we'd find a way to make it easy to get statistics >> about the size of various caches in production systems. Right now >> that's kinda hard, resulting in us having to make a lot of >> guesses... > > What about a simple C extension, that could inspect those caches? Assuming > it could be loaded into a single backend, that should be relatively > acceptable way (compared to loading it to all backends using > shared_preload_libraries). This extension could do a small amount of work on a portion of the syscache entries at each query loop, still I am wondering if that would not be nicer to get that in-core and configurable, which is basically the approach proposed by Horiguchi-san. At least it seems to me that it has some merit, and if we could make that behavior switchable, disabled by default, that would be a win for some class of applications. What do others think? -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
On 01/14/2017 12:06 AM, Andres Freund wrote: Hi, On 2017-01-13 17:58:41 -0500, Tom Lane wrote: But, again, the catcache isn't the only source of per-process bloat and I'm not even sure it's the main one. A more holistic approach might be called for. It'd be helpful if we'd find a way to make it easy to get statistics about the size of various caches in production systems. Right now that's kinda hard, resulting in us having to make a lot of guesses... What about a simple C extension, that could inspect those caches? Assuming it could be loaded into a single backend, that should be relatively acceptable way (compared to loading it to all backends using shared_preload_libraries). -- Tomas Vondra http://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
Hi, On 2017-01-13 17:58:41 -0500, Tom Lane wrote: > But, again, the catcache isn't the only source of per-process bloat > and I'm not even sure it's the main one. A more holistic approach > might be called for. It'd be helpful if we'd find a way to make it easy to get statistics about the size of various caches in production systems. Right now that's kinda hard, resulting in us having to make a lot of guesses... Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
Michael Paquier writes: > ... There are even some apps that do not use pgbouncer, but drop > sessions after a timeout of inactivity to avoid a memory bloat because > of the problem of this thread. Yeah, a certain company I used to work for had to do that, though their problem had more to do with bloat in plpgsql's compiled-functions cache (and ensuing bloat in the plancache), I believe. Still, I'm pretty suspicious of anything that will add overhead to catcache lookups. If you think the performance of those is not absolutely critical, turning off the caches via -DCLOBBER_CACHE_ALWAYS will soon disabuse you of the error. I'm inclined to think that a more profitable direction to look in is finding a way to limit the cache size. I know we got rid of exactly that years ago, but the problems with it were (a) the mechanism was itself pretty expensive --- a global-to-all-caches LRU list IIRC, and (b) there wasn't a way to tune the limit. Possibly somebody can think of some cheaper, perhaps less precise way of aging out old entries. As for (b), this is the sort of problem we made GUCs for. But, again, the catcache isn't the only source of per-process bloat and I'm not even sure it's the main one. A more holistic approach might be called for. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
On Sat, Jan 14, 2017 at 12:32 AM, Robert Haas wrote: > On Fri, Jan 13, 2017 at 8:58 AM, Tom Lane wrote: >> Michael Paquier writes: >>> Have there been ever discussions about having catcache entries in a >>> shared memory area? This does not sound much performance-wise, I am >>> just wondering about the concept and I cannot find references to such >>> discussions. >> >> I'm sure it's been discussed. Offhand I remember the following issues: >> >> * A shared cache would create locking and contention overhead. >> >> * A shared cache would have a very hard size limit, at least if it's >> in SysV-style shared memory (perhaps DSM would let us relax that). >> >> * Transactions that are doing DDL have a requirement for the catcache >> to reflect changes that they've made locally but not yet committed, >> so said changes mustn't be visible globally. >> >> You could possibly get around the third point with a local catcache that's >> searched before the shared one, but tuning that to be performant sounds >> like a mess. Also, I'm not sure how such a structure could cope with >> uncommitted deletions: delete A -> remove A from local catcache, but not >> the shared one -> search for A in local catcache -> not found -> search >> for A in shared catcache -> found -> oops. > > I think the first of those concerns is the key one. If searching the > system catalogs costs $100 and searching the private catcache costs > $1, what's the cost of searching a hypothetical shared catcache? If > the answer is $80, it's not worth doing. If the answer is $5, it's > probably still not worth doing. If the answer is $1.25, then it's > probably worth investing some energy into trying to solve the other > problems you list. For some users, the memory cost of catcache and > syscache entries multiplied by N backends are a very serious problem, > so it would be nice to have some other options. But we do so many > syscache lookups that a shared cache won't be viable unless it's > almost as fast as a backend-private cache, or at least that's my > hunch. Being able to switch from one mode to another would be interesting. Applications using extensing DDLs that require to change the catcache with an exclusive lock would clearly pay the lock contention cost, but do you think that be really the case of a shared lock? A bunch of applications that I work with deploy Postgres once, then don't change the schema except when an upgrade happens. So that would be benefitial for that. There are even some apps that do not use pgbouncer, but drop sessions after a timeout of inactivity to avoid a memory bloat because of the problem of this thread. That won't solve the problem of the local catcache bloat, but some users using few DDLs may be fine to pay some extra concurrency cost if the session handling gets easied. > I think it would be interested for somebody to build a prototype here > that ignores all the problems but the first and uses some > straightforward, relatively unoptimized locking strategy for the first > problem. Then benchmark it. If the results show that the idea has > legs, then we can try to figure out what a real implementation would > look like. > (One possible approach: use Thomas Munro's DHT stuff to build the shared > cache.) Yeah, I'd bet on a couple of days of focus to sort that out. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
On Fri, Jan 13, 2017 at 8:58 AM, Tom Lane wrote: > Michael Paquier writes: >> Have there been ever discussions about having catcache entries in a >> shared memory area? This does not sound much performance-wise, I am >> just wondering about the concept and I cannot find references to such >> discussions. > > I'm sure it's been discussed. Offhand I remember the following issues: > > * A shared cache would create locking and contention overhead. > > * A shared cache would have a very hard size limit, at least if it's > in SysV-style shared memory (perhaps DSM would let us relax that). > > * Transactions that are doing DDL have a requirement for the catcache > to reflect changes that they've made locally but not yet committed, > so said changes mustn't be visible globally. > > You could possibly get around the third point with a local catcache that's > searched before the shared one, but tuning that to be performant sounds > like a mess. Also, I'm not sure how such a structure could cope with > uncommitted deletions: delete A -> remove A from local catcache, but not > the shared one -> search for A in local catcache -> not found -> search > for A in shared catcache -> found -> oops. I think the first of those concerns is the key one. If searching the system catalogs costs $100 and searching the private catcache costs $1, what's the cost of searching a hypothetical shared catcache? If the answer is $80, it's not worth doing. If the answer is $5, it's probably still not worth doing. If the answer is $1.25, then it's probably worth investing some energy into trying to solve the other problems you list. For some users, the memory cost of catcache and syscache entries multiplied by N backends are a very serious problem, so it would be nice to have some other options. But we do so many syscache lookups that a shared cache won't be viable unless it's almost as fast as a backend-private cache, or at least that's my hunch. I think it would be interested for somebody to build a prototype here that ignores all the problems but the first and uses some straightforward, relatively unoptimized locking strategy for the first problem. Then benchmark it. If the results show that the idea has legs, then we can try to figure out what a real implementation would look like. (One possible approach: use Thomas Munro's DHT stuff to build the shared cache.) -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
Michael Paquier writes: > Have there been ever discussions about having catcache entries in a > shared memory area? This does not sound much performance-wise, I am > just wondering about the concept and I cannot find references to such > discussions. I'm sure it's been discussed. Offhand I remember the following issues: * A shared cache would create locking and contention overhead. * A shared cache would have a very hard size limit, at least if it's in SysV-style shared memory (perhaps DSM would let us relax that). * Transactions that are doing DDL have a requirement for the catcache to reflect changes that they've made locally but not yet committed, so said changes mustn't be visible globally. You could possibly get around the third point with a local catcache that's searched before the shared one, but tuning that to be performant sounds like a mess. Also, I'm not sure how such a structure could cope with uncommitted deletions: delete A -> remove A from local catcache, but not the shared one -> search for A in local catcache -> not found -> search for A in shared catcache -> found -> oops. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
On Wed, Dec 21, 2016 at 5:10 AM, Tom Lane wrote: > If I thought that "every ten minutes" was an ideal way to manage this, > I might worry about that, but it doesn't really sound promising at all. > Every so many queries would likely work better, or better yet make it > self-adaptive depending on how much is in the local syscache. > > The bigger picture here though is that we used to have limits on syscache > size, and we got rid of them (commit 8b9bc234a, see also > https://www.postgresql.org/message-id/flat/5141.1150327541%40sss.pgh.pa.us) > not only because of the problem you mentioned about performance falling > off a cliff once the working-set size exceeded the arbitrary limit, but > also because enforcing the limit added significant overhead --- and did so > whether or not you got any benefit from it, ie even if the limit is never > reached. Maybe the present patch avoids imposing a pile of overhead in > situations where no pruning is needed, but it doesn't really look very > promising from that angle in a quick once-over. Have there been ever discussions about having catcache entries in a shared memory area? This does not sound much performance-wise, I am just wondering about the concept and I cannot find references to such discussions. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
At Wed, 21 Dec 2016 10:21:09 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI wrote in <20161221.102109.51106943.horiguchi.kyot...@lab.ntt.co.jp> > At Tue, 20 Dec 2016 15:10:21 -0500, Tom Lane wrote in > <23492.1482264...@sss.pgh.pa.us> > > The bigger picture here though is that we used to have limits on syscache > > size, and we got rid of them (commit 8b9bc234a, see also > > https://www.postgresql.org/message-id/flat/5141.1150327541%40sss.pgh.pa.us) > > not only because of the problem you mentioned about performance falling > > off a cliff once the working-set size exceeded the arbitrary limit, but > > also because enforcing the limit added significant overhead --- and did so > > whether or not you got any benefit from it, ie even if the limit is never > > reached. Maybe the present patch avoids imposing a pile of overhead in > > situations where no pruning is needed, but it doesn't really look very > > promising from that angle in a quick once-over. > > Indeed. As mentioned in the mail at the beginning of this thread, > it hits the whole-cache scanning if at least one negative cache > exists even it is not in a relation with the target relid, and it > can be significantly long on a fat cache. > > Lists of negative entries like CatCacheList would help but needs > additional memeory. > > > BTW, I don't see the point of the second patch at all? Surely, if > > an object is deleted or updated, we already have code that flushes > > related catcache entries. Otherwise the caches would deliver wrong > > data. > > Maybe you take the patch wrongly. Negative entires won't be > flushed by any means. Deletion of a namespace causes cascaded > object deletion according to dependency then finaly goes to > non-neative cache invalidation. But a removal of *negative > entries* in RELNAMENSP won't happen. > > The test script for the case (gen2.pl) does the following thing, > > CREATE SCHEMA foo; > SELECT * FROM foo.invalid; > DROP SCHEMA foo; > > Removing the schema foo leaves a negative cache entry for > 'foo.invalid' in RELNAMENSP. > > However, I'm not sure the above situation happens so frequent > that it is worthwhile to amend. Since 1753b1b conflicts this patch, I rebased this onto the current master HEAD. I'll register this to the next CF. The points of discussion are the following, I think. 1. The first patch seems working well. It costs the time to scan the whole of a catcache that have negative entries for other reloids. However, such negative entries are created by rather unusual usages. Accesing to undefined columns, and accessing columns on which no statistics have created. The whole-catcache scan occurs on ATTNAME, ATTNUM and STATRELATTINH for every invalidation of a relcache entry. 2. The second patch also works, but flushing negative entries by hash values is inefficient. It scans the bucket corresponding to given hash value for OIDs, then flushing negative entries iterating over all the collected OIDs. So this costs more time than 1 and flushes involving entries that is not necessary to be removed. If this feature is valuable but such side effects are not acceptable, new invalidation category based on cacheid-oid pair would be needed. regards, -- Kyotaro Horiguchi NTT Open Source Software Center >From ee0cc13f70d79f23ec9507cf977228bba091bc49 Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi Date: Thu, 15 Dec 2016 17:43:03 +0900 Subject: [PATCH 1/2] Cleanup negative cache of pg_statistic when dropping a relation. Accessing columns that don't have statistics causes leaves negative entries in catcache for pg_statstic, but there's no chance to remove them. Especially when repeatedly creating then dropping temporary tables bloats catcache so much that memory pressure can be significant. This patch removes negative entries in STATRELATTINH, ATTNAME and ATTNUM when corresponding relation is dropped. --- src/backend/utils/cache/catcache.c | 57 +++- src/backend/utils/cache/syscache.c | 277 +++-- src/include/utils/catcache.h | 3 + src/include/utils/syscache.h | 2 + 4 files changed, 265 insertions(+), 74 deletions(-) diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c index 6016d19..c1d9d2f 100644 --- a/src/backend/utils/cache/catcache.c +++ b/src/backend/utils/cache/catcache.c @@ -304,10 +304,11 @@ CatCachePrintStats(int code, Datum arg) if (cache->cc_ntup == 0 && cache->cc_searches == 0) continue; /* don't print unused caches */ - elog(DEBUG2, "catcache %s/%u: %d tup, %ld srch, %ld+%ld=%ld hits, %ld+%ld=%ld loads, %ld invals, %ld lsrch, %ld lhits", + elog(DEBUG2, "catcache %s/%u: %d tup, %d negtup, %ld srch, %ld+%ld=%ld hits, %ld+%ld=%ld loads, %ld invals, %ld lsrch, %ld lhits", cache->cc_relname, cache->cc_indexoid, cache->cc_ntup, + cache->cc_nnegtup, cache->cc_searches, cache->cc_hits, cache->cc_neg_hits, @@ -374,6 +
Re: [HACKERS] Protect syscache from bloating with negative cache entries
Thank you for the discussion. At Tue, 20 Dec 2016 15:10:21 -0500, Tom Lane wrote in <23492.1482264...@sss.pgh.pa.us> > The bigger picture here though is that we used to have limits on syscache > size, and we got rid of them (commit 8b9bc234a, see also > https://www.postgresql.org/message-id/flat/5141.1150327541%40sss.pgh.pa.us) > not only because of the problem you mentioned about performance falling > off a cliff once the working-set size exceeded the arbitrary limit, but > also because enforcing the limit added significant overhead --- and did so > whether or not you got any benefit from it, ie even if the limit is never > reached. Maybe the present patch avoids imposing a pile of overhead in > situations where no pruning is needed, but it doesn't really look very > promising from that angle in a quick once-over. Indeed. As mentioned in the mail at the beginning of this thread, it hits the whole-cache scanning if at least one negative cache exists even it is not in a relation with the target relid, and it can be significantly long on a fat cache. Lists of negative entries like CatCacheList would help but needs additional memeory. > BTW, I don't see the point of the second patch at all? Surely, if > an object is deleted or updated, we already have code that flushes > related catcache entries. Otherwise the caches would deliver wrong > data. Maybe you take the patch wrongly. Negative entires won't be flushed by any means. Deletion of a namespace causes cascaded object deletion according to dependency then finaly goes to non-neative cache invalidation. But a removal of *negative entries* in RELNAMENSP won't happen. The test script for the case (gen2.pl) does the following thing, CREATE SCHEMA foo; SELECT * FROM foo.invalid; DROP SCHEMA foo; Removing the schema foo leaves a negative cache entry for 'foo.invalid' in RELNAMENSP. However, I'm not sure the above situation happens so frequent that it is worthwhile to amend. regards, -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
On Tue, Dec 20, 2016 at 3:10 PM, Tom Lane wrote: > Robert Haas writes: >> On Tue, Dec 20, 2016 at 10:09 AM, Tom Lane wrote: >>> I don't understand why we'd make that a system-wide behavior at all, >>> rather than expecting each process to manage its own cache. > >> Individual backends don't have a really great way to do time-based >> stuff, do they? I mean, yes, there is enable_timeout() and friends, >> but I think that requires quite a bit of bookkeeping. > > If I thought that "every ten minutes" was an ideal way to manage this, > I might worry about that, but it doesn't really sound promising at all. > Every so many queries would likely work better, or better yet make it > self-adaptive depending on how much is in the local syscache. I don't think "every so many queries" is very promising at all. First, it has the same problem as a fixed cap on the number of entries: if you're doing a round-robin just slightly bigger than that value, performance will be poor. Second, what's really important here is to keep the percentage of wall-clock time spent populating the system caches small. If a backend is doing 4000 queries/second and each of those 4000 queries touches a different table, it really needs a cache of at least 4000 entries or it will thrash and slow way down. But if it's doing a query every 10 minutes and those queries round-robin between 4000 different tables, it doesn't really need a 4000-entry cache. If those queries are long-running, the time to repopulate the cache will only be a tiny fraction of runtime. If the queries are short-running, then the effect is, percentage-wise, just the same as for the high-volume system, but in practice it isn't likely to be felt as much. I mean, if we keep a bunch of old cache entries around on a mostly-idle backend, they are going to be pushed out of CPU caches and maybe even paged out. One can't expect a backend that is woken up after a long sleep to be quite as snappy as one that's continuously active. Which gets to my third point: anything that's based on number of queries won't do anything to help the case where backends sometimes go idle and sit there for long periods. Reducing resource utilization in that case would be beneficial. Ideally I'd like to get rid of not only the backend-local cache contents but the backend itself, but that's a much harder project. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
Robert Haas writes: > On Tue, Dec 20, 2016 at 10:09 AM, Tom Lane wrote: >> I don't understand why we'd make that a system-wide behavior at all, >> rather than expecting each process to manage its own cache. > Individual backends don't have a really great way to do time-based > stuff, do they? I mean, yes, there is enable_timeout() and friends, > but I think that requires quite a bit of bookkeeping. If I thought that "every ten minutes" was an ideal way to manage this, I might worry about that, but it doesn't really sound promising at all. Every so many queries would likely work better, or better yet make it self-adaptive depending on how much is in the local syscache. The bigger picture here though is that we used to have limits on syscache size, and we got rid of them (commit 8b9bc234a, see also https://www.postgresql.org/message-id/flat/5141.1150327541%40sss.pgh.pa.us) not only because of the problem you mentioned about performance falling off a cliff once the working-set size exceeded the arbitrary limit, but also because enforcing the limit added significant overhead --- and did so whether or not you got any benefit from it, ie even if the limit is never reached. Maybe the present patch avoids imposing a pile of overhead in situations where no pruning is needed, but it doesn't really look very promising from that angle in a quick once-over. BTW, I don't see the point of the second patch at all? Surely, if an object is deleted or updated, we already have code that flushes related catcache entries. Otherwise the caches would deliver wrong data. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
On Tue, Dec 20, 2016 at 10:09 AM, Tom Lane wrote: > Craig Ringer writes: >> On 20 December 2016 at 21:59, Robert Haas wrote: >>> We could implement this by having >>> some process, like the background writer, >>> SendProcSignal(PROCSIG_HOUSEKEEPING) to every process in the system >>> every 10 minutes or so. > >> ... on a rolling basis. > > I don't understand why we'd make that a system-wide behavior at all, > rather than expecting each process to manage its own cache. Individual backends don't have a really great way to do time-based stuff, do they? I mean, yes, there is enable_timeout() and friends, but I think that requires quite a bit of bookkeeping. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
Craig Ringer writes: > On 20 December 2016 at 21:59, Robert Haas wrote: >> We could implement this by having >> some process, like the background writer, >> SendProcSignal(PROCSIG_HOUSEKEEPING) to every process in the system >> every 10 minutes or so. > ... on a rolling basis. I don't understand why we'd make that a system-wide behavior at all, rather than expecting each process to manage its own cache. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
On 20 December 2016 at 21:59, Robert Haas wrote: > We could implement this by having > some process, like the background writer, > SendProcSignal(PROCSIG_HOUSEKEEPING) to every process in the system > every 10 minutes or so. ... on a rolling basis. Otherwise that'll be no fun at all, especially with some of those lovely "we kept getting errors so we raised max_connections to 5000" systems out there. But also on more sensibly configured ones that're busy and want nice smooth performance without stalls. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protect syscache from bloating with negative cache entries
On Mon, Dec 19, 2016 at 6:15 AM, Kyotaro HORIGUCHI wrote: > Hello, recently one of my customer stumbled over an immoderate > catcache bloat. This isn't only an issue for negative catcache entries. A long time ago, there was a limit on the size of the relcache, which was removed because if you have a workload where the working set of relations is just larger than the limit, performance is terrible. But the problem now is that backend memory usage can grow without bound, and that's also bad, especially on systems with hundreds of long-lived backends. In connection-pooling environments, the problem is worse, because every connection in the pool eventually caches references to everything of interest to any client. Your patches seem to me to have some merit, but I wonder if we should also consider having a time-based threshold of some kind. If, say, a backend hasn't accessed a catcache or relcache entry for many minutes, it becomes eligible to be flushed. We could implement this by having some process, like the background writer, SendProcSignal(PROCSIG_HOUSEKEEPING) to every process in the system every 10 minutes or so. When a process receives this signal, it sets a flag that is checked before going idle. When it sees the flag set, it makes a pass over every catcache and relcache entry. All the ones that are unmarked get marked, and all of the ones that are marked get removed. Access to an entry clears any mark. So anything that's not touched for more than 10 minutes starts dropping out of backend caches. Anyway, that would be a much bigger change from what you are proposing here, and what you are proposing here seems reasonable so I guess I shouldn't distract from it. Your email just made me think of it, because I agree that catcache/relcache bloat is a serious issue. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers