Re: Protect syscache from bloating with negative cache entries

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