Re: [HACKERS] Protect syscache from bloating with negative cacheentries

At Wed, 21 Dec 2016 10:21:09 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in
<20161221.102109.51106943.horiguchi.kyotaro@lab.ntt.co.jp>
> At Tue, 20 Dec 2016 15:10:21 -0500, Tom Lane <tgl@sss.pgh.pa.us> wrote in <23492.1482264621@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
forother  reloids. However, such negative entries are created by rather  unusual usages. Accesing to undefined columns,
andaccessing  columns on which no statistics have created. The  whole-catcache scan occurs on ATTNAME, ATTNUM and
STATRELATTINHfor 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
thiscosts more time  than 1 and flushes involving entries that is not necessary to  be removed. If this feature is
valuablebut 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