Re: Protect syscache from bloating with negative cache entries - Mailing list pgsql-hackers

From Kyotaro Horiguchi
Subject Re: Protect syscache from bloating with negative cache entries
Date
Msg-id 20201119.142536.1685839779534143847.horikyota.ntt@gmail.com
Whole thread Raw
In response to Re: Protect syscache from bloating with negative cache entries  (Robert Haas <robertmhaas@gmail.com>)
Responses Re: Protect syscache from bloating with negative cache entries  (Andres Freund <andres@anarazel.de>)
Re: Protect syscache from bloating with negative cache entries  (Heikki Linnakangas <hlinnaka@iki.fi>)
List pgsql-hackers
Thank you for the comments.

At Tue, 17 Nov 2020 16:22:54 -0500, Robert Haas <robertmhaas@gmail.com> wrote in 
> On Tue, Nov 17, 2020 at 10:46 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> > 0.7% degradation is probably acceptable.
> 
> I haven't looked at this patch in a while and I'm pleased with the way
> it seems to have been redesigned. It seems relatively simple and
> unlikely to cause big headaches. I would say that 0.7% is probably not
> acceptable on a general workload, but it seems fine on a benchmark

Sorry for the confusing notation, "-0.7% degradation" meant +0.7%
*gain*, which I thinks is error.  However, the next patch makes
catcache apparently *faster* so the difference doesn't matter..


> that is specifically designed to be a worst-case for this patch, which
> I gather is what's happening here. I think it would be nice if we
> could enable this feature by default. Does it cause a measurable
> regression on realistic workloads when enabled? I bet a default of 5
> or 10 minutes would help many users.
> 
> One idea for improving things might be to move the "return
> immediately" tests in CatCacheCleanupOldEntries() to the caller, and
> only call this function if they indicate that there is some purpose.
> This would avoid the function call overhead when nothing can be done.
> Perhaps the two tests could be combined into one and simplified. Like,
> suppose the code looks (roughly) like this:
> 
> if (catcacheclock >= time_at_which_we_can_prune)
>     CatCacheCleanupOldEntries(...);

Compiler removes the call (or inlines the function) but of course we
can write that way and it shows the condition for calling the function
better.  The codelet above forgetting consideration on the result of
CatCacheCleanupOldEntries() itself.  The function returns false when
all "old" entries have been invalidated or explicitly removed and we
need to expand the hash in that case.

> To make it that simple, we want catcacheclock and
> time_at_which_we_can_prune to be stored as bare uint64 quantities so
> we don't need TimestampDifference(). And we want
> time_at_which_we_can_prune to be set to PG_UINT64_MAX when the feature
> is disabled. But those both seem like pretty achievable things... and
> it seems like the result would probably be faster than what you have
> now.

The time_at_which_we_can_prune is not global but catcache-local and
needs to change at the time catalog_cache_prune_min_age is changed.

So the next version does as the follwoing:

-    if (CatCacheCleanupOldEntries(cp))
+    if (catcacheclock - cp->cc_oldest_ts > prune_min_age_us &&
+        CatCacheCleanupOldEntries(cp))

On the other hand CatCacheCleanupOldEntries can calcualte the
time_at_which_we_can_prune once at the beginning of the function. That
makes the condition in the loop simpler.

