Re: Protect syscache from bloating with negative cache entries

At Fri, 22 Dec 2017 13:47:16 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in
<20171222.134716.88479707.horiguchi.kyotaro@lab.ntt.co.jp>
> Anyway, I think we are reached to a consensus that the
> time-tick-based expiration is promising. So I'll work on the way
> as the first step.

So this is the patch. It gets simpler.

# I became to think that the second step is not needed.

I'm not sure that no syscache aceess happens outside a statement
but the operations that lead to the bloat seem to be performed
while processing of a statement. So statement timestamp seems
sufficient as the aging clock.

At first I tried the simple strategy that removes entries that
have been left alone for 30 minutes or more but I still want to
alleviate the quick bloat (by non-reused entries) so I introduced
together a clock-sweep like aging mechanism. An entry is created
with naccessed = 0, then incremented up to 2 each time it is
accessed. Removal side decrements naccessed of entriies older
than 600 seconds then actually removes if it becomes 0. Entries
that just created and not used will go off in 600 seconds and
entries that have been accessed several times have 1800 seconds'
grace after the last acccess.

We could shrink bucket array together but I didn't since it is
not so large and is prone to grow up to the same size shortly
again.


regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
*** a/src/backend/access/transam/xact.c
--- b/src/backend/access/transam/xact.c
***************
*** 733,738 **** void
--- 733,741 ----
  SetCurrentStatementStartTimestamp(void)
  {
      stmtStartTimestamp = GetCurrentTimestamp();
+ 
+     /* Set this time stamp as aproximated current time */
+     SetCatCacheClock(stmtStartTimestamp);
  }
  
  /*
*** a/src/backend/utils/cache/catcache.c
--- b/src/backend/utils/cache/catcache.c
***************
*** 74,79 ****
--- 74,82 ----
  /* Cache management header --- pointer is NULL until created */
  static CatCacheHeader *CacheHdr = NULL;
  
+ /* Timestamp used for any operation on caches. */
+ TimestampTz    catcacheclock = 0;
+ 
  static inline HeapTuple SearchCatCacheInternal(CatCache *cache,
                         int nkeys,
                         Datum v1, Datum v2,
***************
*** 866,875 **** InitCatCache(int id,
--- 869,969 ----
       */
      MemoryContextSwitchTo(oldcxt);
  
+     /* initilize catcache reference clock if haven't done yet */
+     if (catcacheclock == 0)
+         catcacheclock = GetCurrentTimestamp();
+ 
      return cp;
  }
  
  /*
+  * Remove entries that haven't been accessed for a certain time.
+  *
+  * Sometimes catcache entries are left unremoved for several reasons. We
+  * cannot allow them to eat up the usable memory and still it is better to
+  * remove entries that are no longer accessed from the perspective of memory
+  * performance ratio. Unfortunately we cannot predict that but we can assume
+  * that entries that are not accessed for long time no longer contribute to
+  * performance.
+  */
+ static bool
+ CatCacheCleanupOldEntries(CatCache *cp)
+ {
+     int            i;
+     int            nremoved = 0;
+ #ifdef CATCACHE_STATS
+     int            ntotal = 0;
+     int            tm[] = {30, 60, 600, 1200, 1800, 0};
+     int            cn[6] = {0, 0, 0, 0, 0};
+     int            cage[3] = {0, 0, 0};
+ #endif
+ 
+     /* Move all entries from old hash table to new. */
+     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 s;
+             int us;
+ 
+ 
+             TimestampDifference(ct->lastaccess, catcacheclock, &s, &us);
+ 
+ #ifdef CATCACHE_STATS
+             {
+                 int j;
+ 
+                 ntotal++;
+                 for (j = 0 ; tm[j] != 0 && s > tm[j] ; j++);
+                 if (tm[j] == 0) j--;
+                 cn[j]++;
+             }
+ #endif
+ 
+             /*
+              * Remove entries older than 600 seconds but not recently used.
+              * Entries that are not accessed after creation are removed in 600
+              * seconds, and that has been used several times are removed after
+              * 30 minumtes ignorance. We don't try shrink buckets since they
+              * are not the major part of syscache bloat and they are expected
+              * to be filled shortly again.
+              */
+             if (s > 600)
+             {
+ #ifdef CATCACHE_STATS
+                 Assert (ct->naccess >= 0 && ct->naccess <= 2);
+                 cage[ct->naccess]++;
+ #endif
+                 if (ct->naccess > 0)
+                     ct->naccess--;
+                 else
+                 {
+                     if (!ct->c_list || ct->c_list->refcount == 0)
+                     {
+                         CatCacheRemoveCTup(cp, ct);
+                         nremoved++;
+                     }
+                 }
+             }
+         }
+     }
+ 
+ #ifdef CATCACHE_STATS
+     ereport(DEBUG2,
+             (errmsg ("removed %d/%d, age(-30s:%d, -60s:%d, -600s:%d, -1200s:%d, -1800:%d) naccessed(0:%d, 1:%d,
2:%d)",
+                 nremoved, ntotal,
+                 cn[0], cn[1], cn[2], cn[3], cn[4],
+                 cage[0], cage[1], cage[2]),
+              errhidestmt(true)));
+ #endif
+ 
+     return nremoved > 0;
+ }
+ 
+ /*
   * Enlarge a catcache, doubling the number of buckets.
   */
  static void
***************
*** 1282,1287 **** SearchCatCacheInternal(CatCache *cache,
--- 1376,1389 ----
           */
          dlist_move_head(bucket, &ct->cache_elem);
  
+ 
+         /*
+          * Update the last access time of this entry
+          */
+         if (ct->naccess < 2)
+             ct->naccess++;
+         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.
***************
*** 1901,1906 **** CatalogCacheCreateEntry(CatCache *cache, HeapTuple ntp, Datum *arguments,
--- 2003,2010 ----
      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);
  
***************
*** 1911,1917 **** CatalogCacheCreateEntry(CatCache *cache, HeapTuple ntp, Datum *arguments,
       * 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);
  
      return ct;
--- 2015,2022 ----
       * 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 &&
!         !CatCacheCleanupOldEntries(cache))
          RehashCatCache(cache);
  
      return ct;
*** a/src/include/utils/catcache.h
--- b/src/include/utils/catcache.h
***************
*** 119,124 **** typedef struct catctup
--- 119,126 ----
      bool        dead;            /* dead but not yet removed? */
      bool        negative;        /* negative cache entry? */
      HeapTupleData tuple;        /* tuple management header */
+     int            naccess;        /* # of access to this entry  */
+     TimestampTz    lastaccess;        /* approx. TS of the last access/modification */
  
      /*
       * The tuple may also be a member of at most one CatCList.  (If a single
***************
*** 189,194 **** typedef struct catcacheheader
--- 191,203 ----
  /* this extern duplicates utils/memutils.h... */
  extern PGDLLIMPORT MemoryContext CacheMemoryContext;
  
+ extern PGDLLIMPORT TimestampTz catcacheclock;
+ static inline void
+ SetCatCacheClock(TimestampTz ts)
+ {
+     catcacheclock = ts;
+ }
+ 
  extern void CreateCacheMemoryContext(void);
  
  extern CatCache *InitCatCache(int id, Oid reloid, Oid indexoid,