Re: Protect syscache from bloating with negative cache entries

At Fri, 05 Apr 2019 09:44:07 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in
<20190405.094407.151644324.horiguchi.kyotaro@lab.ntt.co.jp>
> By the way, I found the reason of the wrong result of the
> previous benchmark. The test 3_0/1 needs to update catcacheclock
> midst of the loop. I'm going to fix it and rerun it.

I found the cause. CataloCacheFlushCatalog() doesn't shring the
hash. So no resize happens once it is bloated. I needed another
version of the function that reset the cc_bucket to the initial
size.

Using the new debug function, I got better numbers.

I focused on the performance when disabled. I rechecked that by
adding the patch part-by-part and identified several causes of
the degradaton. I did:

- MovedpSetCatCacheClock() to AtStart_Cache()
- Maybe improved the caller site of CatCacheCleanupOldEntries().

As the result:

 binary | test | count |   avg   | stddev | 
--------+------+-------+---------+--------+-------
 master | 1_1  |     5 | 7104.90 |   4.40 | 
 master | 2_1  |     5 | 3759.26 |   4.20 | 
 master | 3_1  |     5 | 7954.05 |   2.15 | 
--------+------+-------+---------+--------+-------
 Full   | 1_1  |     5 | 7237.20 |   7.98 | 101.87
 Full   | 2_1  |     5 | 4050.98 |   8.42 | 107.76
 Full   | 3_1  |     5 | 8192.87 |   3.28 | 103.00

But, still fluctulates by around 5%..

If this level of the degradation is still not acceptable, that
means that nothing can be inserted in the code path and the new
code path should be isolated from existing code by using indirect
call.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index bd5024ef00..a9414c0c07 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -1067,6 +1067,7 @@ static void
 AtStart_Cache(void)
 {
     AcceptInvalidationMessages();
+    SetCatCacheClock(GetCurrentStatementStartTimestamp());
 }
 
 /*
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 44a59e1d4f..4d849aeb4c 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -71,6 +71,7 @@
 #include "tcop/pquery.h"
 #include "tcop/tcopprot.h"
 #include "tcop/utility.h"
+#include "utils/catcache.h"
 #include "utils/lsyscache.h"
 #include "utils/memutils.h"
 #include "utils/ps_status.h"
diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c
index d05930bc4c..91814f7210 100644
--- a/src/backend/utils/cache/catcache.c
+++ b/src/backend/utils/cache/catcache.c
@@ -60,9 +60,18 @@
 #define CACHE_elog(...)
 #endif
 
+/*
+ * GUC variable to define the minimum age of entries that will be considered
+ * to be evicted in seconds. -1 to disable the feature.
+ */
+int catalog_cache_prune_min_age = 0;
+
 /* Cache management header --- pointer is NULL until created */
 static CatCacheHeader *CacheHdr = NULL;
 