-        TimestampDifference(ct->lastaccess, catcacheclock, &age, &us);
-
-        if (age > catalog_cache_prune_min_age)
+        if (ct->lastaccess < prune_threshold)
        {

> + * per-statement basis and additionaly udpated periodically
> 
> two words spelled wrong

Ugg. Fixed.  Checked all spellings and found another misspelling.

> +void
> +assign_catalog_cache_prune_min_age(int newval, void *extra)
> +{
> + catalog_cache_prune_min_age = newval;
> +}
> 
> hmm, do we need this?

*That* is actually useless, but the function is kept and not it
maintains the internal-version of the GUC parameter (uint64
prune_min_age).

> + /*
> + * 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.
> + */
> + ct->naccess = Min(2, ct->naccess);
> 
> The code doesn't match the comment, it seems, because the limit here
> is 2, not 3. I wonder if this does anything anyway. My intuition is
> that when a catcache entry gets accessed at all it's probably likely
> to get accessed a bunch of times. If there are any meaningful
> thresholds here I'd expect us to be trying to distinguish things like
> 1000+ accesses vs. 100-1000 vs. 10-100 vs. 1-10. Or maybe we don't
> need to distinguish at all and can just have a single mark bit rather
> than a counter.

Agreed. Since I don't see a clear criteria for the threshold of the
counter, I removed the naccess and related lines.

I did the following changes in the attached.

1. Removed naccess and related lines.

2. Moved the precheck condition out of CatCacheCleanupOldEntries() to
  RehashCatCache().

3. Use uint64 direct comparison instead of TimestampDifference().

4. Removed CatCTup.dead flag.

Performance measurement on the attached showed better result about
searching but maybe worse for cache entry creation.  Each time number
is the mean of 10 runs.

# Cacache (negative) entry creation
           :  time(ms) (% to master)
master     :  3965.61    (100.0)
patched-off:  4040.93    (101.9)
patched-on :  4032.22    (101.7)

# Searching negative cache entries
master     :  8173.46    (100.0)
patched-off:  7983.43    ( 97.7)
patched-on :  8049.88    ( 98.5)

# Creation, searching and expiration
master     :  6393.23    (100.0)
patched-off:  6527.94    (102.1)
patched-on : 15880.01    (248.4)


That is, catcache searching gets faster by 2-3% but creation gets
slower by about 2%. If I moved the condition of 2 further up to
CatalogCacheCreateEntry(), that degradation reduced to 0.6%.

# Cacache (negative) entry creation
master      :  3967.45   (100.0)
patched-off :  3990.43   (100.6)
patched-on  :  4108.96   (103.6)

# Searching negative cache entries
master      :  8106.53   (100.0)
patched-off :  8036.61   ( 99.1)
patched-on  :  8058.18   ( 99.4)

# Creation, searching and expiration
master      :  6395.00   (100.0)
patched-off :  6416.57   (100.3)
patched-on  : 15830.91   (247.6)

It doesn't get smaller if I reverted the changed lines in
CatalogCacheCreateEntry()..


regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
From 990514a853ad92b2d929cc026724194831ef8793 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyoga.ntt@gmail.com>
Date: Wed, 18 Nov 2020 16:54:31 +0900
Subject: [PATCH v5 1/3] CatCache expiration feature

---
 src/backend/access/transam/xact.c  |  3 ++
 src/backend/utils/cache/catcache.c | 87 +++++++++++++++++++++++++++++-
 src/backend/utils/misc/guc.c       | 12 +++++
 src/include/utils/catcache.h       | 19 +++++++
 4 files changed, 120 insertions(+), 1 deletion(-)

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 03c553e7ea..4a2a90ce0c 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -1086,6 +1086,9 @@ static void
 AtStart_Cache(void)
 {
     AcceptInvalidationMessages();
+
+    if (xactStartTimestamp != 0)
+        SetCatCacheClock(xactStartTimestamp);
 }
 
 /*
diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c
index 3613ae5f44..1ebcc7dcd3 100644
--- a/src/backend/utils/cache/catcache.c
+++ b/src/backend/utils/cache/catcache.c
@@ -38,6 +38,7 @@
 #include "utils/rel.h"
 #include "utils/resowner_private.h"
 #include "utils/syscache.h"
+#include "utils/timestamp.h"
 
 
  /* #define CACHEDEBUG */    /* turns DEBUG elogs on */
@@ -60,9 +61,19 @@
 #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 = -1;
+uint64    prune_min_age_us;
+
 /* Cache management header --- pointer is NULL until created */
 static CatCacheHeader *CacheHdr = NULL;
 
+/* Clock for the last accessed time of a catcache entry. */
+uint64    catcacheclock = 0;
+
 static inline HeapTuple SearchCatCacheInternal(CatCache *cache,
                                                int nkeys,
                                                Datum v1, Datum v2,
@@ -74,6 +85,7 @@ static pg_noinline HeapTuple SearchCatCacheMiss(CatCache *cache,
                                                 Index hashIndex,
                                                 Datum v1, Datum v2,
                                                 Datum v3, Datum v4);
+static bool CatCacheCleanupOldEntries(CatCache *cp);
 
 static uint32 CatalogCacheComputeHashValue(CatCache *cache, int nkeys,
                                            Datum v1, Datum v2, Datum v3, Datum v4);
@@ -99,6 +111,15 @@ static void CatCacheFreeKeys(TupleDesc tupdesc, int nkeys, int *attnos,
 static void CatCacheCopyKeys(TupleDesc tupdesc, int nkeys, int *attnos,
                              Datum *srckeys, Datum *dstkeys);
 
+/* GUC assign function */
+void
+assign_catalog_cache_prune_min_age(int newval, void *extra)
+{
+    if (newval < 0)
+        prune_min_age_us = UINT64_MAX;
+    else
+        prune_min_age_us = ((uint64) newval) * USECS_PER_SEC;
+}
 
 /*
  *                    internal support functions
@@ -1264,6 +1285,9 @@ SearchCatCacheInternal(CatCache *cache,
          */
         dlist_move_head(bucket, &ct->cache_elem);
 
+        /* Record the last access timestamp */
+        ct->lastaccess = catcacheclock;
+
         /*
          * If it's a positive entry, bump its refcount and return it. If it's
          * negative, we can report failure to the caller.
@@ -1425,6 +1449,61 @@ SearchCatCacheMiss(CatCache *cache,
     return &ct->tuple;
 }
 
+/*
+ * 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;
+    long    oldest_ts = catcacheclock;
+    uint64    prune_threshold = catcacheclock - prune_min_age_us;
+
+    /* 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);
+
+            /* Don't remove referenced entries */
+            if (ct->refcount == 0 &&
+                (ct->c_list == NULL || ct->c_list->refcount == 0))
+            {
+                if (ct->lastaccess < prune_threshold)
+                {
+                    CatCacheRemoveCTup(cp, ct);
+                    nremoved++;
+
+                    /* don't let the removed entry update oldest_ts */
+                    continue;
+                }
+            }
+
+            /* update the oldest timestamp if the entry remains alive */
+            if (ct->lastaccess < oldest_ts)
+                oldest_ts = ct->lastaccess;
+        }
+    }
+
+    cp->cc_oldest_ts = oldest_ts;
+
+    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;
+}
+
 /*
  *    ReleaseCatCache
  *
@@ -1888,6 +1967,7 @@ CatalogCacheCreateEntry(CatCache *cache, HeapTuple ntp, Datum *arguments,
     ct->dead = false;
     ct->negative = negative;
     ct->hash_value = hashValue;
+    ct->lastaccess = catcacheclock;
 
     dlist_push_head(&cache->cc_bucket[hashIndex], &ct->cache_elem);
 
@@ -1899,7 +1979,12 @@ CatalogCacheCreateEntry(CatCache *cache, HeapTuple ntp, Datum *arguments,
      * arbitrarily, we enlarge when fill factor > 2.
      */
     if (cache->cc_ntup > cache->cc_nbuckets * 2)
