RE: Protect syscache from bloating with negative cache entries

>From: Ideriha, Takeshi [mailto:ideriha.takeshi@jp.fujitsu.com]
>I haven't looked into the code but I'm going to do it later.

Hi, I've taken a look at 0001 patch. Reviewing the rest of patch will be later.

        if (!IsParallelWorker())                                                                                  
+       {                                                                                                         
                stmtStartTimestamp = GetCurrentTimestamp();                                                       
+                                                                                                                 
+               /* Set this timestamp as aproximated current time */                                              
+               SetCatCacheClock(stmtStartTimestamp);                                                             
+       }                                                                                                         
        else    
  
Just confirmation.
At first I thought that when parallel worker is active catcacheclock is not updated.
But when parallel worker is active catcacheclock is updated by the parent and no problem is occurred.

+       int                     tupsize = 0;
                                                  
 

                                                  
 
        /* negative entries have no tuple associated */
                                                  
 
        if (ntp)
                                                  
 
        {
                                                  
 
                int                     i;
                                                  
 
+               int                     tupsize;

+               ct->size = tupsize;

@@ -1906,17 +2051,24 @@ CatalogCacheCreateEntry(CatCache *cache, HeapTuple ntp, Datum *arguments,
        ct->dead = false;
                                                  
 
        ct->negative = negative;
                                                  
 
        ct->hash_value = hashValue;
                                                  
 
+       ct->naccess = 0;
                                                  
 
+       ct->lastaccess = catcacheclock;
                                                  
 
+       ct->size = tupsize;

tupsize is declared twice inside and outiside of if scope but it doesn't seem you need to do so. 
And ct->size = tupsize is executed twice at if block and outside of if-else block.

+static inline TimestampTz
                                                  
 
+GetCatCacheClock(void)
                                                  
 

This function is not called by anyone in this version of patch. In previous version, this one is called by plancache.
Will further patch focus only on catcache? In this case this one can be removed. 

There are some typos.
+       int                     size;                   /* palloc'ed size off this tuple */ 
typo: off->of

+               /* Set this timestamp as aproximated current time */
typo: aproximated->approximated

+ * GUC variable to define the minimum size of hash to cosider entry eviction.
typo: cosider -> consider

+       /* initilize catcache reference clock if haven't done yet */ 
typo:initilize -> initialize

Regards,
Takeshi Ideriha