+/* Clock for the last accessed time of a catcache entry. */
+TimestampTz    catcacheclock = 0;
+
 static inline HeapTuple SearchCatCacheInternal(CatCache *cache,
                        int nkeys,
                        Datum v1, Datum v2,
@@ -850,9 +859,83 @@ InitCatCache(int id,
      */
     MemoryContextSwitchTo(oldcxt);
 
+    /* initialize catcache reference clock if haven't done yet */
+    if (catcacheclock == 0)
+        catcacheclock = GetCurrentTimestamp();
+
     return cp;
 }
 
+/*
+ * CatCacheCleanupOldEntries - Remove infrequently-used entries
+ *
+ * Catcache entries happen to be left unused for a long time for several
+ * reasons. Remove such entries to prevent catcache from bloating. It is based
+ * on the similar algorithm with buffer eviction. Entries that are accessed
+ * several times in a certain period live longer than those that have had less
+ * access in the same duration.
+ */
+static bool
+CatCacheCleanupOldEntries(CatCache *cp)
+{
+    int    nremoved = 0;
+    int i;
+
+    /* Return immediately if disabled */
+    if (catalog_cache_prune_min_age == 0)
+        return false;
+
+    /* Scan over the whole hash to find entries to remove */
+    for (i = 0 ; i < cp->cc_nbuckets ; i++)
+    {
+        dlist_mutable_iter    iter;
+
+        dlist_foreach_modify(iter, &cp->cc_bucket[i])
+        {
+            CatCTup    *ct = dlist_container(CatCTup, cache_elem, iter.cur);
+            long        entry_age;
+            int            us;
+
+            /* Don't remove referenced entries */
+            if (ct->refcount != 0 ||
+                (ct->c_list && ct->c_list->refcount != 0))
+                continue;
+
+            /*
+             * 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.
+             */
+            TimestampDifference(ct->lastaccess, catcacheclock, &entry_age, &us);
+
+            if (entry_age < catalog_cache_prune_min_age)
+                continue;
+
+            /*
+             * Entries that are not accessed after the last pruning are
+             * removed in that seconds, and their lives are prolonged
+             * according to how many times they are accessed up to three times
+             * of the duration. We don't try shrink buckets since pruning
+             * effectively caps catcache expansion in the long term.
+             */
+            if (ct->naccess > 1)
+                ct->naccess--;
+            else
+            {
+                CatCacheRemoveCTup(cp, ct);
+                nremoved++;
+            }
+        }
+    }
+
+    if (nremoved > 0)
+        elog(DEBUG1, "pruning catalog cache id=%d for %s: removed %d / %d",
+             cp->id, cp->cc_relname, nremoved, cp->cc_ntup + nremoved);
+
+    return nremoved > 0;
+}
+
 /*
  * Enlarge a catcache, doubling the number of buckets.
  */
@@ -1263,6 +1346,10 @@ SearchCatCacheInternal(CatCache *cache,
          * near the front of the hashbucket's list.)
          */
         dlist_move_head(bucket, &ct->cache_elem);
+        ct->naccess++;
+        ct->naccess &= 3;
+
+        ct->lastaccess = catcacheclock;
 
         /*
          * If it's a positive entry, bump its refcount and return it. If it's
@@ -1888,6 +1975,8 @@ CatalogCacheCreateEntry(CatCache *cache, HeapTuple ntp, Datum *arguments,
     ct->dead = false;
     ct->negative = negative;
     ct->hash_value = hashValue;
+    ct->naccess = 0;
+    ct->lastaccess = catcacheclock;
 
     dlist_push_head(&cache->cc_bucket[hashIndex], &ct->cache_elem);
 
@@ -1895,11 +1984,25 @@ CatalogCacheCreateEntry(CatCache *cache, HeapTuple ntp, Datum *arguments,
     CacheHdr->ch_ntup++;
 
     /*
-     * If the hash table has become too full, enlarge the buckets array. Quite
-     * arbitrarily, we enlarge when fill factor > 2.
-     */
-    if (cache->cc_ntup > cache->cc_nbuckets * 2)
-        RehashCatCache(cache);
+     * If the hash table has become too full, try removing infrequently used
+     * entries to make a room for the new entry. If failed, enlarge the bucket
+     * array instead.  Quite arbitrarily, we try this when fill factor > 2.
+      */
+    if (unlikely(cache->cc_ntup > cache->cc_nbuckets * 2))
+    {
+        bool rehash = true;
+
+        if (unlikely(catalog_cache_prune_min_age > 0))
+        {
+            /* increase refcount so that the new entry survives pruning */
+            ct->refcount++;
+            rehash = !CatCacheCleanupOldEntries(cache);
+            ct->refcount--;
+        }
+
+        if (likely(rehash))
+            RehashCatCache(cache);
+    } 
 
     return ct;
 }
diff --git a/src/include/utils/catcache.h b/src/include/utils/catcache.h
index 65d816a583..871f51fe34 100644
--- a/src/include/utils/catcache.h
+++ b/src/include/utils/catcache.h
@@ -22,6 +22,7 @@
 
 #include "access/htup.h"
 #include "access/skey.h"
+#include "datatype/timestamp.h"
 #include "lib/ilist.h"
 #include "utils/relcache.h"
 
@@ -119,6 +120,8 @@ typedef struct catctup
     bool        dead;            /* dead but not yet removed? */
     bool        negative;        /* negative cache entry? */
     HeapTupleData tuple;        /* tuple management header */
+    int            naccess;        /* # of access to this entry, up to 2  */
+    TimestampTz    lastaccess;        /* timestamp of the last usage */
 
     /*
      * The tuple may also be a member of at most one CatCList.  (If a single
@@ -185,6 +188,18 @@ typedef struct catcacheheader
     int            ch_ntup;        /* # of tuples in all caches */
 } CatCacheHeader;
 
+/* for guc.c, not PGDLLPMPORT'ed */
+extern int catalog_cache_prune_min_age;
+
+/* source clock for access timestamp of catcache entries */
+extern TimestampTz catcacheclock;
+
+/* SetCatCacheClock - set catcache timestamp source clodk */
+static inline void
+SetCatCacheClock(TimestampTz ts)
+{
+    catcacheclock = ts;
+}
 
 /* this extern duplicates utils/memutils.h... */
 extern PGDLLIMPORT MemoryContext CacheMemoryContext;