-        RehashCatCache(cache);
+    {
+        /* try removing old entries before expanding hash */
+        if (catcacheclock - cache->cc_oldest_ts < prune_min_age_us ||
+            !CatCacheCleanupOldEntries(cache))
+            RehashCatCache(cache);
+    }
 
     return ct;
 }
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index bb34630e8e..95213853aa 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -88,6 +88,7 @@
 #include "utils/acl.h"
 #include "utils/builtins.h"
 #include "utils/bytea.h"
+#include "utils/catcache.h"
 #include "utils/float.h"
 #include "utils/guc_tables.h"
 #include "utils/memutils.h"
@@ -3399,6 +3400,17 @@ static struct config_int ConfigureNamesInt[] =
         check_huge_page_size, NULL, NULL
     },
 
+    {
+        {"catalog_cache_prune_min_age", PGC_USERSET, RESOURCES_MEM,
+            gettext_noop("System catalog cache entries that are living unused more than this seconds are considered
forremoval."),
 
+            gettext_noop("The value of -1 turns off pruning."),
+            GUC_UNIT_S
+        },
+        &catalog_cache_prune_min_age,
+        -1, -1, INT_MAX,
+        NULL, assign_catalog_cache_prune_min_age, NULL
+    },
+
     /* End-of-list marker */
     {
         {NULL, 0, 0, NULL, NULL}, NULL, 0, 0, 0, NULL, NULL, NULL
diff --git a/src/include/utils/catcache.h b/src/include/utils/catcache.h
index f4aa316604..81587c3fe6 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"
 
@@ -61,6 +62,7 @@ typedef struct catcache
     slist_node    cc_next;        /* list link */
     ScanKeyData cc_skey[CATCACHE_MAXKEYS];    /* precomputed key info for heap
                                              * scans */
+    uint64        cc_oldest_ts;    /* timestamp (us) of the oldest tuple */
 
     /*
      * Keep these at the end, so that compiling catcache.c with CATCACHE_STATS
@@ -119,6 +121,7 @@ typedef struct catctup
     bool        dead;            /* dead but not yet removed? */
     bool        negative;        /* negative cache entry? */
     HeapTupleData tuple;        /* tuple management header */
+    uint64        lastaccess;        /* timestamp in us of the last usage */
 
     /*
      * The tuple may also be a member of at most one CatCList.  (If a single
@@ -189,6 +192,22 @@ typedef struct catcacheheader
 /* this extern duplicates utils/memutils.h... */
 extern PGDLLIMPORT MemoryContext CacheMemoryContext;
 
+
+/* for guc.c, not PGDLLPMPORT'ed */
+extern int catalog_cache_prune_min_age;
+
+/* source clock for access timestamp of catcache entries */
+extern uint64 catcacheclock;
+
+/* SetCatCacheClock - set catcache timestamp source clock */
+static inline void
+SetCatCacheClock(TimestampTz ts)
+{
+    catcacheclock = (uint64) ts;
+}
+
+extern void assign_catalog_cache_prune_min_age(int newval, void *extra);
+
 extern void CreateCacheMemoryContext(void);
 
 extern CatCache *InitCatCache(int id, Oid reloid, Oid indexoid,
-- 
2.18.4

From 2bc7eb221768ee8484fb65db48fa16f6e2c4b347 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyoga.ntt@gmail.com>
Date: Wed, 18 Nov 2020 16:57:05 +0900
Subject: [PATCH v5 2/3] Remove "dead" flag from catcache tuple

---
 src/backend/utils/cache/catcache.c | 43 +++++++++++++-----------------
 src/include/utils/catcache.h       | 10 -------
 2 files changed, 18 insertions(+), 35 deletions(-)

diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c
index 1ebcc7dcd3..3e6c4720dc 100644
--- a/src/backend/utils/cache/catcache.c
+++ b/src/backend/utils/cache/catcache.c
@@ -480,6 +480,13 @@ CatCacheRemoveCTup(CatCache *cache, CatCTup *ct)
     Assert(ct->refcount == 0);
     Assert(ct->my_cache == cache);
 
+    /* delink from linked list if not yet */
+    if (ct->cache_elem.prev)
+    {
+        dlist_delete(&ct->cache_elem);
+        ct->cache_elem.prev = NULL;
+    }
+
     if (ct->c_list)
     {
         /*
@@ -487,14 +494,10 @@ CatCacheRemoveCTup(CatCache *cache, CatCTup *ct)
          * which will recurse back to me, and the recursive call will do the
          * work.  Set the "dead" flag to make sure it does recurse.
          */
-        ct->dead = true;
         CatCacheRemoveCList(cache, ct->c_list);
         return;                    /* nothing left to do */
     }
 
-    /* delink from linked list */
-    dlist_delete(&ct->cache_elem);
-
     /*
      * Free keys when we're dealing with a negative entry, normal entries just
      * point into tuple, allocated together with the CatCTup.
@@ -534,7 +537,7 @@ CatCacheRemoveCList(CatCache *cache, CatCList *cl)
         /* if the member is dead and now has no references, remove it */
         if (
 #ifndef CATCACHE_FORCE_RELEASE
-            ct->dead &&
+            ct->cache_elem.prev == NULL &&
 #endif
             ct->refcount == 0)
             CatCacheRemoveCTup(cache, ct);
@@ -609,7 +612,9 @@ CatCacheInvalidate(CatCache *cache, uint32 hashValue)
             if (ct->refcount > 0 ||
                 (ct->c_list && ct->c_list->refcount > 0))
             {
-                ct->dead = true;
+                dlist_delete(&ct->cache_elem);
+                ct->cache_elem.prev = NULL;
+
                 /* list, if any, was marked dead above */
                 Assert(ct->c_list == NULL || ct->c_list->dead);
             }
@@ -688,7 +693,8 @@ ResetCatalogCache(CatCache *cache)
             if (ct->refcount > 0 ||
                 (ct->c_list && ct->c_list->refcount > 0))
             {
-                ct->dead = true;
+                dlist_delete(&ct->cache_elem);
+                ct->cache_elem.prev = NULL;
                 /* list, if any, was marked dead above */
                 Assert(ct->c_list == NULL || ct->c_list->dead);
             }
@@ -1268,9 +1274,6 @@ SearchCatCacheInternal(CatCache *cache,
     {
         ct = dlist_container(CatCTup, cache_elem, iter.cur);
 
-        if (ct->dead)
-            continue;            /* ignore dead entries */
-
         if (ct->hash_value != hashValue)
             continue;            /* quickly skip entry if wrong hash val */
 
@@ -1522,7 +1525,6 @@ ReleaseCatCache(HeapTuple tuple)
                                   offsetof(CatCTup, tuple));
 
     /* Safety checks to ensure we were handed a cache entry */
-    Assert(ct->ct_magic == CT_MAGIC);
     Assert(ct->refcount > 0);
 
     ct->refcount--;
@@ -1530,7 +1532,7 @@ ReleaseCatCache(HeapTuple tuple)
 
     if (
 #ifndef CATCACHE_FORCE_RELEASE
-        ct->dead &&
+        ct->cache_elem.prev == NULL &&
 #endif
         ct->refcount == 0 &&
         (ct->c_list == NULL || ct->c_list->refcount == 0))
@@ -1737,8 +1739,8 @@ SearchCatCacheList(CatCache *cache,
             {
                 ct = dlist_container(CatCTup, cache_elem, iter.cur);
 
-                if (ct->dead || ct->negative)
-                    continue;    /* ignore dead and negative entries */
+                if (ct->negative)
+                    continue;    /* ignore negative entries */
 
                 if (ct->hash_value != hashValue)
                     continue;    /* quickly skip entry if wrong hash val */
@@ -1799,14 +1801,13 @@ SearchCatCacheList(CatCache *cache,
     {
         foreach(ctlist_item, ctlist)
         {
+            Assert (ct->cache_elem.prev != NULL);
+
             ct = (CatCTup *) lfirst(ctlist_item);
             Assert(ct->c_list == NULL);
             Assert(ct->refcount > 0);
             ct->refcount--;
             if (
-#ifndef CATCACHE_FORCE_RELEASE
-                ct->dead &&
-#endif
                 ct->refcount == 0 &&
                 (ct->c_list == NULL || ct->c_list->refcount == 0))
                 CatCacheRemoveCTup(cache, ct);
@@ -1834,9 +1835,6 @@ SearchCatCacheList(CatCache *cache,
         /* release the temporary refcount on the member */
         Assert(ct->refcount > 0);
         ct->refcount--;
-        /* mark list dead if any members already dead */
-        if (ct->dead)
-            cl->dead = true;
     }
     Assert(i == nmembers);
 
@@ -1960,11 +1958,9 @@ CatalogCacheCreateEntry(CatCache *cache, HeapTuple ntp, Datum *arguments,
      * Finish initializing the CatCTup header, and add it to the cache's
      * linked list and counts.
      */
-    ct->ct_magic = CT_MAGIC;
     ct->my_cache = cache;
     ct->c_list = NULL;
     ct->refcount = 0;            /* for the moment */
-    ct->dead = false;
     ct->negative = negative;
     ct->hash_value = hashValue;
     ct->lastaccess = catcacheclock;
@@ -2158,9 +2154,6 @@ PrintCatCacheLeakWarning(HeapTuple tuple)
     CatCTup    *ct = (CatCTup *) (((char *) tuple) -
                                   offsetof(CatCTup, tuple));
 
-    /* Safety check to ensure we were handed a cache entry */
-    Assert(ct->ct_magic == CT_MAGIC);
-
     elog(WARNING, "cache reference leak: cache %s (%d), tuple %u/%u has count %d",
          ct->my_cache->cc_relname, ct->my_cache->id,
          ItemPointerGetBlockNumber(&(tuple->t_self)),
diff --git a/src/include/utils/catcache.h b/src/include/utils/catcache.h
index 81587c3fe6..36940f4e3b 100644
--- a/src/include/utils/catcache.h
+++ b/src/include/utils/catcache.h
@@ -87,9 +87,6 @@ typedef struct catcache
 
 typedef struct catctup
 {
-    int            ct_magic;        /* for identifying CatCTup entries */
-#define CT_MAGIC   0x57261502
-
     uint32        hash_value;        /* hash value for this tuple's keys */
 
     /*
@@ -106,19 +103,12 @@ typedef struct catctup
     dlist_node    cache_elem;        /* list member of per-bucket list */
 
     /*
-     * A tuple marked "dead" must not be returned by subsequent searches.
-     * However, it won't be physically deleted from the cache until its
-     * refcount goes to zero.  (If it's a member of a CatCList, the list's
-     * refcount must go to zero, too; also, remember to mark the list dead at
-     * the same time the tuple is marked.)
-     *
      * 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
      * negative entries is identical.
      */
     int            refcount;        /* number of active references */
-    bool        dead;            /* dead but not yet removed? */
     bool        negative;        /* negative cache entry? */
     HeapTupleData tuple;        /* tuple management header */
     uint64        lastaccess;        /* timestamp in us of the last usage */
-- 
2.18.4

From 61806132ea82e90997e38a6cb0fa0d74bc3c4c2b Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyoga.ntt@gmail.com>
Date: Wed, 18 Nov 2020 16:56:41 +0900
Subject: [PATCH v5 3/3] catcachebench

---
 contrib/catcachebench/Makefile               |  17 +
 contrib/catcachebench/catcachebench--0.0.sql |  14 +
 contrib/catcachebench/catcachebench.c        | 330 +++++++++++++++++++
 contrib/catcachebench/catcachebench.control  |   6 +
 src/backend/utils/cache/catcache.c           |  33 ++
 src/backend/utils/cache/syscache.c           |   2 +-
 6 files changed, 401 insertions(+), 1 deletion(-)
 create mode 100644 contrib/catcachebench/Makefile
 create mode 100644 contrib/catcachebench/catcachebench--0.0.sql
 create mode 100644 contrib/catcachebench/catcachebench.c
 create mode 100644 contrib/catcachebench/catcachebench.control

diff --git a/contrib/catcachebench/Makefile b/contrib/catcachebench/Makefile
new file mode 100644
index 0000000000..0478818b25
--- /dev/null
+++ b/contrib/catcachebench/Makefile
@@ -0,0 +1,17 @@
+MODULE_big = catcachebench
+OBJS = catcachebench.o
+
+EXTENSION = catcachebench
+DATA = catcachebench--0.0.sql
+PGFILEDESC = "catcachebench - benchmark for catcache pruning feature"
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = contrib/catcachebench
+top_builddir = ../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/contrib/catcachebench/catcachebench--0.0.sql b/contrib/catcachebench/catcachebench--0.0.sql
new file mode 100644
index 0000000000..ea9cd62abb
--- /dev/null
+++ b/contrib/catcachebench/catcachebench--0.0.sql
@@ -0,0 +1,14 @@
+/* contrib/catcachebench/catcachebench--0.0.sql */
+
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION catcachebench" to load this file. \quit
+
+CREATE FUNCTION catcachebench(IN type int)
+RETURNS double precision
+AS 'MODULE_PATHNAME', 'catcachebench'
+LANGUAGE C STRICT VOLATILE;
+
+CREATE FUNCTION catcachereadstats(OUT catid int, OUT reloid oid, OUT searches bigint, OUT hits bigint, OUT neg_hits
bigint)
+RETURNS SETOF record
+AS 'MODULE_PATHNAME', 'catcachereadstats'
+LANGUAGE C STRICT VOLATILE;
diff --git a/contrib/catcachebench/catcachebench.c b/contrib/catcachebench/catcachebench.c
new file mode 100644
index 0000000000..b5a4d794ed
--- /dev/null
+++ b/contrib/catcachebench/catcachebench.c
@@ -0,0 +1,330 @@
+/*
+ * catcachebench: test code for cache pruning feature
+ */
+/* #define CATCACHE_STATS */
+#include "postgres.h"
+#include "catalog/pg_type.h"
+#include "catalog/pg_statistic.h"
+#include "executor/spi.h"
+#include "funcapi.h"
+#include "libpq/pqsignal.h"
+#include "utils/catcache.h"
+#include "utils/syscache.h"
+#include "utils/timestamp.h"
+
+Oid        tableoids[10000];
+int        ntables = 0;
+int16    attnums[1000];
+int        natts = 0;
+
+PG_MODULE_MAGIC;
+
+double catcachebench1(void);
+double catcachebench2(void);
+double catcachebench3(void);
+void collectinfo(void);
+void catcachewarmup(void);
+
+PG_FUNCTION_INFO_V1(catcachebench);
+PG_FUNCTION_INFO_V1(catcachereadstats);
+
+extern void CatalogCacheFlushCatalog2(Oid catId);
+extern int64 catcache_called;
+extern CatCache *SysCache[];
+
+typedef struct catcachestatsstate
+{
+    TupleDesc tupd;
+    int          catId;
+} catcachestatsstate;
+
+Datum
+catcachereadstats(PG_FUNCTION_ARGS)
+{
+    catcachestatsstate *state_data = NULL;
+    FuncCallContext *fctx;
+
+    if (SRF_IS_FIRSTCALL())
+    {
+        TupleDesc    tupdesc;
+        MemoryContext mctx;
+
+        fctx = SRF_FIRSTCALL_INIT();
+        mctx = MemoryContextSwitchTo(fctx->multi_call_memory_ctx);
+
+        state_data = palloc(sizeof(catcachestatsstate));
+
+        /* Build a tuple descriptor for our result type */
+        if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+            elog(ERROR, "return type must be a row type");
+
+        state_data->tupd = tupdesc;
+        state_data->catId = 0;
+
+        fctx->user_fctx = state_data;
+
+        MemoryContextSwitchTo(mctx);
+    }
+
+    fctx = SRF_PERCALL_SETUP();
+    state_data = fctx->user_fctx;
+
+    if (state_data->catId < SysCacheSize)
+    {
+        Datum    values[5];
+        bool    nulls[5];
+        HeapTuple    resulttup;
+        Datum    result;
+        int        catId = state_data->catId++;
+
+        memset(nulls, 0, sizeof(nulls));
+        memset(values, 0, sizeof(values));
+        values[0] = Int16GetDatum(catId);
+        values[1] = ObjectIdGetDatum(SysCache[catId]->cc_reloid);
+#ifdef CATCACHE_STATS        
+        values[2] = Int64GetDatum(SysCache[catId]->cc_searches);
+        values[3] = Int64GetDatum(SysCache[catId]->cc_hits);
+        values[4] = Int64GetDatum(SysCache[catId]->cc_neg_hits);
+#endif
+        resulttup = heap_form_tuple(state_data->tupd, values, nulls);
+        result = HeapTupleGetDatum(resulttup);
+
+        SRF_RETURN_NEXT(fctx, result);
+    }
+
+    SRF_RETURN_DONE(fctx);
+}
+
+Datum
+catcachebench(PG_FUNCTION_ARGS)
+{
+    int        testtype = PG_GETARG_INT32(0);
+    double    ms;
+
+    collectinfo();
+
+    /* flush the catalog -- safe? don't mind. */
+    CatalogCacheFlushCatalog2(StatisticRelationId);
+
+    switch (testtype)
+    {
+    case 0:
+        catcachewarmup(); /* prewarm of syscatalog */
+        PG_RETURN_NULL();
+    case 1:
+        ms = catcachebench1(); break;
+    case 2:
+        ms = catcachebench2(); break;
+    case 3:
+        ms = catcachebench3(); break;
+    default:
+        elog(ERROR, "Invalid test type: %d", testtype);
+    }
+
+    PG_RETURN_DATUM(Float8GetDatum(ms));
+}
+
+/*
+ * fetch all attribute entires of all tables.
+ */
+double
+catcachebench1(void)
+{
+    int t, a;
+    instr_time    start,
+                duration;
+
+    PG_SETMASK(&BlockSig);
+    INSTR_TIME_SET_CURRENT(start);
+    for (t = 0 ; t < ntables ; t++)
+    {
+        for (a = 0 ; a < natts ; a++)
+        {
+            HeapTuple tup;
+
+            tup = SearchSysCache3(STATRELATTINH,
+                                  ObjectIdGetDatum(tableoids[t]),
+                                  Int16GetDatum(attnums[a]),
+                                  BoolGetDatum(false));
+            /* should be null, but.. */
+            if (HeapTupleIsValid(tup))
+                ReleaseSysCache(tup);
+        }
+    }
+    INSTR_TIME_SET_CURRENT(duration);
+    INSTR_TIME_SUBTRACT(duration, start);
+    PG_SETMASK(&UnBlockSig);
+
+    return INSTR_TIME_GET_MILLISEC(duration);
+};
+
+/*
+ * fetch all attribute entires of a table 6000 times.
+ */
+double
+catcachebench2(void)
+{
+    int t, a;
+    instr_time    start,
+                duration;
+
+    PG_SETMASK(&BlockSig);
+    INSTR_TIME_SET_CURRENT(start);
+    for (t = 0 ; t < 240000 ; t++)
+    {
+        for (a = 0 ; a < natts ; a++)
+        {
+            HeapTuple tup;
+
+            tup = SearchSysCache3(STATRELATTINH,
+                                  ObjectIdGetDatum(tableoids[0]),
+                                  Int16GetDatum(attnums[a]),
+                                  BoolGetDatum(false));
+            /* should be null, but.. */
+            if (HeapTupleIsValid(tup))
+                ReleaseSysCache(tup);
+        }
+    }
+    INSTR_TIME_SET_CURRENT(duration);
+    INSTR_TIME_SUBTRACT(duration, start);
+    PG_SETMASK(&UnBlockSig);
+
+    return INSTR_TIME_GET_MILLISEC(duration);
+};
+
+/*
+ * fetch all attribute entires of all tables twice with having expiration
+ * happen.
+ */
+double
+catcachebench3(void)
+{
+    const int clock_step = 1000;
+    int i, t, a;
+    instr_time    start,
+                duration;
+
+    PG_SETMASK(&BlockSig);
+    INSTR_TIME_SET_CURRENT(start);
+    for (i = 0 ; i < 4 ; i++)
+    {
+        int ct = clock_step;
+
+        for (t = 0 ; t < ntables ; t++)
+        {
+            /*
+             * catcacheclock is updated by transaction timestamp, so needs to
+             * be updated by other means for this test to work. Here I choosed
+             * to update the clock every 1000 tables scan.
+             */
+            if (--ct < 0)
+            {
+                SetCatCacheClock(GetCurrentTimestamp());
+                ct = clock_step;
+            }
+            for (a = 0 ; a < natts ; a++)
+            {
+                HeapTuple tup;
+
+                tup = SearchSysCache3(STATRELATTINH,
+                                      ObjectIdGetDatum(tableoids[t]),
+                                      Int16GetDatum(attnums[a]),
+                                      BoolGetDatum(false));
+                /* should be null, but.. */
+                if (HeapTupleIsValid(tup))
+                    ReleaseSysCache(tup);
+            }
+        }
+    }
+    INSTR_TIME_SET_CURRENT(duration);
+    INSTR_TIME_SUBTRACT(duration, start);
+    PG_SETMASK(&UnBlockSig);
+
+    return INSTR_TIME_GET_MILLISEC(duration);
+};
+
+void
+catcachewarmup(void)
+{
+    int t, a;
+
+    /* load up catalog tables */
+    for (t = 0 ; t < ntables ; t++)
+    {
+        for (a = 0 ; a < natts ; a++)
+        {
+            HeapTuple tup;
+
+            tup = SearchSysCache3(STATRELATTINH,
+                                  ObjectIdGetDatum(tableoids[t]),
+                                  Int16GetDatum(attnums[a]),
+                                  BoolGetDatum(false));
+            /* should be null, but.. */
+            if (HeapTupleIsValid(tup))
+                ReleaseSysCache(tup);
+        }
+    }
+}
+
+void
+collectinfo(void)
+{
+    int ret;
+    Datum    values[10000];
+    bool    nulls[10000];
+    Oid        types0[] = {OIDOID};
+    int i;
+
+    ntables = 0;
+    natts = 0;
+
+    SPI_connect();
+    /* collect target tables */
+    ret = SPI_execute("select oid from pg_class where relnamespace = (select oid from pg_namespace where nspname =
\'test\')",
+                      true, 0);
+    if (ret != SPI_OK_SELECT)
+        elog(ERROR, "Failed 1");
+    if (SPI_processed == 0)
+        elog(ERROR, "no relation found in schema \"test\"");
+    if (SPI_processed > 10000)
+        elog(ERROR, "too many relation found in schema \"test\"");
+
+    for (i = 0 ; i < SPI_processed ; i++)
+    {
+        heap_deform_tuple(SPI_tuptable->vals[i], SPI_tuptable->tupdesc,
+                          values, nulls);
+        if (nulls[0])
+            elog(ERROR, "Failed 2");
+
+        tableoids[ntables++] = DatumGetObjectId(values[0]);
+    }
+    SPI_finish();
+    elog(DEBUG1, "%d tables found", ntables);
+
+    values[0] = ObjectIdGetDatum(tableoids[0]);
+    nulls[0] = false;
+    SPI_connect();
+    ret = SPI_execute_with_args("select attnum from pg_attribute where attrelid = (select oid from pg_class where oid
=$1)",
 
+                                1, types0, values, NULL, true, 0);
+    if (SPI_processed == 0)
+        elog(ERROR, "no attribute found in table %d", tableoids[0]);
+    if (SPI_processed > 10000)
+        elog(ERROR, "too many relation found in table %d", tableoids[0]);
+    
+    /* collect target attributes. assuming all tables have the same attnums */
+    for (i = 0 ; i < SPI_processed ; i++)
+    {
+        int16 attnum;
+
+        heap_deform_tuple(SPI_tuptable->vals[i], SPI_tuptable->tupdesc,
+                          values, nulls);
+        if (nulls[0])
+            elog(ERROR, "Failed 3");
+        attnum = DatumGetInt16(values[0]);
+
+        if (attnum > 0)
+            attnums[natts++] = attnum;
+    }
+    SPI_finish();
+    elog(DEBUG1, "%d attributes found", natts);
+}
diff --git a/contrib/catcachebench/catcachebench.control b/contrib/catcachebench/catcachebench.control
new file mode 100644
index 0000000000..3fc9d2e420
--- /dev/null
+++ b/contrib/catcachebench/catcachebench.control
@@ -0,0 +1,6 @@
+# catcachebench
+
+comment = 'benchmark for catcache pruning'
+default_version = '0.0'
+module_pathname = '$libdir/catcachebench'
+relocatable = true
diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c
index 3e6c4720dc..294d906416 100644
--- a/src/backend/utils/cache/catcache.c
+++ b/src/backend/utils/cache/catcache.c
@@ -767,6 +767,39 @@ CatalogCacheFlushCatalog(Oid catId)
     CACHE_elog(DEBUG2, "end of CatalogCacheFlushCatalog call");
 }
 
+
+/* FUNCTION FOR BENCHMARKING */
+void
+CatalogCacheFlushCatalog2(Oid catId)
+{
+    slist_iter    iter;
+
+    CACHE_elog(DEBUG2, "CatalogCacheFlushCatalog called for %u", catId);
+
+    slist_foreach(iter, &CacheHdr->ch_caches)
+    {
+        CatCache   *cache = slist_container(CatCache, cc_next, iter.cur);
+
+        /* Does this cache store tuples of the target catalog? */
+        if (cache->cc_reloid == catId)
+        {
+            /* Yes, so flush all its contents */
+            ResetCatalogCache(cache);
+
+            /* Tell inval.c to call syscache callbacks for this cache */
+            CallSyscacheCallbacks(cache->id, 0);
+
+            cache->cc_nbuckets = 128;
+            pfree(cache->cc_bucket);
+            cache->cc_bucket = palloc0(128 * sizeof(dlist_head));
+            elog(LOG, "Catcache reset");
+        }
+    }
+
+    CACHE_elog(DEBUG2, "end of CatalogCacheFlushCatalog call");
+}
+/* END: FUNCTION FOR BENCHMARKING */
+
 /*
  *        InitCatCache
  *
diff --git a/src/backend/utils/cache/syscache.c b/src/backend/utils/cache/syscache.c
index 809b27a038..e83b3f66d1 100644
--- a/src/backend/utils/cache/syscache.c
+++ b/src/backend/utils/cache/syscache.c
@@ -982,7 +982,7 @@ static const struct cachedesc cacheinfo[] = {
     }
 };
 
-static CatCache *SysCache[SysCacheSize];
+CatCache *SysCache[SysCacheSize];
 
 static bool CacheInitialized = false;
 
-- 
2.18.4


pgsql-hackers by date:

Previous
From: "osumi.takamichi@fujitsu.com"
Date:
Subject: RE: Disable WAL logging to speed up data loading
Next
From: Greg Stark
Date:
Subject: Re: Is postgres ready for 2038?