Protect syscache from bloating with negative cache entries

On Mon, Dec 19, 2016 at 6:15 AM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:

Hello, recently one of my customer stumbled over an immoderate
catcache bloat.

This isn't only an issue for negative catcache entries. A long time
ago, there was a limit on the size of the relcache, which was removed
because if you have a workload where the working set of relations is
just larger than the limit, performance is terrible. But the problem
now is that backend memory usage can grow without bound, and that's
also bad, especially on systems with hundreds of long-lived backends.
In connection-pooling environments, the problem is worse, because
every connection in the pool eventually caches references to
everything of interest to any client.

Your patches seem to me to have some merit, but I wonder if we should
also consider having a time-based threshold of some kind. If, say, a
backend hasn't accessed a catcache or relcache entry for many minutes,
it becomes eligible to be flushed. We could implement this by having
some process, like the background writer,
SendProcSignal(PROCSIG_HOUSEKEEPING) to every process in the system
every 10 minutes or so. When a process receives this signal, it sets
a flag that is checked before going idle. When it sees the flag set,
it makes a pass over every catcache and relcache entry. All the ones
that are unmarked get marked, and all of the ones that are marked get
removed. Access to an entry clears any mark. So anything that's not
touched for more than 10 minutes starts dropping out of backend
caches.

Anyway, that would be a much bigger change from what you are proposing
here, and what you are proposing here seems reasonable so I guess I
shouldn't distract from it. Your email just made me think of it,
because I agree that catcache/relcache bloat is a serious issue.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 20 December 2016 at 21:59, Robert Haas <robertmhaas@gmail.com> wrote:

We could implement this by having
some process, like the background writer,
SendProcSignal(PROCSIG_HOUSEKEEPING) to every process in the system
every 10 minutes or so.

... on a rolling basis.

Otherwise that'll be no fun at all, especially with some of those
lovely "we kept getting errors so we raised max_connections to 5000"
systems out there. But also on more sensibly configured ones that're
busy and want nice smooth performance without stalls.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Craig Ringer <craig@2ndquadrant.com> writes:

On 20 December 2016 at 21:59, Robert Haas <robertmhaas@gmail.com> wrote:

We could implement this by having
some process, like the background writer,
SendProcSignal(PROCSIG_HOUSEKEEPING) to every process in the system
every 10 minutes or so.

... on a rolling basis.

I don't understand why we'd make that a system-wide behavior at all,
rather than expecting each process to manage its own cache.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Tue, Dec 20, 2016 at 10:09 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Craig Ringer <craig@2ndquadrant.com> writes:

On 20 December 2016 at 21:59, Robert Haas <robertmhaas@gmail.com> wrote:

We could implement this by having
some process, like the background writer,
SendProcSignal(PROCSIG_HOUSEKEEPING) to every process in the system
every 10 minutes or so.

... on a rolling basis.

I don't understand why we'd make that a system-wide behavior at all,
rather than expecting each process to manage its own cache.

Individual backends don't have a really great way to do time-based
stuff, do they? I mean, yes, there is enable_timeout() and friends,
but I think that requires quite a bit of bookkeeping.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Dec 20, 2016 at 10:09 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I don't understand why we'd make that a system-wide behavior at all,
rather than expecting each process to manage its own cache.

Individual backends don't have a really great way to do time-based
stuff, do they? I mean, yes, there is enable_timeout() and friends,
but I think that requires quite a bit of bookkeeping.

If I thought that "every ten minutes" was an ideal way to manage this,
I might worry about that, but it doesn't really sound promising at all.
Every so many queries would likely work better, or better yet make it
self-adaptive depending on how much is in the local syscache.

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
/messages/by-id/5141.1150327541@sss.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.

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.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Tue, Dec 20, 2016 at 3:10 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Dec 20, 2016 at 10:09 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I don't understand why we'd make that a system-wide behavior at all,
rather than expecting each process to manage its own cache.

Individual backends don't have a really great way to do time-based
stuff, do they? I mean, yes, there is enable_timeout() and friends,
but I think that requires quite a bit of bookkeeping.

If I thought that "every ten minutes" was an ideal way to manage this,
I might worry about that, but it doesn't really sound promising at all.
Every so many queries would likely work better, or better yet make it
self-adaptive depending on how much is in the local syscache.

I don't think "every so many queries" is very promising at all.
First, it has the same problem as a fixed cap on the number of
entries: if you're doing a round-robin just slightly bigger than that
value, performance will be poor. Second, what's really important here
is to keep the percentage of wall-clock time spent populating the
system caches small. If a backend is doing 4000 queries/second and
each of those 4000 queries touches a different table, it really needs
a cache of at least 4000 entries or it will thrash and slow way down.
But if it's doing a query every 10 minutes and those queries
round-robin between 4000 different tables, it doesn't really need a
4000-entry cache. If those queries are long-running, the time to
repopulate the cache will only be a tiny fraction of runtime. If the
queries are short-running, then the effect is, percentage-wise, just
the same as for the high-volume system, but in practice it isn't
likely to be felt as much. I mean, if we keep a bunch of old cache
entries around on a mostly-idle backend, they are going to be pushed
out of CPU caches and maybe even paged out. One can't expect a
backend that is woken up after a long sleep to be quite as snappy as
one that's continuously active.

Which gets to my third point: anything that's based on number of
queries won't do anything to help the case where backends sometimes go
idle and sit there for long periods. Reducing resource utilization in
that case would be beneficial. Ideally I'd like to get rid of not
only the backend-local cache contents but the backend itself, but
that's a much harder project.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Thank you for the discussion.

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
/messages/by-id/5141.1150327541@sss.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.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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
/messages/by-id/5141.1150327541@sss.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

On Wed, Dec 21, 2016 at 5:10 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

If I thought that "every ten minutes" was an ideal way to manage this,
I might worry about that, but it doesn't really sound promising at all.
Every so many queries would likely work better, or better yet make it
self-adaptive depending on how much is in the local syscache.

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
/messages/by-id/5141.1150327541@sss.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.

Have there been ever discussions about having catcache entries in a
shared memory area? This does not sound much performance-wise, I am
just wondering about the concept and I cannot find references to such
discussions.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Michael Paquier <michael.paquier@gmail.com> writes:

Have there been ever discussions about having catcache entries in a
shared memory area? This does not sound much performance-wise, I am
just wondering about the concept and I cannot find references to such
discussions.

I'm sure it's been discussed. Offhand I remember the following issues:

* A shared cache would create locking and contention overhead.

* A shared cache would have a very hard size limit, at least if it's
in SysV-style shared memory (perhaps DSM would let us relax that).

* Transactions that are doing DDL have a requirement for the catcache
to reflect changes that they've made locally but not yet committed,
so said changes mustn't be visible globally.

You could possibly get around the third point with a local catcache that's
searched before the shared one, but tuning that to be performant sounds
like a mess. Also, I'm not sure how such a structure could cope with
uncommitted deletions: delete A -> remove A from local catcache, but not
the shared one -> search for A in local catcache -> not found -> search
for A in shared catcache -> found -> oops.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Jan 13, 2017 at 8:58 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Michael Paquier <michael.paquier@gmail.com> writes:

Have there been ever discussions about having catcache entries in a
shared memory area? This does not sound much performance-wise, I am
just wondering about the concept and I cannot find references to such
discussions.

I'm sure it's been discussed. Offhand I remember the following issues:

* A shared cache would create locking and contention overhead.

* A shared cache would have a very hard size limit, at least if it's
in SysV-style shared memory (perhaps DSM would let us relax that).

* Transactions that are doing DDL have a requirement for the catcache
to reflect changes that they've made locally but not yet committed,
so said changes mustn't be visible globally.

You could possibly get around the third point with a local catcache that's
searched before the shared one, but tuning that to be performant sounds
like a mess. Also, I'm not sure how such a structure could cope with
uncommitted deletions: delete A -> remove A from local catcache, but not
the shared one -> search for A in local catcache -> not found -> search
for A in shared catcache -> found -> oops.

I think the first of those concerns is the key one. If searching the
system catalogs costs $100 and searching the private catcache costs
$1, what's the cost of searching a hypothetical shared catcache? If
the answer is $80, it's not worth doing. If the answer is $5, it's
probably still not worth doing. If the answer is $1.25, then it's
probably worth investing some energy into trying to solve the other
problems you list. For some users, the memory cost of catcache and
syscache entries multiplied by N backends are a very serious problem,
so it would be nice to have some other options. But we do so many
syscache lookups that a shared cache won't be viable unless it's
almost as fast as a backend-private cache, or at least that's my
hunch.

I think it would be interested for somebody to build a prototype here
that ignores all the problems but the first and uses some
straightforward, relatively unoptimized locking strategy for the first
problem. Then benchmark it. If the results show that the idea has
legs, then we can try to figure out what a real implementation would
look like.

(One possible approach: use Thomas Munro's DHT stuff to build the shared cache.)

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Sat, Jan 14, 2017 at 12:32 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Fri, Jan 13, 2017 at 8:58 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Michael Paquier <michael.paquier@gmail.com> writes:

Have there been ever discussions about having catcache entries in a
shared memory area? This does not sound much performance-wise, I am
just wondering about the concept and I cannot find references to such
discussions.

I'm sure it's been discussed. Offhand I remember the following issues:

* A shared cache would create locking and contention overhead.

* A shared cache would have a very hard size limit, at least if it's
in SysV-style shared memory (perhaps DSM would let us relax that).

* Transactions that are doing DDL have a requirement for the catcache
to reflect changes that they've made locally but not yet committed,
so said changes mustn't be visible globally.

You could possibly get around the third point with a local catcache that's
searched before the shared one, but tuning that to be performant sounds
like a mess. Also, I'm not sure how such a structure could cope with
uncommitted deletions: delete A -> remove A from local catcache, but not
the shared one -> search for A in local catcache -> not found -> search
for A in shared catcache -> found -> oops.

I think the first of those concerns is the key one. If searching the
system catalogs costs $100 and searching the private catcache costs
$1, what's the cost of searching a hypothetical shared catcache? If
the answer is $80, it's not worth doing. If the answer is $5, it's
probably still not worth doing. If the answer is $1.25, then it's
probably worth investing some energy into trying to solve the other
problems you list. For some users, the memory cost of catcache and
syscache entries multiplied by N backends are a very serious problem,
so it would be nice to have some other options. But we do so many
syscache lookups that a shared cache won't be viable unless it's
almost as fast as a backend-private cache, or at least that's my
hunch.

Being able to switch from one mode to another would be interesting.
Applications using extensing DDLs that require to change the catcache
with an exclusive lock would clearly pay the lock contention cost, but
do you think that be really the case of a shared lock? A bunch of
applications that I work with deploy Postgres once, then don't change
the schema except when an upgrade happens. So that would be benefitial
for that. There are even some apps that do not use pgbouncer, but drop
sessions after a timeout of inactivity to avoid a memory bloat because
of the problem of this thread. That won't solve the problem of the
local catcache bloat, but some users using few DDLs may be fine to pay
some extra concurrency cost if the session handling gets easied.

I think it would be interested for somebody to build a prototype here
that ignores all the problems but the first and uses some
straightforward, relatively unoptimized locking strategy for the first
problem. Then benchmark it. If the results show that the idea has
legs, then we can try to figure out what a real implementation would
look like.
(One possible approach: use Thomas Munro's DHT stuff to build the shared cache.)

Yeah, I'd bet on a couple of days of focus to sort that out.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Michael Paquier <michael.paquier@gmail.com> writes:

... There are even some apps that do not use pgbouncer, but drop
sessions after a timeout of inactivity to avoid a memory bloat because
of the problem of this thread.

Yeah, a certain company I used to work for had to do that, though their
problem had more to do with bloat in plpgsql's compiled-functions cache
(and ensuing bloat in the plancache), I believe.

Still, I'm pretty suspicious of anything that will add overhead to
catcache lookups. If you think the performance of those is not absolutely
critical, turning off the caches via -DCLOBBER_CACHE_ALWAYS will soon
disabuse you of the error.

I'm inclined to think that a more profitable direction to look in is
finding a way to limit the cache size. I know we got rid of exactly that
years ago, but the problems with it were (a) the mechanism was itself
pretty expensive --- a global-to-all-caches LRU list IIRC, and (b) there
wasn't a way to tune the limit. Possibly somebody can think of some
cheaper, perhaps less precise way of aging out old entries. As for
(b), this is the sort of problem we made GUCs for.

But, again, the catcache isn't the only source of per-process bloat
and I'm not even sure it's the main one. A more holistic approach
might be called for.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi,

On 2017-01-13 17:58:41 -0500, Tom Lane wrote:

But, again, the catcache isn't the only source of per-process bloat
and I'm not even sure it's the main one. A more holistic approach
might be called for.

It'd be helpful if we'd find a way to make it easy to get statistics
about the size of various caches in production systems. Right now that's
kinda hard, resulting in us having to make a lot of guesses...

Andres

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 01/14/2017 12:06 AM, Andres Freund wrote:

Hi,

On 2017-01-13 17:58:41 -0500, Tom Lane wrote:

But, again, the catcache isn't the only source of per-process bloat
and I'm not even sure it's the main one. A more holistic approach
might be called for.

It'd be helpful if we'd find a way to make it easy to get statistics
about the size of various caches in production systems. Right now
that's kinda hard, resulting in us having to make a lot of
guesses...

What about a simple C extension, that could inspect those caches?
Assuming it could be loaded into a single backend, that should be
relatively acceptable way (compared to loading it to all backends using
shared_preload_libraries).

--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Sat, Jan 14, 2017 at 9:36 AM, Tomas Vondra
<tomas.vondra@2ndquadrant.com> wrote:

On 01/14/2017 12:06 AM, Andres Freund wrote:

On 2017-01-13 17:58:41 -0500, Tom Lane wrote:

But, again, the catcache isn't the only source of per-process bloat
and I'm not even sure it's the main one. A more holistic approach
might be called for.

It'd be helpful if we'd find a way to make it easy to get statistics
about the size of various caches in production systems. Right now
that's kinda hard, resulting in us having to make a lot of
guesses...

What about a simple C extension, that could inspect those caches? Assuming
it could be loaded into a single backend, that should be relatively
acceptable way (compared to loading it to all backends using
shared_preload_libraries).

This extension could do a small amount of work on a portion of the
syscache entries at each query loop, still I am wondering if that
would not be nicer to get that in-core and configurable, which is
basically the approach proposed by Horiguchi-san. At least it seems to
me that it has some merit, and if we could make that behavior
switchable, disabled by default, that would be a win for some class of
applications. What do others think?
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 12/26/16 2:31 AM, Kyotaro HORIGUCHI wrote:

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.

I took a look at this. It looks sane, though I've got a few minor
comment tweaks:

+ * Remove negative cache tuples maching a partial key.
s/maching/matching/

+/* searching with a paritial key needs scanning the whole cache */

s/needs/means/

+ * a negative cache entry cannot be referenced so we can remove

s/referenced/referenced,/

I was wondering if there's a way to test the performance impact of
deleting negative entries.

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.

I glanced at this and it looks sane. Didn't go any farther since this
one's pretty up in the air. ISTM it'd be better to do some kind of aging
instead of patch 2.

The other (possibly naive) question I have is how useful negative
entries really are? Will Postgres regularly incur negative lookups, or
will these only happen due to user activity? I can't think of a case
where an app would need to depend on fast negative lookup (in other
words, it should be considered a bug in the app). I can see where
getting rid of them completely might be problematic, but maybe we can
just keep a relatively small number of them around. I'm thinking a
simple LRU list of X number of negative entries; when that fills you
reuse the oldest one. You'd have to pay the LRU maintenance cost on
every negative hit, but if those shouldn't be that common it shouldn't
be bad.

That might well necessitate another GUC, but it seems a lot simpler than
most of the other ideas.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Jim Nasby <Jim.Nasby@bluetreble.com> writes:

The other (possibly naive) question I have is how useful negative
entries really are? Will Postgres regularly incur negative lookups, or
will these only happen due to user activity?

It varies depending on the particular syscache, but in at least some
of them, negative cache entries are critical for performance.
See for example RelnameGetRelid(), which basically does a RELNAMENSP
cache lookup for each schema down the search path until it finds a
match. For any user table name with the standard search_path, there's
a guaranteed failure in pg_catalog before you can hope to find a match.
If we don't have negative cache entries, then *every invocation of this
function has to go to disk* (or at least to shared buffers).

It's possible that we could revise all our lookup patterns to avoid this
sort of thing. But I don't have much faith in that always being possible,
and exactly none that we won't introduce new lookup patterns that need it
in future. I spent some time, for instance, wondering if RelnameGetRelid
could use a SearchSysCacheList lookup instead, doing the lookup on table
name only and then inspecting the whole list to see which entry is
frontmost according to the current search path. But that has performance
failure modes of its own, for example if you have identical table names in
a boatload of different schemas. We do it that way for some other cases
such as function lookups, but I think it's much less likely that people
have identical function names in N schemas than that they have identical
table names in N schemas.

If you want to poke into this for particular test scenarios, building with
CATCACHE_STATS defined will yield a bunch of numbers dumped to the
postmaster log at each backend exit.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 1/21/17 8:54 PM, Tom Lane wrote:

Jim Nasby <Jim.Nasby@bluetreble.com> writes:

The other (possibly naive) question I have is how useful negative
entries really are? Will Postgres regularly incur negative lookups, or
will these only happen due to user activity?

It varies depending on the particular syscache, but in at least some
of them, negative cache entries are critical for performance.
See for example RelnameGetRelid(), which basically does a RELNAMENSP
cache lookup for each schema down the search path until it finds a
match.

Ahh, I hadn't considered that. So one idea would be to only track
negative entries on caches where we know they're actually useful. That
might make the performance hit of some of the other ideas more
tolerable. Presumably you're much less likely to pollute the namespace
cache than some of the others.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 1/22/17 4:41 PM, Jim Nasby wrote:

On 1/21/17 8:54 PM, Tom Lane wrote:

Jim Nasby <Jim.Nasby@bluetreble.com> writes:

The other (possibly naive) question I have is how useful negative
entries really are? Will Postgres regularly incur negative lookups, or
will these only happen due to user activity?

It varies depending on the particular syscache, but in at least some
of them, negative cache entries are critical for performance.
See for example RelnameGetRelid(), which basically does a RELNAMENSP
cache lookup for each schema down the search path until it finds a
match.

Ahh, I hadn't considered that. So one idea would be to only track
negative entries on caches where we know they're actually useful. That
might make the performance hit of some of the other ideas more
tolerable. Presumably you're much less likely to pollute the namespace
cache than some of the others.

Ok, after reading the code I see I only partly understood what you were
saying. In any case, it might still be useful to do some testing with
CATCACHE_STATS defined to see if there's caches that don't accumulate a
lot of negative entries.

Attached is a patch that tries to document some of this.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)

Jim Nasby <Jim.Nasby@bluetreble.com> writes:

Ahh, I hadn't considered that. So one idea would be to only track
negative entries on caches where we know they're actually useful. That
might make the performance hit of some of the other ideas more
tolerable. Presumably you're much less likely to pollute the namespace
cache than some of the others.

Ok, after reading the code I see I only partly understood what you were
saying. In any case, it might still be useful to do some testing with
CATCACHE_STATS defined to see if there's caches that don't accumulate a
lot of negative entries.

There definitely are, according to my testing, but by the same token
it's not clear that a shutoff check would save anything.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 1/22/17 5:03 PM, Tom Lane wrote:

Ok, after reading the code I see I only partly understood what you were
saying. In any case, it might still be useful to do some testing with
CATCACHE_STATS defined to see if there's caches that don't accumulate a
lot of negative entries.

There definitely are, according to my testing, but by the same token
it's not clear that a shutoff check would save anything.

Currently they wouldn't, but there's concerns about the performance of
some of the other ideas in this thread. Getting rid of negative entries
that don't really help could reduce some of those concerns. Or perhaps
the original complaint about STATRELATTINH could be solved by just
disabling negative entries on that cache.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 1/21/17 6:42 PM, Jim Nasby wrote:

On 12/26/16 2:31 AM, Kyotaro HORIGUCHI wrote:

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.

I took a look at this. It looks sane, though I've got a few minor
comment tweaks:

+ * Remove negative cache tuples maching a partial key.
s/maching/matching/

+/* searching with a paritial key needs scanning the whole cache */

s/needs/means/

+ * a negative cache entry cannot be referenced so we can remove

s/referenced/referenced,/

I was wondering if there's a way to test the performance impact of
deleting negative entries.

I did a make installcheck run with CATCACHE_STATS to see how often we
get negative entries in the 3 caches affected by this patch. The caches
on pg_attribute get almost no negative entries. pg_statistic gets a good
amount of negative entries, presumably because we start off with no
entries in there. On a stable system that presumably won't be an issue,
but if temporary tables are in use and being analyzed I'd think there
could be a moderate amount of inval traffic on that cache. I'll leave it
to a committer to decide if they thing that's an issue, but you might
want to try and quantify how big a hit that is. I think it'd also be
useful to know how much bloat you were seeing in the field.

The patch is currently conflicting against master though, due to some
caches being added. Can you rebase? BTW, if you set a slightly larger
context size on the patch you might be able to avoid rebases; right now
the patch doesn't include enough context to uniquely identify the chunks
against cacheinfo[].
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hello, thank you for lookin this.

At Mon, 23 Jan 2017 16:54:36 -0600, Jim Nasby <Jim.Nasby@BlueTreble.com> wrote in <21803f50-a823-c444-ee2b-9a153114f454@BlueTreble.com>

On 1/21/17 6:42 PM, Jim Nasby wrote:

On 12/26/16 2:31 AM, Kyotaro HORIGUCHI wrote:

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.

I took a look at this. It looks sane, though I've got a few minor
comment tweaks:

+ * Remove negative cache tuples maching a partial key.
s/maching/matching/

+/* searching with a paritial key needs scanning the whole cache */

s/needs/means/

+ * a negative cache entry cannot be referenced so we can remove

s/referenced/referenced,/

I was wondering if there's a way to test the performance impact of
deleting negative entries.

Thanks for the pointing out. These are addressed.

I did a make installcheck run with CATCACHE_STATS to see how often we
get negative entries in the 3 caches affected by this patch. The
caches on pg_attribute get almost no negative entries. pg_statistic
gets a good amount of negative entries, presumably because we start
off with no entries in there. On a stable system that presumably won't
be an issue, but if temporary tables are in use and being analyzed I'd
think there could be a moderate amount of inval traffic on that
cache. I'll leave it to a committer to decide if they thing that's an
issue, but you might want to try and quantify how big a hit that is. I
think it'd also be useful to know how much bloat you were seeing in
the field.

The patch is currently conflicting against master though, due to some
caches being added. Can you rebase?

Six new syscaches in 665d1fa was conflicted and 3-way merge
worked correctly. The new syscaches don't seem to be targets of
this patch.

BTW, if you set a slightly larger
context size on the patch you might be able to avoid rebases; right
now the patch doesn't include enough context to uniquely identify the
chunks against cacheinfo[].

git format-patch -U5 fuses all hunks on cacheinfo[] together. I'm
not sure that such a hunk can avoid rebases. Is this what you
suggested? -U4 added an identifiable forward context line for
some elements so the attached patch is made with four context
lines.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

Hello,

I have tried to cap the number of negative entries for myself (by
removing negative entries in least recentrly created first order)
but the ceils cannot be reasonably determined both absolutely or
relatively to positive entries. Apparently it differs widely
among caches and applications.

At Mon, 23 Jan 2017 08:16:49 -0600, Jim Nasby <Jim.Nasby@BlueTreble.com> wrote in <6519b7ad-0aa6-c9f4-8869-20691107fb69@BlueTreble.com>

On 1/22/17 5:03 PM, Tom Lane wrote:

Ok, after reading the code I see I only partly understood what you
were
saying. In any case, it might still be useful to do some testing with
CATCACHE_STATS defined to see if there's caches that don't accumulate
a
lot of negative entries.

There definitely are, according to my testing, but by the same token
it's not clear that a shutoff check would save anything.

Currently they wouldn't, but there's concerns about the performance of
some of the other ideas in this thread. Getting rid of negative
entries that don't really help could reduce some of those concerns. Or
perhaps the original complaint about STATRELATTINH could be solved by
just disabling negative entries on that cache.

As for STATRELATTINH, planning involving small temporary tables
that frequently accessed willget benefit from negative entries,
but it might ignorably small. ATTNAME, ATTNUM and RENAMENSP also
might not get so much from negative entries. If these are true,
the whole stuff this patch adds can be replaced with just a
boolean in cachedesc that inhibits negatvie entries. Anyway this
patch don't save the case of the cache bloat relaed to function
reference. I'm not sure how that could be reproduced, though.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Tue, Jan 24, 2017 at 4:58 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:

Six new syscaches in 665d1fa was conflicted and 3-way merge
worked correctly. The new syscaches don't seem to be targets of
this patch.

To be honest, I am not completely sure what to think about this patch.
Moved to next CF as there is a new version, and no new reviews to make
the discussion perhaps move on.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hello, thank you for moving this to the next CF.

At Wed, 1 Feb 2017 13:09:51 +0900, Michael Paquier <michael.paquier@gmail.com> wrote in <CAB7nPqRFhUv+GX=eH1bo7xYHS79-gRj1ecu2QoQtHvX9RS=JYA@mail.gmail.com>

On Tue, Jan 24, 2017 at 4:58 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:

Six new syscaches in 665d1fa was conflicted and 3-way merge
worked correctly. The new syscaches don't seem to be targets of
this patch.

To be honest, I am not completely sure what to think about this patch.
Moved to next CF as there is a new version, and no new reviews to make
the discussion perhaps move on.

I'm thinking the following is the status of this topic.

- The patch stll is not getting conflicted.

- This is not a hollistic measure for memory leak but surely
saves some existing cases.

- Shared catcache is another discussion (and won't really
proposed in a short time due to the issue on locking.)

- As I mentioned, a patch that caps the number of negative
entries is avaiable (in first-created - first-delete manner)
but it is having a loose end of how to determine the
limitation.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2/1/17 1:25 AM, Kyotaro HORIGUCHI wrote:

Hello, thank you for moving this to the next CF.

At Wed, 1 Feb 2017 13:09:51 +0900, Michael Paquier <michael.paquier@gmail.com> wrote in <CAB7nPqRFhUv+GX=eH1bo7xYHS79-gRj1ecu2QoQtHvX9RS=JYA@mail.gmail.com>

On Tue, Jan 24, 2017 at 4:58 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:

Six new syscaches in 665d1fa was conflicted and 3-way merge
worked correctly. The new syscaches don't seem to be targets of
this patch.

To be honest, I am not completely sure what to think about this patch.
Moved to next CF as there is a new version, and no new reviews to make
the discussion perhaps move on.

I'm thinking the following is the status of this topic.

- The patch stll is not getting conflicted.

- This is not a hollistic measure for memory leak but surely
saves some existing cases.

- Shared catcache is another discussion (and won't really
proposed in a short time due to the issue on locking.)

- As I mentioned, a patch that caps the number of negative
entries is avaiable (in first-created - first-delete manner)
but it is having a loose end of how to determine the
limitation.

While preventing bloat in the syscache is a worthwhile goal, it appears
there are a number of loose ends here and a new patch has not been provided.

It's a pretty major change so I recommend moving this patch to the
2017-07 CF.

--
-David
david@pgmasters.net

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 3/3/17 4:54 PM, David Steele wrote:

On 2/1/17 1:25 AM, Kyotaro HORIGUCHI wrote:

Hello, thank you for moving this to the next CF.

At Wed, 1 Feb 2017 13:09:51 +0900, Michael Paquier <michael.paquier@gmail.com> wrote in <CAB7nPqRFhUv+GX=eH1bo7xYHS79-gRj1ecu2QoQtHvX9RS=JYA@mail.gmail.com>

On Tue, Jan 24, 2017 at 4:58 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:

Six new syscaches in 665d1fa was conflicted and 3-way merge
worked correctly. The new syscaches don't seem to be targets of
this patch.

To be honest, I am not completely sure what to think about this patch.
Moved to next CF as there is a new version, and no new reviews to make
the discussion perhaps move on.

I'm thinking the following is the status of this topic.

- The patch stll is not getting conflicted.

- This is not a hollistic measure for memory leak but surely
saves some existing cases.

- Shared catcache is another discussion (and won't really
proposed in a short time due to the issue on locking.)

- As I mentioned, a patch that caps the number of negative
entries is avaiable (in first-created - first-delete manner)
but it is having a loose end of how to determine the
limitation.

While preventing bloat in the syscache is a worthwhile goal, it appears
there are a number of loose ends here and a new patch has not been provided.

It's a pretty major change so I recommend moving this patch to the
2017-07 CF.

Not hearing any opinions pro or con, I'm moving this patch to the
2017-07 CF.

--
-David
david@pgmasters.net

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

At Tue, 7 Mar 2017 19:23:14 -0800, David Steele <david@pgmasters.net> wrote in <3b7b7f90-db46-8c37-c4f7-443330c3ae33@pgmasters.net>

On 3/3/17 4:54 PM, David Steele wrote:

On 2/1/17 1:25 AM, Kyotaro HORIGUCHI wrote:

Hello, thank you for moving this to the next CF.

At Wed, 1 Feb 2017 13:09:51 +0900, Michael Paquier
<michael.paquier@gmail.com> wrote in
<CAB7nPqRFhUv+GX=eH1bo7xYHS79-gRj1ecu2QoQtHvX9RS=JYA@mail.gmail.com>

On Tue, Jan 24, 2017 at 4:58 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:

Six new syscaches in 665d1fa was conflicted and 3-way merge
worked correctly. The new syscaches don't seem to be targets of
this patch.

To be honest, I am not completely sure what to think about this patch.
Moved to next CF as there is a new version, and no new reviews to make
the discussion perhaps move on.

I'm thinking the following is the status of this topic.

- The patch stll is not getting conflicted.

- This is not a hollistic measure for memory leak but surely
saves some existing cases.

- Shared catcache is another discussion (and won't really
proposed in a short time due to the issue on locking.)

- As I mentioned, a patch that caps the number of negative
entries is avaiable (in first-created - first-delete manner)
but it is having a loose end of how to determine the
limitation.

While preventing bloat in the syscache is a worthwhile goal, it
appears
there are a number of loose ends here and a new patch has not been
provided.

It's a pretty major change so I recommend moving this patch to the
2017-07 CF.

Not hearing any opinions pro or con, I'm moving this patch to the
2017-07 CF.

Ah. Yes, I agree on this. Thanks.

--
Kyotaro Horiguchi
NTT Open Source Software Center

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 1/24/17 02:58, Kyotaro HORIGUCHI wrote:

BTW, if you set a slightly larger
context size on the patch you might be able to avoid rebases; right
now the patch doesn't include enough context to uniquely identify the
chunks against cacheinfo[].

git format-patch -U5 fuses all hunks on cacheinfo[] together. I'm
not sure that such a hunk can avoid rebases. Is this what you
suggested? -U4 added an identifiable forward context line for
some elements so the attached patch is made with four context
lines.

This patch needs another rebase for the upcoming commit fest.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Thank you for your attention.

At Mon, 14 Aug 2017 17:33:48 -0400, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote in <09fa011f-4536-b05d-0625-11f3625d8332@2ndquadrant.com>

On 1/24/17 02:58, Kyotaro HORIGUCHI wrote:

BTW, if you set a slightly larger
context size on the patch you might be able to avoid rebases; right
now the patch doesn't include enough context to uniquely identify the
chunks against cacheinfo[].

git format-patch -U5 fuses all hunks on cacheinfo[] together. I'm
not sure that such a hunk can avoid rebases. Is this what you
suggested? -U4 added an identifiable forward context line for
some elements so the attached patch is made with four context
lines.

This patch needs another rebase for the upcoming commit fest.

This patch have had interferences from several commits after the
last submission. I amended this patch to follow them (up to
f97c55c), removed an unnecessary branch and edited some comments.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

On Mon, Aug 28, 2017 at 5:24 AM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:

This patch have had interferences from several commits after the
last submission. I amended this patch to follow them (up to
f97c55c), removed an unnecessary branch and edited some comments.

I think the core problem for this patch is that there's no consensus
on what approach to take. Until that somehow gets sorted out, I think
this isn't going to make any progress. Unfortunately, I don't have a
clear idea what sort of solution everybody could tolerate.

I still think that some kind of slow-expire behavior -- like a clock
hand that hits each backend every 10 minutes and expires entries not
used since the last hit -- is actually pretty sensible. It ensures
that idle or long-running backends don't accumulate infinite bloat
while still allowing the cache to grow large enough for good
performance when all entries are being regularly used. But Tom
doesn't like it. Other approaches were also discussed; none of them
seem like an obvious slam-dunk.

Turning to the patch itself, I don't know how we decide whether the
patch is worth it. Scanning the whole (potentially large) cache to
remove negative entries has a cost, mostly in CPU cycles; keeping
those negative entries around for a long time also has a cost, mostly
in memory. I don't know how to decide whether these patches will help
more people than it hurts, or the other way around -- and it's not
clear that anyone else has a good idea about that either.

Typos: funciton, paritial.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, Aug 28, 2017 at 9:24 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:

This patch have had interferences from several commits after the
last submission. I amended this patch to follow them (up to
f97c55c), removed an unnecessary branch and edited some comments.

Hi Kyotaro-san,

This applies but several regression tests fail for me. Here is a
sample backtrace:

frame #3: 0x000000010f0614c0
postgres`ExceptionalCondition(conditionName="!(attnum < 0 ? attnum ==
(-2) : cache->cc_tupdesc->attrs[attnum].atttypid == 26)",
errorType="FailedAssertion", fileName="catcache.c", lineNumber=1384) +
128 at assert.c:54
frame #4: 0x000000010f03b5fd
postgres`CollectOIDsForHashValue(cache=0x00007fe273821268,
hashValue=994410284, attnum=0) + 253 at catcache.c:1383
frame #5: 0x000000010f055e8e
postgres`SysCacheSysCacheInvalCallback(arg=140610577303984, cacheid=0,
hashValue=994410284) + 94 at syscache.c:1692
frame #6: 0x000000010f03fbbb
postgres`CallSyscacheCallbacks(cacheid=0, hashvalue=994410284) + 219
at inval.c:1468
frame #7: 0x000000010f03f878
postgres`LocalExecuteInvalidationMessage(msg=0x00007fff51213ff8) + 88
at inval.c:566
frame #8: 0x000000010ee7a3f2
postgres`ReceiveSharedInvalidMessages(invalFunction=(postgres`LocalExecuteInvalidationMessage
at inval.c:555), resetFunction=(postgres`InvalidateSystemCaches at
inval.c:647)) + 354 at sinval.c:121
frame #9: 0x000000010f03fcb7 postgres`AcceptInvalidationMessages +
23 at inval.c:686
frame #10: 0x000000010eade609 postgres`AtStart_Cache + 9 at xact.c:987
frame #11: 0x000000010ead8c2f postgres`StartTransaction + 655 at xact.c:1921
frame #12: 0x000000010ead8896 postgres`StartTransactionCommand +
70 at xact.c:2691
frame #13: 0x000000010eea9746 postgres`start_xact_command + 22 at
postgres.c:2438
frame #14: 0x000000010eea722e
postgres`exec_simple_query(query_string="RESET SESSION
AUTHORIZATION;") + 126 at postgres.c:913
frame #15: 0x000000010eea68d7 postgres`PostgresMain(argc=1,
argv=0x00007fe2738036a8, dbname="regression", username="munro") + 2375
at postgres.c:4090
frame #16: 0x000000010eded40e
postgres`BackendRun(port=0x00007fe2716001a0) + 654 at
postmaster.c:4357
frame #17: 0x000000010edec793
postgres`BackendStartup(port=0x00007fe2716001a0) + 483 at
postmaster.c:4029
frame #18: 0x000000010edeb785 postgres`ServerLoop + 597 at postmaster.c:1753
frame #19: 0x000000010ede8f71 postgres`PostmasterMain(argc=8,
argv=0x00007fe271403860) + 5553 at postmaster.c:1361
frame #20: 0x000000010ed0ccd9 postgres`main(argc=8,
argv=0x00007fe271403860) + 761 at main.c:228
frame #21: 0x00007fff8333a5ad libdyld.dylib`start + 1

--
Thomas Munro
http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Thank you for reviewing this.

At Sat, 2 Sep 2017 12:12:47 +1200, Thomas Munro <thomas.munro@enterprisedb.com> wrote in <CAEepm=3wqPFFSKP_yhkuHLZtOOwZskGuHJdSctVnbHQ4DFEH+Q@mail.gmail.com>

On Mon, Aug 28, 2017 at 9:24 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:

This patch have had interferences from several commits after the
last submission. I amended this patch to follow them (up to
f97c55c), removed an unnecessary branch and edited some comments.

Hi Kyotaro-san,

This applies but several regression tests fail for me. Here is a
sample backtrace:

Sorry for the silly mistake. STAEXTNAMENSP and STATRELATTINH was
missing additional elements in their definitions. Somehow I've
removed them.

The attached patch doesn't crash by regression test. And fixed
some typos pointed by Robert and found by myself.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

Thank you for the comment.

At Mon, 28 Aug 2017 21:31:58 -0400, Robert Haas <robertmhaas@gmail.com> wrote in <CA+TgmoZjn28uYJRQ2K+5idhYxWBDER68sctoc2p_nW7h7JbhYw@mail.gmail.com>

On Mon, Aug 28, 2017 at 5:24 AM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:

This patch have had interferences from several commits after the
last submission. I amended this patch to follow them (up to
f97c55c), removed an unnecessary branch and edited some comments.

I think the core problem for this patch is that there's no consensus
on what approach to take. Until that somehow gets sorted out, I think
this isn't going to make any progress. Unfortunately, I don't have a
clear idea what sort of solution everybody could tolerate.

I still think that some kind of slow-expire behavior -- like a clock
hand that hits each backend every 10 minutes and expires entries not
used since the last hit -- is actually pretty sensible. It ensures
that idle or long-running backends don't accumulate infinite bloat
while still allowing the cache to grow large enough for good
performance when all entries are being regularly used. But Tom
doesn't like it. Other approaches were also discussed; none of them
seem like an obvious slam-dunk.

I suppose that it slows intermittent lookup of non-existent
objects. I have tried a slight different thing. Removing entries
by 'age', preserving specified number (or ratio to live entries)
of younger negative entries. The problem of that approach was I
didn't find how to determine the number of entries to preserve,
or I didn't want to offer additional knobs for them. Finally I
proposed the patch upthread since it doesn't need any assumption
on usage.

Though I can make another patch that does the same thing based on
LRU, the same how-many-to-preserve problem ought to be resolved
in order to avoid slowing the inermittent lookup.

Turning to the patch itself, I don't know how we decide whether the
patch is worth it. Scanning the whole (potentially large) cache to
remove negative entries has a cost, mostly in CPU cycles; keeping
those negative entries around for a long time also has a cost, mostly
in memory. I don't know how to decide whether these patches will help
more people than it hurts, or the other way around -- and it's not
clear that anyone else has a good idea about that either.

Scanning a hash on invalidation of several catalogs (hopefully
slightly) slows certain percentage of inavlidations on maybe most
of workloads. Holding no-longer-lookedup entries surely kills a
backend under certain workloads sooner or later. This doesn't
save the pg_proc cases, but saves pg_statistic and pg_class
cases. I'm not sure what other catalogs can bloat.

I could reduce the complexity of this. Inval mechanism conveys
only a hash value so this scans the whole of a cache for the
target OIDs (with possible spurious targets). This will be
resolved by letting inval mechanism convey an OID. (but this may
need additional members in an inval entry.)

Still, the full scan perfomed in CleanupCatCacheNegEntries
doesn't seem easily avoidable. Separating the hash by OID of key
or provide special dlist that points tuples in buckets will
introduce another complexity.

Typos: funciton, paritial.

Thanks. ispell told me of additional typos corresnpond, belive
and undistinguisable.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

This is a rebased version of the patch.

At Fri, 17 Mar 2017 14:23:13 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in <20170317.142313.232290068.horiguchi.kyotaro@lab.ntt.co.jp>

At Tue, 7 Mar 2017 19:23:14 -0800, David Steele <david@pgmasters.net> wrote in <3b7b7f90-db46-8c37-c4f7-443330c3ae33@pgmasters.net>

On 3/3/17 4:54 PM, David Steele wrote:

On 2/1/17 1:25 AM, Kyotaro HORIGUCHI wrote:

Hello, thank you for moving this to the next CF.

At Wed, 1 Feb 2017 13:09:51 +0900, Michael Paquier
<michael.paquier@gmail.com> wrote in
<CAB7nPqRFhUv+GX=eH1bo7xYHS79-gRj1ecu2QoQtHvX9RS=JYA@mail.gmail.com>

On Tue, Jan 24, 2017 at 4:58 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:

Six new syscaches in 665d1fa was conflicted and 3-way merge
worked correctly. The new syscaches don't seem to be targets of
this patch.

To be honest, I am not completely sure what to think about this patch.
Moved to next CF as there is a new version, and no new reviews to make
the discussion perhaps move on.

I'm thinking the following is the status of this topic.

- The patch stll is not getting conflicted.

- This is not a hollistic measure for memory leak but surely
saves some existing cases.

- Shared catcache is another discussion (and won't really
proposed in a short time due to the issue on locking.)

- As I mentioned, a patch that caps the number of negative
entries is avaiable (in first-created - first-delete manner)
but it is having a loose end of how to determine the
limitation.

While preventing bloat in the syscache is a worthwhile goal, it
appears
there are a number of loose ends here and a new patch has not been
provided.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

On Tue, Oct 31, 2017 at 6:46 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:

This is a rebased version of the patch.

As far as I can see, the patch still applies, compiles, and got no
reviews. So moved to next CF.
--
Michael

On Wed, Nov 29, 2017 at 8:25 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Tue, Oct 31, 2017 at 6:46 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:

This is a rebased version of the patch.

As far as I can see, the patch still applies, compiles, and got no
reviews. So moved to next CF.

I think we have to mark this as returned with feedback or rejected for
the reasons mentioned here:

/messages/by-id/CA+TgmoZjn28uYJRQ2K+5idhYxWBDER68sctoc2p_nW7h7JbhYw@mail.gmail.com

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Thu, Nov 30, 2017 at 12:32 PM, Robert Haas <robertmhaas@gmail.com> wrote:

On Wed, Nov 29, 2017 at 8:25 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Tue, Oct 31, 2017 at 6:46 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:

This is a rebased version of the patch.

As far as I can see, the patch still applies, compiles, and got no
reviews. So moved to next CF.

I think we have to mark this as returned with feedback or rejected for
the reasons mentioned here:

/messages/by-id/CA+TgmoZjn28uYJRQ2K+5idhYxWBDER68sctoc2p_nW7h7JbhYw@mail.gmail.com

Good point. I forgot this bit. Thanks for mentioning it I am switching
the patch as returned with feedback.
--
Michael

Michael Paquier <michael.paquier@gmail.com> writes:

On Thu, Nov 30, 2017 at 12:32 PM, Robert Haas <robertmhaas@gmail.com> wrote:

I think we have to mark this as returned with feedback or rejected for
the reasons mentioned here:
/messages/by-id/CA+TgmoZjn28uYJRQ2K+5idhYxWBDER68sctoc2p_nW7h7JbhYw@mail.gmail.com

Good point. I forgot this bit. Thanks for mentioning it I am switching
the patch as returned with feedback.

We had a bug report just today that seemed to me to trace to relcache
bloat:
/messages/by-id/20171129100649.1473.73990@wrigleys.postgresql.org

ISTM that there's definitely work to be done here, but as I said upthread,
I think we need a more holistic approach than just focusing on negative
catcache entries, or even just catcache entries.

The thing that makes me uncomfortable about this is that we used to have a
catcache size limitation mechanism, and ripped it out because it had too
much overhead (see commit 8b9bc234a). I'm not sure how we can avoid that
problem within a fresh implementation.

regards, tom lane

On Wed, Nov 29, 2017 at 11:17 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

The thing that makes me uncomfortable about this is that we used to have a
catcache size limitation mechanism, and ripped it out because it had too
much overhead (see commit 8b9bc234a). I'm not sure how we can avoid that
problem within a fresh implementation.

At the risk of beating a dead horse, I still think that the amount of
wall clock time that has elapsed since an entry was last accessed is
very relevant. The problem with a fixed maximum size is that you can
hit it arbitrarily frequently; time-based expiration solves that
problem. It allows backends that are actively using a lot of stuff to
hold on to as many cache entries as they need, while forcing backends
that have moved on to a different set of tables -- or that are
completely idle -- to let go of cache entries that are no longer being
actively used. I think that's what we want. Nobody wants to keep the
cache size small when a big cache is necessary for good performance,
but what people do want to avoid is having long-running backends
eventually accumulate huge numbers of cache entries most of which
haven't been touched in hours or, maybe, weeks.

To put that another way, we should only hang on to a cache entry for
so long as the bytes of memory that it consumes are more valuable than
some other possible use of those bytes of memory. That is very likely
to be true when we've accessed those bytes recently, but progressively
less likely to be true the more time has passed.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:

On Wed, Nov 29, 2017 at 11:17 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

The thing that makes me uncomfortable about this is that we used to have a
catcache size limitation mechanism, and ripped it out because it had too
much overhead (see commit 8b9bc234a). I'm not sure how we can avoid that
problem within a fresh implementation.

At the risk of beating a dead horse, I still think that the amount of
wall clock time that has elapsed since an entry was last accessed is
very relevant.

While I don't object to that statement, I'm not sure how it helps us
here. If we couldn't afford DLMoveToFront(), doing a gettimeofday()
during each syscache access is surely right out.

regards, tom lane

On Thu, Nov 30, 2017 at 11:17 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Wed, Nov 29, 2017 at 11:17 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

The thing that makes me uncomfortable about this is that we used to have a
catcache size limitation mechanism, and ripped it out because it had too
much overhead (see commit 8b9bc234a). I'm not sure how we can avoid that
problem within a fresh implementation.

At the risk of beating a dead horse, I still think that the amount of
wall clock time that has elapsed since an entry was last accessed is
very relevant.

While I don't object to that statement, I'm not sure how it helps us
here. If we couldn't afford DLMoveToFront(), doing a gettimeofday()
during each syscache access is surely right out.

Well, yeah, that would be insane. But I think even something very
rough could work well enough. I think our goal should be to eliminate
cache entries that are have gone unused for many *minutes*, and
there's no urgency about getting it to any sort of exact value. For
non-idle backends, using the most recent statement start time as a
proxy would probably be plenty good enough. Idle backends might need
a bit more thought.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 2017-12-01 16:20:44 -0500, Robert Haas wrote:

Well, yeah, that would be insane. But I think even something very
rough could work well enough. I think our goal should be to eliminate
cache entries that are have gone unused for many *minutes*, and
there's no urgency about getting it to any sort of exact value. For
non-idle backends, using the most recent statement start time as a
proxy would probably be plenty good enough. Idle backends might need
a bit more thought.

Our timer framework is flexible enough that we can install a
once-a-minute timer without much overhead. That timer could increment a
'cache generation' integer. Upon cache access we write the current
generation into relcache / syscache (and potentially also plancache?)
entries. Not entirely free, but cheap enough. In those once-a-minute
passes entries that haven't been touched in X cycles get pruned.

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:

On 2017-12-01 16:20:44 -0500, Robert Haas wrote:

Well, yeah, that would be insane. But I think even something very
rough could work well enough. I think our goal should be to eliminate
cache entries that are have gone unused for many *minutes*, and
there's no urgency about getting it to any sort of exact value. For
non-idle backends, using the most recent statement start time as a
proxy would probably be plenty good enough. Idle backends might need
a bit more thought.

Our timer framework is flexible enough that we can install a
once-a-minute timer without much overhead. That timer could increment a
'cache generation' integer. Upon cache access we write the current
generation into relcache / syscache (and potentially also plancache?)
entries. Not entirely free, but cheap enough. In those once-a-minute
passes entries that haven't been touched in X cycles get pruned.

I have no faith in either of these proposals, because they both assume
that the problem only arises over the course of many minutes. In the
recent complaint about pg_dump causing relcache bloat, it probably does
not take nearly that long for the bloat to occur.

Maybe you could make it work on the basis of number of cache accesses,
or some other normalized-to-workload-not-wall-clock time reference.

regards, tom lane

On 2017-12-01 16:40:23 -0500, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

On 2017-12-01 16:20:44 -0500, Robert Haas wrote:

Well, yeah, that would be insane. But I think even something very
rough could work well enough. I think our goal should be to eliminate
cache entries that are have gone unused for many *minutes*, and
there's no urgency about getting it to any sort of exact value. For
non-idle backends, using the most recent statement start time as a
proxy would probably be plenty good enough. Idle backends might need
a bit more thought.

Our timer framework is flexible enough that we can install a
once-a-minute timer without much overhead. That timer could increment a
'cache generation' integer. Upon cache access we write the current
generation into relcache / syscache (and potentially also plancache?)
entries. Not entirely free, but cheap enough. In those once-a-minute
passes entries that haven't been touched in X cycles get pruned.

I have no faith in either of these proposals, because they both assume
that the problem only arises over the course of many minutes. In the
recent complaint about pg_dump causing relcache bloat, it probably does
not take nearly that long for the bloat to occur.

To me that's a bit of a different problem than what I was discussing
here. It also actually doesn't seem that hard - if your caches are
growing fast, you'll continually get hash-resizing of the
various. Adding cache-pruning to the resizing code doesn't seem hard,
and wouldn't add meaningful overhead.

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:

On 2017-12-01 16:40:23 -0500, Tom Lane wrote:

I have no faith in either of these proposals, because they both assume
that the problem only arises over the course of many minutes. In the
recent complaint about pg_dump causing relcache bloat, it probably does
not take nearly that long for the bloat to occur.

To me that's a bit of a different problem than what I was discussing
here. It also actually doesn't seem that hard - if your caches are
growing fast, you'll continually get hash-resizing of the
various. Adding cache-pruning to the resizing code doesn't seem hard,
and wouldn't add meaningful overhead.

That's an interesting way to think about it, as well, though I'm not
sure it's quite that simple. If you tie this to cache resizing then
the cache will have to grow up to the newly increased size before
you'll prune it again. That doesn't sound like it will lead to nice
steady-state behavior.

regards, tom lane

On 2017-12-01 17:03:28 -0500, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

On 2017-12-01 16:40:23 -0500, Tom Lane wrote:

I have no faith in either of these proposals, because they both assume
that the problem only arises over the course of many minutes. In the
recent complaint about pg_dump causing relcache bloat, it probably does
not take nearly that long for the bloat to occur.

To me that's a bit of a different problem than what I was discussing
here. It also actually doesn't seem that hard - if your caches are
growing fast, you'll continually get hash-resizing of the
various. Adding cache-pruning to the resizing code doesn't seem hard,
and wouldn't add meaningful overhead.

That's an interesting way to think about it, as well, though I'm not
sure it's quite that simple. If you tie this to cache resizing then
the cache will have to grow up to the newly increased size before
you'll prune it again. That doesn't sound like it will lead to nice
steady-state behavior.

Yea, it's not perfect - but if we do pruning both at resize *and* on
regular intervals, like once-a-minute as I was suggesting, I don't think
it's that bad. The steady state won't be reached within seconds, true,
but the negative consequences of only attempting to shrink the cache
upon resizing when the cache size is growing fast anyway doesn't seem
that large.

I don't think we need to be super accurate here, there just needs to be
*some* backpressure.

I've had cases in the past where just occasionally blasting the cache
away would've been good enough.

Greetings,

Andres Freund

At Fri, 1 Dec 2017 14:12:20 -0800, Andres Freund <andres@anarazel.de> wrote in <20171201221220.z5e6wtlpl264wzik@alap3.anarazel.de>

On 2017-12-01 17:03:28 -0500, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

On 2017-12-01 16:40:23 -0500, Tom Lane wrote:

I have no faith in either of these proposals, because they both assume
that the problem only arises over the course of many minutes. In the
recent complaint about pg_dump causing relcache bloat, it probably does
not take nearly that long for the bloat to occur.

To me that's a bit of a different problem than what I was discussing
here. It also actually doesn't seem that hard - if your caches are
growing fast, you'll continually get hash-resizing of the
various. Adding cache-pruning to the resizing code doesn't seem hard,
and wouldn't add meaningful overhead.

That's an interesting way to think about it, as well, though I'm not
sure it's quite that simple. If you tie this to cache resizing then
the cache will have to grow up to the newly increased size before
you'll prune it again. That doesn't sound like it will lead to nice
steady-state behavior.

Yea, it's not perfect - but if we do pruning both at resize *and* on
regular intervals, like once-a-minute as I was suggesting, I don't think
it's that bad. The steady state won't be reached within seconds, true,
but the negative consequences of only attempting to shrink the cache
upon resizing when the cache size is growing fast anyway doesn't seem
that large.

I don't think we need to be super accurate here, there just needs to be
*some* backpressure.

I've had cases in the past where just occasionally blasting the cache
away would've been good enough.

Thank you very much for the valuable suggestions. I still would
like to solve this problem and the
a-counter-freely-running-in-minute(or several seconds)-resolution
and pruning-too-long-unaccessed-entries-on-resizing seems to me
to work enough for at least several known bloat cases. This still
has a defect that this is not workable for a very quick
bloating. I'll try thinking about the remaining issue.

If no one has immediate objection to the direction, I'll come up
with an implementation.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

On Wed, Dec 13, 2017 at 11:20 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:

Thank you very much for the valuable suggestions. I still would
like to solve this problem and the
a-counter-freely-running-in-minute(or several seconds)-resolution
and pruning-too-long-unaccessed-entries-on-resizing seems to me
to work enough for at least several known bloat cases. This still
has a defect that this is not workable for a very quick
bloating. I'll try thinking about the remaining issue.

I'm not sure we should regard very quick bloating as a problem in need
of solving. Doesn't that just mean we need the cache to be bigger, at
least temporarily?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 2017-12-16 22:25:48 -0500, Robert Haas wrote:

On Wed, Dec 13, 2017 at 11:20 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:

Thank you very much for the valuable suggestions. I still would
like to solve this problem and the
a-counter-freely-running-in-minute(or several seconds)-resolution
and pruning-too-long-unaccessed-entries-on-resizing seems to me
to work enough for at least several known bloat cases. This still
has a defect that this is not workable for a very quick
bloating. I'll try thinking about the remaining issue.

I'm not sure we should regard very quick bloating as a problem in need
of solving. Doesn't that just mean we need the cache to be bigger, at
least temporarily?

Leaving that aside, is that actually not at least to a good degree,
solved by that problem? By bumping the generation on hash resize, we
have recency information we can take into account.

Greetings,

Andres Freund

On Sat, Dec 16, 2017 at 11:42 PM, Andres Freund <andres@anarazel.de> wrote:

I'm not sure we should regard very quick bloating as a problem in need
of solving. Doesn't that just mean we need the cache to be bigger, at
least temporarily?

Leaving that aside, is that actually not at least to a good degree,
solved by that problem? By bumping the generation on hash resize, we
have recency information we can take into account.

I agree that we can do it. I'm just not totally sure it's a good
idea. I'm also not totally sure it's a bad idea, either. That's why
I asked the question.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 2017-12-17 19:23:45 -0500, Robert Haas wrote:

On Sat, Dec 16, 2017 at 11:42 PM, Andres Freund <andres@anarazel.de> wrote:

I'm not sure we should regard very quick bloating as a problem in need
of solving. Doesn't that just mean we need the cache to be bigger, at
least temporarily?

Leaving that aside, is that actually not at least to a good degree,
solved by that problem? By bumping the generation on hash resize, we
have recency information we can take into account.

I agree that we can do it. I'm just not totally sure it's a good
idea. I'm also not totally sure it's a bad idea, either. That's why
I asked the question.

I'm not 100% convinced either - but I also don't think it matters all
that terribly much. As long as the overall hash hit rate is decent,
minor increases in the absolute number of misses don't really matter
that much for syscache imo. I'd personally go for something like:

1) When about to resize, check if there's entries of a generation -2
around.

Don't resize if more than 15% of entries could be freed. Also, stop
reclaiming at that threshold, to avoid unnecessary purging cache
entries.

Using two generations allows a bit more time for cache entries to
marked as fresh before resizing next.

2) While resizing increment generation count by one.

3) Once a minute, increment generation count by one.

The one thing I'm not quite have a good handle upon is how much, and if
any, cache reclamation to do at 3). We don't really want to throw away
all the caches just because a connection has been idle for a few
minutes, in a connection pool that can happen occasionally. I think I'd
for now *not* do any reclamation except at resize boundaries.

Greetings,

Andres Freund

On Mon, Dec 18, 2017 at 11:46 AM, Andres Freund <andres@anarazel.de> wrote:

I'm not 100% convinced either - but I also don't think it matters all
that terribly much. As long as the overall hash hit rate is decent,
minor increases in the absolute number of misses don't really matter
that much for syscache imo. I'd personally go for something like:

1) When about to resize, check if there's entries of a generation -2
around.

Don't resize if more than 15% of entries could be freed. Also, stop
reclaiming at that threshold, to avoid unnecessary purging cache
entries.

Using two generations allows a bit more time for cache entries to
marked as fresh before resizing next.

2) While resizing increment generation count by one.

3) Once a minute, increment generation count by one.

The one thing I'm not quite have a good handle upon is how much, and if
any, cache reclamation to do at 3). We don't really want to throw away
all the caches just because a connection has been idle for a few
minutes, in a connection pool that can happen occasionally. I think I'd
for now *not* do any reclamation except at resize boundaries.

My starting inclination was almost the opposite. I think that you
might be right that a minute or two of idle time isn't sufficient
reason to flush our local cache, but I'd be inclined to fix that by
incrementing the generation count every 10 minutes or so rather than
every minute, and still flush things more then 1 generation old. The
reason for that is that I think we should ensure that the system
doesn't sit there idle forever with a giant cache. If it's not using
those cache entries, I'd rather have it discard them and rebuild the
cache when it becomes active again.

Now, I also see that your point about trying to clean up before
resizing. That does seem like a good idea, although we have to be
careful not to be too eager to clean up there, or we'll just result in
artificially limiting the cache size when it's unwise to do so. But I
guess that's what you meant by "Also, stop reclaiming at that
threshold, to avoid unnecessary purging cache entries." I think the
idea you are proposing is that:

1. The first time we are due to expand the hash table, we check
whether we can forestall that expansion by doing a cleanup; if so, we
do that instead.

2. After that, we just expand.

That seems like a fairly good idea, although it might be a better idea
to allow cleanup if enough time has passed. If we hit the expansion
threshold twice an hour apart, there's no reason not to try cleanup
again.

Generally, the way I'm viewing this is that a syscache entry means
paying memory to save CPU time. Each 8kB of memory we use to store
system cache entries is one less block we have for the OS page cache
to hold onto our data blocks. If we had an oracle (the kind from
Delphi, not Redwood City) that told us with perfect accuracy when to
discard syscache entries, it would throw away syscache entries
whenever the marginal execution-time performance we could buy from
another 8kB in the page cache is greater than the marginal
execution-time performance we could buy from those syscache entries.
In reality, it's hard to know which of those things is of greater
value. If the system isn't meaningfully memory-constrained, we ought
to just always hang onto the syscache entries, as we do today, but
it's hard to know that. I think the place where this really becomes a
problem is on system with hundreds of connections + thousands of
tables + connection pooling; without some back-pressure, every backend
eventually caches everything, putting the system under severe memory
pressure for basically no performance gain. Each new use of the
connection is probably for a limited set of tables, and only those
tables really syscache entries; holding onto things used long in the
past doesn't save enough to justify the memory used.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

At Mon, 18 Dec 2017 12:14:24 -0500, Robert Haas <robertmhaas@gmail.com> wrote in <CA+TgmoaWLBzUasvVs-q=dfBr3pLWSUCQnbqLk-MT7iX4eyrinA@mail.gmail.com>

On Mon, Dec 18, 2017 at 11:46 AM, Andres Freund <andres@anarazel.de> wrote:

I'm not 100% convinced either - but I also don't think it matters all
that terribly much. As long as the overall hash hit rate is decent,
minor increases in the absolute number of misses don't really matter
that much for syscache imo. I'd personally go for something like:

1) When about to resize, check if there's entries of a generation -2
around.

Don't resize if more than 15% of entries could be freed. Also, stop
reclaiming at that threshold, to avoid unnecessary purging cache
entries.

Using two generations allows a bit more time for cache entries to
marked as fresh before resizing next.

2) While resizing increment generation count by one.

3) Once a minute, increment generation count by one.

The one thing I'm not quite have a good handle upon is how much, and if
any, cache reclamation to do at 3). We don't really want to throw away
all the caches just because a connection has been idle for a few
minutes, in a connection pool that can happen occasionally. I think I'd
for now *not* do any reclamation except at resize boundaries.

My starting inclination was almost the opposite. I think that you
might be right that a minute or two of idle time isn't sufficient
reason to flush our local cache, but I'd be inclined to fix that by
incrementing the generation count every 10 minutes or so rather than
every minute, and still flush things more then 1 generation old. The
reason for that is that I think we should ensure that the system
doesn't sit there idle forever with a giant cache. If it's not using
those cache entries, I'd rather have it discard them and rebuild the
cache when it becomes active again.

I see three kinds of syscache entries.

A. An entry for an actually existing object.

This is literally a syscache entry. This kind of entry is not
necessary to be removed but can be removed after ignorance for
a certain period of time.

B. An entry for an object which once existed but no longer.

This can be removed any time after the removal of the object
and is a main cause of stats bloat or relcache bloat which are
the motive of this thread. We can know whether the entries of
this kind are removable using cache invalidation
mechanism. (the patch upthread)

We can queue the oids that specify the entries to remove, then
actually remove at the next resize. (And this also could be
another cause of bloat. So we could forcibly flush a hash when
the oid list becomes longer than some threashold.)

C. An entry for a just non-existent objects.

I'm not sure how we should treat this since the necessity of a
entry of the kind purely stands on whether the entry will be
accessed sometime. But we could put the same assumption to A.

Now, I also see that your point about trying to clean up before
resizing. That does seem like a good idea, although we have to be
careful not to be too eager to clean up there, or we'll just result in
artificially limiting the cache size when it's unwise to do so. But I
guess that's what you meant by "Also, stop reclaiming at that
threshold, to avoid unnecessary purging cache entries." I think the
idea you are proposing is that:

1. The first time we are due to expand the hash table, we check
whether we can forestall that expansion by doing a cleanup; if so, we
do that instead.

2. After that, we just expand.

That seems like a fairly good idea, although it might be a better idea
to allow cleanup if enough time has passed. If we hit the expansion
threshold twice an hour apart, there's no reason not to try cleanup
again.

Aa session with intermittently executes queries run in a very
short time could be considered as an example workload where
cleanup with such criteria is unwelcomed. But syscache won't
bloat in the case.

Generally, the way I'm viewing this is that a syscache entry means
paying memory to save CPU time. Each 8kB of memory we use to store
system cache entries is one less block we have for the OS page cache
to hold onto our data blocks. If we had an oracle (the kind from

Sure

Delphi, not Redwood City) that told us with perfect accuracy when to
discard syscache entries, it would throw away syscache entries

Except for the B in the aboves. The logic seems somewhat alien to
the time-based cleanup but this can be the measure for quick
bloat of some syscahces.

whenever the marginal execution-time performance we could buy from
another 8kB in the page cache is greater than the marginal
execution-time performance we could buy from those syscache entries.
In reality, it's hard to know which of those things is of greater
value. If the system isn't meaningfully memory-constrained, we ought
to just always hang onto the syscache entries, as we do today, but
it's hard to know that. I think the place where this really becomes a
problem is on system with hundreds of connections + thousands of
tables + connection pooling; without some back-pressure, every backend
eventually caches everything, putting the system under severe memory
pressure for basically no performance gain. Each new use of the
connection is probably for a limited set of tables, and only those
tables really syscache entries; holding onto things used long in the
past doesn't save enough to justify the memory used.

Agreed. The following is the whole image of the measure for
syscache bloat considering "quick bloat". (I still think it is
wanted under some situations.)

1. If a removal of any objects that make some syscache entries
stale (this cannot be checked without scanning whole a hash so
just queue it into, for exameple, recently_removed_relations
OID hash.)

2. If the number of the oid-hash entries reasches 1000 or 10000
(mmm. quite arbitrary..), Immediately clean up syscaches that
accepts/needs removed-reloid cleanup. (The oid hash might be
needed separately for each target cache to avoid readandunt
scan, or to get rid a kind of generation management in the oid
hash.)

3.

1. The first time we are due to expand the hash table, we check
whether we can forestall that expansion by doing a cleanup; if so, we
do that instead.

And if there's any entry in the removed-reloid hash it is
considered while cleanup.

4.

2. After that, we just expand.

That seems like a fairly good idea, although it might be a better idea
to allow cleanup if enough time has passed. If we hit the expansion
threshold twice an hour apart, there's no reason not to try cleanup
again.

1 + 2 and 3 + 4 can be implemented as separate patches and I'll
do the latter first.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

On Tue, Dec 19, 2017 at 3:31 AM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:

I see three kinds of syscache entries.

A. An entry for an actually existing object.
B. An entry for an object which once existed but no longer.
C. An entry for a just non-existent objects.

I'm not convinced that it's useful to divide things up this way.
Regardless of whether the syscache entries is a positive entry, a
negative entry for a dropped object, or a negative energy for an
object that never existed in the first place, it's valuable if it's
likely to get used again and worthless if not. Positive entries may
get used repeatedly, or not; negative entries may get used repeatedly,
or not.

Generally, the way I'm viewing this is that a syscache entry means
paying memory to save CPU time. Each 8kB of memory we use to store
system cache entries is one less block we have for the OS page cache
to hold onto our data blocks. If we had an oracle (the kind from

Sure

Delphi, not Redwood City) that told us with perfect accuracy when to
discard syscache entries, it would throw away syscache entries

Except for the B in the aboves. The logic seems somewhat alien to
the time-based cleanup but this can be the measure for quick
bloat of some syscahces.

I guess I still don't see why B is different. If somebody sits there
and runs queries against non-existent table names at top speed, maybe
they'll query the same non-existent table entries more than once, in
which case keeping the negative entries for the non-existent table
names around until they stop doing it may improve performance. If
they are sitting there and running queries against randomly-generated
non-existent table names at top speed, then they'll generate a lot of
catcache bloat, but that's not really any different from a database
with a large number of tables that DO exist which are queried at
random. Workloads that access a lot of objects, whether those objects
exist or not, are going to use up a lot of cache entries, and I guess
that just seems OK to me.

Agreed. The following is the whole image of the measure for
syscache bloat considering "quick bloat". (I still think it is
wanted under some situations.)

1. If a removal of any objects that make some syscache entries
stale (this cannot be checked without scanning whole a hash so
just queue it into, for exameple, recently_removed_relations
OID hash.)

If we just let some sort of cleanup process that generally blows away
rarely-used entries get rid of those entries too, then it should
handle this case, too, because the cache entries pertaining to removed
relations (or schemas) probably won't get used after that (and if they
do, we should keep them). So I don't see that there is a need for
this, and it drew objections upthread because of the cost of scanning
the whole hash table. Batching relations together might help, but it
doesn't really seem worth trying to sort out the problems with this
idea when we can do something better and more general.

2. If the number of the oid-hash entries reasches 1000 or 10000
(mmm. quite arbitrary..), Immediately clean up syscaches that
accepts/needs removed-reloid cleanup. (The oid hash might be
needed separately for each target cache to avoid readandunt
scan, or to get rid a kind of generation management in the oid
hash.)

That is bound to draw a strong negative response from Tom, and for
good reason. If the number of relations in the working set is 1001
and your cleanup threshold is 1000, cleanups will happen constantly
and performance will be poor. This is exactly why, as I said in the
second email on this thread, the limit of on the size of the relcache
was removed.

1. The first time we are due to expand the hash table, we check
whether we can forestall that expansion by doing a cleanup; if so, we
do that instead.

And if there's any entry in the removed-reloid hash it is
considered while cleanup.

As I say, I don't think there's any need for a removed-reloid hash.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Dec 19, 2017 at 3:31 AM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:

I see three kinds of syscache entries.

A. An entry for an actually existing object.
B. An entry for an object which once existed but no longer.
C. An entry for a just non-existent objects.

I'm not convinced that it's useful to divide things up this way.

Actually, I don't believe that case B exists at all; such an entry
should get blown away by syscache invalidation when we commit the
DROP command. If one were to stick around, we'd risk false positive
lookups later.

I guess I still don't see why B is different. If somebody sits there
and runs queries against non-existent table names at top speed, maybe
they'll query the same non-existent table entries more than once, in
which case keeping the negative entries for the non-existent table
names around until they stop doing it may improve performance.

FWIW, my recollection is that the reason for negative cache entries
is that there are some very common patterns where we probe for object
names (not just table names, either) that aren't there, typically as
a side effect of walking through the search_path looking for a match
to an unqualified object name. Those cache entries aren't going to
get any less useful than the positive entry for the ultimately-found
object. So from a lifespan point of view I'm not very sure that it's
worth distinguishing cases A and C.

It's conceivable that we could rewrite all the lookup algorithms
so that they didn't require negative cache entries to have good
performance ... but I doubt that that's easy to do.

regards, tom lane

At Tue, 19 Dec 2017 13:14:09 -0500, Tom Lane <tgl@sss.pgh.pa.us> wrote in <748.1513707249@sss.pgh.pa.us>

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Dec 19, 2017 at 3:31 AM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:

I see three kinds of syscache entries.

A. An entry for an actually existing object.
B. An entry for an object which once existed but no longer.
C. An entry for a just non-existent objects.

I'm not convinced that it's useful to divide things up this way.

Actually, I don't believe that case B exists at all; such an entry
should get blown away by syscache invalidation when we commit the
DROP command. If one were to stick around, we'd risk false positive
lookups later.

As I have shown upthread, access to a temporary table (*1) leaves
several STATRELATTINH entries after DROPing, and it doesn't have
a chance to be deleted. SELECTing a nonexistent table in a schema
(*2) also leaves a RELNAMENSP entry after DROPing the schema. I'm
not sure that the latter happen so frequently but the former
happens rather frequently and quickly bloats the syscache once
happens. However no false positive can happen since such entiries
cannot be reached without parent objects, but on the other hand
they have no chance to be deleted.

*1: begin; create temp table t1 (a int, b int, c int, d int, e int, f int, g int, h int, i int, j int) on commit drop; insert into t1 values (1, 2, 3, 4, 5, 6, 7, 8, 9, 10); select * from t1; commit;

*2: create schema foo; select * from foo.invalid; drop schema foo;

I guess I still don't see why B is different. If somebody sits there
and runs queries against non-existent table names at top speed, maybe
they'll query the same non-existent table entries more than once, in
which case keeping the negative entries for the non-existent table
names around until they stop doing it may improve performance.

FWIW, my recollection is that the reason for negative cache entries
is that there are some very common patterns where we probe for object
names (not just table names, either) that aren't there, typically as
a side effect of walking through the search_path looking for a match
to an unqualified object name. Those cache entries aren't going to
get any less useful than the positive entry for the ultimately-found
object. So from a lifespan point of view I'm not very sure that it's
worth distinguishing cases A and C.

Agreed.

It's conceivable that we could rewrite all the lookup algorithms
so that they didn't require negative cache entries to have good
performance ... but I doubt that that's easy to do.

That sounds to me to be the same as improving the performance of
systable scan as the same as local hash. Lockless systable
(index) might work (if possible)?

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.

Thanks!

--
Kyotaro Horiguchi
NTT Open Source Software Center

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

On 2017-12-26 18:19:16 +0900, Kyotaro HORIGUCHI wrote:

--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -733,6 +733,9 @@ void
SetCurrentStatementStartTimestamp(void)
{
stmtStartTimestamp = GetCurrentTimestamp();
+
+	/* Set this time stamp as aproximated current time */
+	SetCatCacheClock(stmtStartTimestamp);
}

Hm.

+ * Remove entries that haven't been accessed for a certain time.
+ *
+ * Sometimes catcache entries are left unremoved for several reasons.

I'm unconvinced that that's ok for positive entries, entirely regardless
of this patch.

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.
+ */

This needs polish.

+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

This doesn't look nice, the names descriptive enough to be self evident,
and there's no comments what these random arrays mean. And some specify
lenght (and have differing number of elements!) and others don't.

+	/* 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

What?

+			/*
+			 * 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)
+			{

So this is hardcoded, without any sort of cache pressure logic? Doesn't
that mean we'll often *severely* degrade performance if a backend is
idle for a while?

Greetings,

Andres Freund

On Thu, Mar 1, 2018 at 1:54 PM, Andres Freund <andres@anarazel.de> wrote:

So this is hardcoded, without any sort of cache pressure logic? Doesn't
that mean we'll often *severely* degrade performance if a backend is
idle for a while?

Well, it is true that if we flush cache entries that haven't been used
in a long time, a backend that is idle for a long time might be a bit
slow when (and if) it eventually becomes non-idle, because it may have
to reload some of those flushed entries. On the other hand, a backend
that holds onto a large number of cache entries that it's not using
for tens of minutes at a time degrades the performance of the whole
system unless, of course, you're running on a machine that is under no
memory pressure at all. I don't understand why people keep acting as
if holding onto cache entries regardless of how infrequently they're
being used is an unalloyed good.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Hi,

On 2018-03-01 14:24:56 -0500, Robert Haas wrote:

On Thu, Mar 1, 2018 at 1:54 PM, Andres Freund <andres@anarazel.de> wrote:

So this is hardcoded, without any sort of cache pressure logic? Doesn't
that mean we'll often *severely* degrade performance if a backend is
idle for a while?

Well, it is true that if we flush cache entries that haven't been used
in a long time, a backend that is idle for a long time might be a bit
slow when (and if) it eventually becomes non-idle, because it may have
to reload some of those flushed entries.

Right. Which might be very painful latency wise. And with poolers it's
pretty easy to get into situations like that, without the app
influencing it.

On the other hand, a backend that holds onto a large number of cache
entries that it's not using for tens of minutes at a time degrades the
performance of the whole system unless, of course, you're running on a
machine that is under no memory pressure at all.

But it's *extremely* common to have no memory pressure these days. The
inverse definitely also exists.

I don't understand why people keep acting as if holding onto cache
entries regardless of how infrequently they're being used is an
unalloyed good.

Huh? I'm definitely not arguing for that? I think we want a feature like
this, I just don't think the logic when to prune is quite sophisticated
enough?

Greetings,

Andres Freund

On Thu, Mar 1, 2018 at 2:29 PM, Andres Freund <andres@anarazel.de> wrote:

Right. Which might be very painful latency wise. And with poolers it's
pretty easy to get into situations like that, without the app
influencing it.

Really? I'm not sure I believe that. You're talking perhaps a few
milliseconds - maybe less - of additional latency on a connection
that's been idle for many minutes. You need to have a workload that
involves leaving connections idle for very long periods but has
extremely tight latency requirements when it does finally send a
query. I suppose such workloads exist, but I would not think them
common.

Anyway, I don't mind making the exact timeout a GUC (with 0 disabling
the feature altogether) if that addresses your concern, but in general
I think that it's reasonable to accept that a connection that's been
idle for a long time may have a little bit more latency than usual
when you start using it again. That could happen for other reasons
anyway -- e.g. the cache could have been flushed because of concurrent
DDL on the objects you were accessing, by a syscache reset caused by a
flood of temp objects being created, or by the operating system
deciding to page out some of your data, or by your data getting
evicted from the CPU caches, or by being scheduled onto a NUMA node
different than the one that contains its data. Operating systems have
been optimizing for the performance of relatively active processes
over ones that have been idle for a long time since the 1960s or
earlier, and I don't know of any reason why PostgreSQL shouldn't do
the same.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 2018-03-01 14:49:26 -0500, Robert Haas wrote:

On Thu, Mar 1, 2018 at 2:29 PM, Andres Freund <andres@anarazel.de> wrote:

Right. Which might be very painful latency wise. And with poolers it's
pretty easy to get into situations like that, without the app
influencing it.

Really? I'm not sure I believe that. You're talking perhaps a few
milliseconds - maybe less - of additional latency on a connection
that's been idle for many minutes.

I've seen latency increases in second+ ranges due to empty cat/sys/rel
caches. And the connection doesn't have to be idle, it might just have
been active for a different application doing different things, thus
accessing different cache entries. With a pooler you can trivially end
up switch connections occasionally between different [parts of]
applications, and you don't want performance to suck after each time.
You also don't want to use up all memory, I entirely agree on that.

Anyway, I don't mind making the exact timeout a GUC (with 0 disabling
the feature altogether) if that addresses your concern, but in general
I think that it's reasonable to accept that a connection that's been
idle for a long time may have a little bit more latency than usual
when you start using it again.

I don't think that'd quite address my concern. I just don't think that
the granularity (drop all entries older than xxx sec at the next resize)
is right. For one I don't want to drop stuff if the cache size isn't a
problem for the current memory budget. For another, I'm not convinced
that dropping entries from the current "generation" at resize won't end
up throwing away too much.

If we'd a guc 'syscache_memory_target' and we'd only start pruning if
above it, I'd be much happier.

Greetings,

Andres Freund

On Thu, Mar 1, 2018 at 3:01 PM, Andres Freund <andres@anarazel.de> wrote:

On 2018-03-01 14:49:26 -0500, Robert Haas wrote:

On Thu, Mar 1, 2018 at 2:29 PM, Andres Freund <andres@anarazel.de> wrote:

Right. Which might be very painful latency wise. And with poolers it's
pretty easy to get into situations like that, without the app
influencing it.

Really? I'm not sure I believe that. You're talking perhaps a few
milliseconds - maybe less - of additional latency on a connection
that's been idle for many minutes.

I've seen latency increases in second+ ranges due to empty cat/sys/rel
caches.

How is that even possible unless the system is grossly overloaded?

Anyway, I don't mind making the exact timeout a GUC (with 0 disabling
the feature altogether) if that addresses your concern, but in general
I think that it's reasonable to accept that a connection that's been
idle for a long time may have a little bit more latency than usual
when you start using it again.

I don't think that'd quite address my concern. I just don't think that
the granularity (drop all entries older than xxx sec at the next resize)
is right. For one I don't want to drop stuff if the cache size isn't a
problem for the current memory budget. For another, I'm not convinced
that dropping entries from the current "generation" at resize won't end
up throwing away too much.

I think that a fixed memory budget for the syscache is an idea that
was tried many years ago and basically failed, because it's very easy
to end up with terrible eviction patterns -- e.g. if you are accessing
11 relations in round-robin fashion with a 10-relation cache, your
cache nets you a 0% hit rate but takes a lot more maintenance than
having no cache at all. The time-based approach lets the cache grow
with no fixed upper limit without allowing unused entries to stick
around forever.

If we'd a guc 'syscache_memory_target' and we'd only start pruning if
above it, I'd be much happier.

It does seem reasonable to skip pruning altogether if the cache is
below some threshold size.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Hi,

On 2018-03-01 15:19:26 -0500, Robert Haas wrote:

On Thu, Mar 1, 2018 at 3:01 PM, Andres Freund <andres@anarazel.de> wrote:

On 2018-03-01 14:49:26 -0500, Robert Haas wrote:

On Thu, Mar 1, 2018 at 2:29 PM, Andres Freund <andres@anarazel.de> wrote:

Right. Which might be very painful latency wise. And with poolers it's
pretty easy to get into situations like that, without the app
influencing it.

Really? I'm not sure I believe that. You're talking perhaps a few
milliseconds - maybe less - of additional latency on a connection
that's been idle for many minutes.

I've seen latency increases in second+ ranges due to empty cat/sys/rel
caches.

How is that even possible unless the system is grossly overloaded?

You just need to have catalog contents out of cache and statements
touching a few relations, functions, etc. Indexscan + heap fetch
latencies do add up quite quickly if done sequentially.

I don't think that'd quite address my concern. I just don't think that
the granularity (drop all entries older than xxx sec at the next resize)
is right. For one I don't want to drop stuff if the cache size isn't a
problem for the current memory budget. For another, I'm not convinced
that dropping entries from the current "generation" at resize won't end
up throwing away too much.

I think that a fixed memory budget for the syscache is an idea that
was tried many years ago and basically failed, because it's very easy
to end up with terrible eviction patterns -- e.g. if you are accessing
11 relations in round-robin fashion with a 10-relation cache, your
cache nets you a 0% hit rate but takes a lot more maintenance than
having no cache at all. The time-based approach lets the cache grow
with no fixed upper limit without allowing unused entries to stick
around forever.

I definitely think we want a time based component to this, I just want
to not prune at all if we're below a certain size.

If we'd a guc 'syscache_memory_target' and we'd only start pruning if
above it, I'd be much happier.

It does seem reasonable to skip pruning altogether if the cache is
below some threshold size.

Cool. There might be some issues making that check performant enough,
but I don't have a good intuition on it.

Greetings,

Andres Freund

Hello.

Thank you for the discussion, and sorry for being late to come.

At Thu, 1 Mar 2018 12:26:30 -0800, Andres Freund <andres@anarazel.de> wrote in <20180301202630.2s6untij2x5hpksn@alap3.anarazel.de>

Hi,

On 2018-03-01 15:19:26 -0500, Robert Haas wrote:

On Thu, Mar 1, 2018 at 3:01 PM, Andres Freund <andres@anarazel.de> wrote:

On 2018-03-01 14:49:26 -0500, Robert Haas wrote:

On Thu, Mar 1, 2018 at 2:29 PM, Andres Freund <andres@anarazel.de> wrote:

Right. Which might be very painful latency wise. And with poolers it's
pretty easy to get into situations like that, without the app
influencing it.

Really? I'm not sure I believe that. You're talking perhaps a few
milliseconds - maybe less - of additional latency on a connection
that's been idle for many minutes.

I've seen latency increases in second+ ranges due to empty cat/sys/rel
caches.

How is that even possible unless the system is grossly overloaded?

You just need to have catalog contents out of cache and statements
touching a few relations, functions, etc. Indexscan + heap fetch
latencies do add up quite quickly if done sequentially.

I don't think that'd quite address my concern. I just don't think that
the granularity (drop all entries older than xxx sec at the next resize)
is right. For one I don't want to drop stuff if the cache size isn't a
problem for the current memory budget. For another, I'm not convinced
that dropping entries from the current "generation" at resize won't end
up throwing away too much.

I think that a fixed memory budget for the syscache is an idea that
was tried many years ago and basically failed, because it's very easy
to end up with terrible eviction patterns -- e.g. if you are accessing
11 relations in round-robin fashion with a 10-relation cache, your
cache nets you a 0% hit rate but takes a lot more maintenance than
having no cache at all. The time-based approach lets the cache grow
with no fixed upper limit without allowing unused entries to stick
around forever.

I definitely think we want a time based component to this, I just want
to not prune at all if we're below a certain size.

If we'd a guc 'syscache_memory_target' and we'd only start pruning if
above it, I'd be much happier.

It does seem reasonable to skip pruning altogether if the cache is
below some threshold size.

Cool. There might be some issues making that check performant enough,
but I don't have a good intuition on it.

So..

- Now it gets two new GUC variables named syscache_prune_min_age
and syscache_memory_target. The former is the replacement of
the previous magic number 600 and defaults to the same
number. The latter prevens syscache pruning until exceeding the
size and defaults to 0, means that pruning is always
considered. Documentation for the two variables are also
added.

- Revised the pointed mysterious comment for
CatcacheCleanupOldEntries and some comments are added.

- Fixed the name of the variables for CATCACHE_STATS to be more
descriptive, and added some comments for the code.

The catcache entries accessed within the current transaction
won't be pruned so theoretically a long transaction can bloat
catcache. But I believe it is quite rare, or at least this saves
the most other cases.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

Oops! The previous ptach contained garbage printing in debugging
output.

The attached is the new version without the garbage. Addition to
it, I changed my mind to use DEBUG1 for the debug message since
the frequency is quite low.

No changes in the following cited previous mail.

At Wed, 07 Mar 2018 16:19:23 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in <20180307.161923.178158050.horiguchi.kyotaro@lab.ntt.co.jp>
==================
Hello.

Thank you for the discussion, and sorry for being late to come.

At Thu, 1 Mar 2018 12:26:30 -0800, Andres Freund <andres@anarazel.de> wrote in <20180301202630.2s6untij2x5hpksn@alap3.anarazel.de>

Hi,

On 2018-03-01 15:19:26 -0500, Robert Haas wrote:

On Thu, Mar 1, 2018 at 3:01 PM, Andres Freund <andres@anarazel.de> wrote:

On 2018-03-01 14:49:26 -0500, Robert Haas wrote:

On Thu, Mar 1, 2018 at 2:29 PM, Andres Freund <andres@anarazel.de> wrote:

Right. Which might be very painful latency wise. And with poolers it's
pretty easy to get into situations like that, without the app
influencing it.

Really? I'm not sure I believe that. You're talking perhaps a few
milliseconds - maybe less - of additional latency on a connection
that's been idle for many minutes.

I've seen latency increases in second+ ranges due to empty cat/sys/rel
caches.

How is that even possible unless the system is grossly overloaded?

You just need to have catalog contents out of cache and statements
touching a few relations, functions, etc. Indexscan + heap fetch
latencies do add up quite quickly if done sequentially.

I don't think that'd quite address my concern. I just don't think that
the granularity (drop all entries older than xxx sec at the next resize)
is right. For one I don't want to drop stuff if the cache size isn't a
problem for the current memory budget. For another, I'm not convinced
that dropping entries from the current "generation" at resize won't end
up throwing away too much.

I think that a fixed memory budget for the syscache is an idea that
was tried many years ago and basically failed, because it's very easy
to end up with terrible eviction patterns -- e.g. if you are accessing
11 relations in round-robin fashion with a 10-relation cache, your
cache nets you a 0% hit rate but takes a lot more maintenance than
having no cache at all. The time-based approach lets the cache grow
with no fixed upper limit without allowing unused entries to stick
around forever.

I definitely think we want a time based component to this, I just want
to not prune at all if we're below a certain size.

If we'd a guc 'syscache_memory_target' and we'd only start pruning if
above it, I'd be much happier.

It does seem reasonable to skip pruning altogether if the cache is
below some threshold size.

Cool. There might be some issues making that check performant enough,
but I don't have a good intuition on it.

So..

- Now it gets two new GUC variables named syscache_prune_min_age
and syscache_memory_target. The former is the replacement of
the previous magic number 600 and defaults to the same
number. The latter prevens syscache pruning until exceeding the
size and defaults to 0, means that pruning is always
considered. Documentation for the two variables are also
added.

- Revised the pointed mysterious comment for
CatcacheCleanupOldEntries and some comments are added.

- Fixed the name of the variables for CATCACHE_STATS to be more
descriptive, and added some comments for the code.

The catcache entries accessed within the current transaction
won't be pruned so theoretically a long transaction can bloat
catcache. But I believe it is quite rare, or at least this saves
the most other cases.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

The thing that comes to mind when reading this patch is that some time
ago we made fun of other database software, "they are so complicated to
configure, they have some magical settings that few people understand
how to set". Postgres was so much better because it was simple to set
up, no magic crap. But now it becomes apparent that that only was so
because Postgres sucked, ie., we hadn't yet gotten to the point where we
*needed* to introduce settings like that. Now we finally are?

I have to admit being a little disappointed about that outcome.

I wonder if this is just because we refuse to acknowledge the notion of
a connection pooler. If we did, and the pooler told us "here, this
session is being given back to us by the application, we'll keep it
around until the next app comes along", could we clean the oldest
inactive cache entries at that point? Currently they use DISCARD for
that. Though this does nothing to fix hypothetical cache bloat for
pg_dump in bug #14936.

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Hi,

On 2018-03-07 08:01:38 -0300, Alvaro Herrera wrote:

I wonder if this is just because we refuse to acknowledge the notion of
a connection pooler. If we did, and the pooler told us "here, this
session is being given back to us by the application, we'll keep it
around until the next app comes along", could we clean the oldest
inactive cache entries at that point? Currently they use DISCARD for
that. Though this does nothing to fix hypothetical cache bloat for
pg_dump in bug #14936.

I'm not seeing how this solves anything? You don't want to throw all
caches away, therefore you need a target size. Then there's also the
case of the cache being too large in a single "session".

Greetings,

Andres Freund

Hello,

Andres Freund wrote:

On 2018-03-07 08:01:38 -0300, Alvaro Herrera wrote:

I wonder if this is just because we refuse to acknowledge the notion of
a connection pooler. If we did, and the pooler told us "here, this
session is being given back to us by the application, we'll keep it
around until the next app comes along", could we clean the oldest
inactive cache entries at that point? Currently they use DISCARD for
that. Though this does nothing to fix hypothetical cache bloat for
pg_dump in bug #14936.

I'm not seeing how this solves anything? You don't want to throw all
caches away, therefore you need a target size. Then there's also the
case of the cache being too large in a single "session".

Oh, I wasn't suggesting to throw away the whole cache at that point;
only that that is a convenient to do whatever cleanup we want to do.
What I'm not clear about is exactly what is the cleanup that we want to
do at that point. You say it should be based on some configured size;
Robert says any predefined size breaks [performance for] the case where
the workload uses size+1, so let's use time instead (evict anything not
used in more than X seconds?), but keeping in mind that a workload that
requires X+1 would also break. So it seems we've arrived at the
conclusion that the only possible solution is to let the user tell us
what time/size to use. But that sucks, because the user doesn't know
either (maybe they can measure, but how?), and they don't even know that
this setting is there to be tweaked; and if there is a performance
problem, how do they figure whether or not it can be fixed by fooling
with this parameter? I mean, maybe it's set to 10 and we suggest "maybe
11 works better" but it turns out not to, so "maybe 12 works better"?
How do you know when to stop increasing it?

This seems a bit like max_fsm_pages, that is to say, a disaster that was
only fixed by removing it.

Thanks,

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2018-03-07 14:48:48 -0300, Alvaro Herrera wrote:

Oh, I wasn't suggesting to throw away the whole cache at that point;
only that that is a convenient to do whatever cleanup we want to do.

But why is that better than doing so continuously?

What I'm not clear about is exactly what is the cleanup that we want to
do at that point. You say it should be based on some configured size;
Robert says any predefined size breaks [performance for] the case where
the workload uses size+1, so let's use time instead (evict anything not
used in more than X seconds?), but keeping in mind that a workload that
requires X+1 would also break.

We mostly seem to have found that adding a *minimum* size before
starting evicting basedon time solves both of our concerns?

So it seems we've arrived at the
conclusion that the only possible solution is to let the user tell us
what time/size to use. But that sucks, because the user doesn't know
either (maybe they can measure, but how?), and they don't even know that
this setting is there to be tweaked; and if there is a performance
problem, how do they figure whether or not it can be fixed by fooling
with this parameter? I mean, maybe it's set to 10 and we suggest "maybe
11 works better" but it turns out not to, so "maybe 12 works better"?
How do you know when to stop increasing it?

I don't think it's that complicated, for the size figure. Having a knob
that controls how much memory a backend uses isn't a new concept, and
can definitely depend on the usecase.

This seems a bit like max_fsm_pages, that is to say, a disaster that was
only fixed by removing it.

I don't think that's a meaningful comparison. max_fms_pages had
persistent effect, couldn't be tuned without restarts, and the
performance dropoffs were much more "cliff" like.

Greetings,

Andres Freund

On Wed, Mar 7, 2018 at 6:01 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:

The thing that comes to mind when reading this patch is that some time
ago we made fun of other database software, "they are so complicated to
configure, they have some magical settings that few people understand
how to set". Postgres was so much better because it was simple to set
up, no magic crap. But now it becomes apparent that that only was so
because Postgres sucked, ie., we hadn't yet gotten to the point where we
*needed* to introduce settings like that. Now we finally are?

I have to admit being a little disappointed about that outcome.

I think your disappointment is a little excessive. I am not convinced
of the need either for this to have any GUCs at all, but if it makes
other people happy to have them, then I think it's worth accepting
that as the price of getting the feature into the tree. These are
scarcely the first GUCs we have that are hard to tune. work_mem is a
terrible knob, and there are probably like very few people who know
how to set ssl_ecdh_curve to anything other than the default, and
what's geqo_selection_bias good for, anyway? I'm not sure what makes
the settings we're adding here any different. Most people will ignore
them, and a few people who really care can change the values.

I wonder if this is just because we refuse to acknowledge the notion of
a connection pooler. If we did, and the pooler told us "here, this
session is being given back to us by the application, we'll keep it
around until the next app comes along", could we clean the oldest
inactive cache entries at that point? Currently they use DISCARD for
that. Though this does nothing to fix hypothetical cache bloat for
pg_dump in bug #14936.

We could certainly clean the oldest inactive cache entries at that
point, but there's no guarantee that would be the right thing to do.
If the working set across all applications is small enough that you
can keep them all in the caches all the time, then you should do that,
for maximum performance. If not, DISCARD ALL should probably flush
everything that the last application needed and the next application
won't. But without some configuration knob, you have zero way of
knowing how concerned the user is about saving memory in this place
vs. improving performance by reducing catalog scans. Even with such a
knob it's a little difficult to say which things actually ought to be
thrown away.

I think this is a related problem, but a different one. I also think
we ought to have built-in connection pooling. :-)

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

From: Alvaro Herrera [mailto:alvherre@alvh.no-ip.org]

The thing that comes to mind when reading this patch is that some time ago
we made fun of other database software, "they are so complicated to configure,
they have some magical settings that few people understand how to set".
Postgres was so much better because it was simple to set up, no magic crap.
But now it becomes apparent that that only was so because Postgres sucked,
ie., we hadn't yet gotten to the point where we
*needed* to introduce settings like that. Now we finally are?

Yes. We are now facing the problem of too much memory use by PostgreSQL, where about some applications randomly access about 200,000 tables. It is estimated based on a small experiment that each backend will use several to ten GBs of local memory for CacheMemoryContext. The total memory use will become over 1 TB when the expected maximum connections are used.

I haven't looked at this patch, but does it evict all kinds of entries in CacheMemoryContext, ie. relcache, plancache, etc?

Regards
Takayuki Tsunakawa

Hello,

At Thu, 8 Mar 2018 00:28:04 +0000, "Tsunakawa, Takayuki" <tsunakawa.takay@jp.fujitsu.com> wrote in <0A3221C70F24FB45833433255569204D1F8FF0D9@G01JPEXMBYT05>

From: Alvaro Herrera [mailto:alvherre@alvh.no-ip.org]

The thing that comes to mind when reading this patch is that some time ago
we made fun of other database software, "they are so complicated to configure,
they have some magical settings that few people understand how to set".
Postgres was so much better because it was simple to set up, no magic crap.
But now it becomes apparent that that only was so because Postgres sucked,
ie., we hadn't yet gotten to the point where we
*needed* to introduce settings like that. Now we finally are?

Yes. We are now facing the problem of too much memory use by PostgreSQL, where about some applications randomly access about 200,000 tables. It is estimated based on a small experiment that each backend will use several to ten GBs of local memory for CacheMemoryContext. The total memory use will become over 1 TB when the expected maximum connections are used.

I haven't looked at this patch, but does it evict all kinds of entries in CacheMemoryContext, ie. relcache, plancache, etc?

This works only for syscaches, which could bloat with entries for
nonexistent objects.

Plan cache is a utterly deferent thing. It is abandoned at the
end of a transaction or such like.

Relcache is not based on catcache and out of the scope of this
patch since it doesn't get bloat with nonexistent entries. It
uses dynahash and we could introduce a similar feature to it if
we are willing to cap relcache size.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> writes:

At Thu, 8 Mar 2018 00:28:04 +0000, "Tsunakawa, Takayuki" <tsunakawa.takay@jp.fujitsu.com> wrote in <0A3221C70F24FB45833433255569204D1F8FF0D9@G01JPEXMBYT05>

Yes. We are now facing the problem of too much memory use by PostgreSQL, where about some applications randomly access about 200,000 tables. It is estimated based on a small experiment that each backend will use several to ten GBs of local memory for CacheMemoryContext. The total memory use will become over 1 TB when the expected maximum connections are used.

I haven't looked at this patch, but does it evict all kinds of entries in CacheMemoryContext, ie. relcache, plancache, etc?

This works only for syscaches, which could bloat with entries for
nonexistent objects.

Plan cache is a utterly deferent thing. It is abandoned at the
end of a transaction or such like.

When I was at Salesforce, we had *substantial* problems with plancache
bloat. The driving factor there was plans associated with plpgsql
functions, which Salesforce had a huge number of. In an environment
like that, there would be substantial value in being able to prune
both the plancache and plpgsql's function cache. (Note that neither
of those things are "abandoned at the end of a transaction".)

Relcache is not based on catcache and out of the scope of this
patch since it doesn't get bloat with nonexistent entries. It
uses dynahash and we could introduce a similar feature to it if
we are willing to cap relcache size.

I think if the case of concern is an application with 200,000 tables,
it's just nonsense to claim that relcache size isn't an issue.

In short, it's not really apparent to me that negative syscache entries
are the major problem of this kind. I'm afraid that you're drawing very
large conclusions from a specific workload. Maybe we could fix that
workload some other way.

regards, tom lane

At Wed, 07 Mar 2018 23:12:29 -0500, Tom Lane <tgl@sss.pgh.pa.us> wrote in <352.1520482349@sss.pgh.pa.us>

Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> writes:

At Thu, 8 Mar 2018 00:28:04 +0000, "Tsunakawa, Takayuki" <tsunakawa.takay@jp.fujitsu.com> wrote in <0A3221C70F24FB45833433255569204D1F8FF0D9@G01JPEXMBYT05>

Yes. We are now facing the problem of too much memory use by PostgreSQL, where about some applications randomly access about 200,000 tables. It is estimated based on a small experiment that each backend will use several to ten GBs of local memory for CacheMemoryContext. The total memory use will become over 1 TB when the expected maximum connections are used.

I haven't looked at this patch, but does it evict all kinds of entries in CacheMemoryContext, ie. relcache, plancache, etc?

This works only for syscaches, which could bloat with entries for
nonexistent objects.

Plan cache is a utterly deferent thing. It is abandoned at the
end of a transaction or such like.

When I was at Salesforce, we had *substantial* problems with plancache
bloat. The driving factor there was plans associated with plpgsql
functions, which Salesforce had a huge number of. In an environment
like that, there would be substantial value in being able to prune
both the plancache and plpgsql's function cache. (Note that neither
of those things are "abandoned at the end of a transaction".)

Mmm. Right. Thanks for pointing it. Anyway plan cache seems to be
a different thing.

Relcache is not based on catcache and out of the scope of this
patch since it doesn't get bloat with nonexistent entries. It
uses dynahash and we could introduce a similar feature to it if
we are willing to cap relcache size.

I think if the case of concern is an application with 200,000 tables,
it's just nonsense to claim that relcache size isn't an issue.

In short, it's not really apparent to me that negative syscache entries
are the major problem of this kind. I'm afraid that you're drawing very
large conclusions from a specific workload. Maybe we could fix that
workload some other way.

The current patch doesn't consider whether an entry is negative
or positive(?). Just clean up all entries based on time.

If relation has to have the same characterictics to syscaches, it
might be better be on the catcache mechanism, instaed of adding
the same pruning mechanism to dynahash..

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

At Fri, 09 Mar 2018 17:40:01 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in <20180309.174001.202113825.horiguchi.kyotaro@lab.ntt.co.jp>

In short, it's not really apparent to me that negative syscache entries
are the major problem of this kind. I'm afraid that you're drawing very
large conclusions from a specific workload. Maybe we could fix that
workload some other way.

The current patch doesn't consider whether an entry is negative
or positive(?). Just clean up all entries based on time.

If relation has to have the same characterictics to syscaches, it
might be better be on the catcache mechanism, instaed of adding
the same pruning mechanism to dynahash..

For the moment, I added such feature to dynahash and let only
relcache use it in this patch. Hash element has different shape
in "prunable" hash and pruning is performed in a similar way
sharing the setting with syscache. This seems working fine.

It is bit uneasy that all syscaches and relcache shares the same
value of syscache_memory_target...

Something like the sttached test script causes relcache
"bloat". Server emits the following log entries in DEBUG1 message
level.

DEBUG: removed 11240/32769 entries from hash "Relcache by OID" at character 15

# The last few words are just garbage I mentioned in another thread.

The last two patches do that (as PoC).

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

Oops.

At Mon, 12 Mar 2018 17:34:08 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in <20180312.173408.162882093.horiguchi.kyotaro@lab.ntt.co.jp>

Something like the sttached test script causes relcache

This is that.

At Mon, 12 Mar 2018 17:34:08 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in <20180312.173408.162882093.horiguchi.kyotaro@lab.ntt.co.jp>

In short, it's not really apparent to me that negative syscache entries
are the major problem of this kind. I'm afraid that you're drawing very
large conclusions from a specific workload. Maybe we could fix that
workload some other way.

The current patch doesn't consider whether an entry is negative
or positive(?). Just clean up all entries based on time.

If relation has to have the same characterictics to syscaches, it
might be better be on the catcache mechanism, instaed of adding
the same pruning mechanism to dynahash..

For the moment, I added such feature to dynahash and let only
relcache use it in this patch. Hash element has different shape
in "prunable" hash and pruning is performed in a similar way
sharing the setting with syscache. This seems working fine.

I gave consideration on plancache. The most different
characteristics from catcache and relcache is the fact that it is
not voluntarily removable since CachedPlanSource, the root struct
of a plan cache, holds some indispensable inforamtion. In regards
to prepared queries, even if we store the information into
another location, for example in "Prepred Queries" hash, it
merely moving a big data into another place.

Looking into CachedPlanSoruce, generic plan is a part that is
safely removable since it is rebuilt as necessary. Keeping "old"
plancache entries not holding a generic plan can reduce memory
usage.

For testing purpose, I made 50000 parepared statement like
"select sum(c) from p where e < $" on 100 partitions,

With disabling the feature (0004 patch) VSZ of the backend
exceeds 3GB (It is still increasing at the moment), while it
stops to increase at about 997MB for min_cached_plans = 1000 and
plancache_prune_min_age = '10s'.

# 10s is apparently short for acutual use, of course.

It is expected to be significant amount if the plan is large
enough but I'm still not sure it is worth doing, or is a right
way.

The attached is the patch set including this plancache stuff.

0001- catcache time-based expiration (The origin of this thread)
0002- introduces dynahash pruning feature
0003- implement relcache pruning using 0002
0004- (perhaps) independent from the three above. PoC of
plancache pruning. Details are shown above.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

On 3/15/18 1:12 AM, Kyotaro HORIGUCHI wrote:

At Mon, 12 Mar 2018 17:34:08 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in >
The attached is the patch set including this plancache stuff.

0001- catcache time-based expiration (The origin of this thread)
0002- introduces dynahash pruning feature
0003- implement relcache pruning using 0002
0004- (perhaps) independent from the three above. PoC of
plancache pruning. Details are shown above.

It looks like this should be marked Needs Review so I have done so. If
that's not right please change it back or let me know and I will.

Regards,
--
-David
david@pgmasters.net

Hello.

At Wed, 21 Mar 2018 15:28:07 -0400, David Steele <david@pgmasters.net> wrote in <43095b16-14fc-e4d8-3310-2b86eaaab662@pgmasters.net>

On 3/15/18 1:12 AM, Kyotaro HORIGUCHI wrote:

At Mon, 12 Mar 2018 17:34:08 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in >
The attached is the patch set including this plancache stuff.

0001- catcache time-based expiration (The origin of this thread)
0002- introduces dynahash pruning feature
0003- implement relcache pruning using 0002
0004- (perhaps) independent from the three above. PoC of
plancache pruning. Details are shown above.

It looks like this should be marked Needs Review so I have done so. If
that's not right please change it back or let me know and I will.

Mmm. I haven't noticed that. Thanks!

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

On 2018-03-23 17:01:11 +0900, Kyotaro HORIGUCHI wrote:

Hello.

At Wed, 21 Mar 2018 15:28:07 -0400, David Steele <david@pgmasters.net> wrote in <43095b16-14fc-e4d8-3310-2b86eaaab662@pgmasters.net>

On 3/15/18 1:12 AM, Kyotaro HORIGUCHI wrote:

At Mon, 12 Mar 2018 17:34:08 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in >
The attached is the patch set including this plancache stuff.

0001- catcache time-based expiration (The origin of this thread)
0002- introduces dynahash pruning feature
0003- implement relcache pruning using 0002
0004- (perhaps) independent from the three above. PoC of
plancache pruning. Details are shown above.

It looks like this should be marked Needs Review so I have done so. If
that's not right please change it back or let me know and I will.

Mmm. I haven't noticed that. Thanks!

I actually think this should be marked as returned with feedback, or at
the very least moved to the next CF. This is entirely new development
within the last CF. There's no realistic way we can get this into v11.

Greetings,

Andres Freund

At Thu, 29 Mar 2018 18:22:59 -0700, Andres Freund <andres@anarazel.de> wrote in <20180330012259.7k3442yz7jighg2t@alap3.anarazel.de>

On 2018-03-23 17:01:11 +0900, Kyotaro HORIGUCHI wrote:

Hello.

At Wed, 21 Mar 2018 15:28:07 -0400, David Steele <david@pgmasters.net> wrote in <43095b16-14fc-e4d8-3310-2b86eaaab662@pgmasters.net>

On 3/15/18 1:12 AM, Kyotaro HORIGUCHI wrote:

At Mon, 12 Mar 2018 17:34:08 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in >
The attached is the patch set including this plancache stuff.

0001- catcache time-based expiration (The origin of this thread)
0002- introduces dynahash pruning feature
0003- implement relcache pruning using 0002
0004- (perhaps) independent from the three above. PoC of
plancache pruning. Details are shown above.

It looks like this should be marked Needs Review so I have done so. If
that's not right please change it back or let me know and I will.

Mmm. I haven't noticed that. Thanks!

I actually think this should be marked as returned with feedback, or at
the very least moved to the next CF. This is entirely new development
within the last CF. There's no realistic way we can get this into v11.

0002-0004 is new, in response to the comment that caches other
than the catcache ought to get the same feature. These can be a
separate development from 0001 for v12. I don't find a measures
to catch the all case at once.

If we agree on the point. I wish to discuss only 0001 for v11.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

Hi,

On 2018-03-30 10:35:48 +0900, Kyotaro HORIGUCHI wrote:

0002-0004 is new, in response to the comment that caches other
than the catcache ought to get the same feature. These can be a
separate development from 0001 for v12. I don't find a measures
to catch the all case at once.

If we agree on the point. I wish to discuss only 0001 for v11.

I'd personally not want to commit a solution for catcaches without also
commiting a solution for a least relcaches in the same release cycle. I
think this patch simply has missed the window for v11.

Greetings,

Andres Freund

At Thu, 29 Mar 2018 18:51:45 -0700, Andres Freund <andres@anarazel.de> wrote in <20180330015145.pvsr6kjtf6tw4uwe@alap3.anarazel.de>

Hi,

On 2018-03-30 10:35:48 +0900, Kyotaro HORIGUCHI wrote:

0002-0004 is new, in response to the comment that caches other
than the catcache ought to get the same feature. These can be a
separate development from 0001 for v12. I don't find a measures
to catch the all case at once.

If we agree on the point. I wish to discuss only 0001 for v11.

I'd personally not want to commit a solution for catcaches without also
commiting a solution for a least relcaches in the same release cycle. I
think this patch simply has missed the window for v11.

Ok. Agreed. I moved this to the next CF.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

Hello. I rebased this patchset.

At Thu, 15 Mar 2018 14:12:46 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in <20180315.141246.130742928.horiguchi.kyotaro@lab.ntt.co.jp>

At Mon, 12 Mar 2018 17:34:08 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in <20180312.173408.162882093.horiguchi.kyotaro@lab.ntt.co.jp>

In short, it's not really apparent to me that negative syscache entries
are the major problem of this kind. I'm afraid that you're drawing very
large conclusions from a specific workload. Maybe we could fix that
workload some other way.

The current patch doesn't consider whether an entry is negative
or positive(?). Just clean up all entries based on time.

If relation has to have the same characterictics to syscaches, it
might be better be on the catcache mechanism, instaed of adding
the same pruning mechanism to dynahash..

This means unifying catcache and dynahash. It doesn't seem
win-win consolidation. Addition to that relcache links palloc'ed
memory which needs additional treat.

Or we could abstract the pruning mechanism applicable to both
machinaries. Specifically unifying CatCacheCleanupOldEntries in
0001 and prune_entries in 0002. Or could refactor dynahash and
rebuild catcache based on dynahash.

For the moment, I added such feature to dynahash and let only
relcache use it in this patch. Hash element has different shape
in "prunable" hash and pruning is performed in a similar way
sharing the setting with syscache. This seems working fine.

I gave consideration on plancache. The most different
characteristics from catcache and relcache is the fact that it is
not voluntarily removable since CachedPlanSource, the root struct
of a plan cache, holds some indispensable inforamtion. In regards
to prepared queries, even if we store the information into
another location, for example in "Prepred Queries" hash, it
merely moving a big data into another place.

Looking into CachedPlanSoruce, generic plan is a part that is
safely removable since it is rebuilt as necessary. Keeping "old"
plancache entries not holding a generic plan can reduce memory
usage.

For testing purpose, I made 50000 parepared statement like
"select sum(c) from p where e < $" on 100 partitions,

With disabling the feature (0004 patch) VSZ of the backend
exceeds 3GB (It is still increasing at the moment), while it
stops to increase at about 997MB for min_cached_plans = 1000 and
plancache_prune_min_age = '10s'.

# 10s is apparently short for acutual use, of course.

It is expected to be significant amount if the plan is large
enough but I'm still not sure it is worth doing, or is a right
way.

The attached is the patch set including this plancache stuff.

0001- catcache time-based expiration (The origin of this thread)
0002- introduces dynahash pruning feature
0003- implement relcache pruning using 0002
0004- (perhaps) independent from the three above. PoC of
plancache pruning. Details are shown above.

I found up to v3 in this thread so I named this version 4.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

On 06/26/2018 05:00 AM, Kyotaro HORIGUCHI wrote:

The attached is the patch set including this plancache stuff.

0001- catcache time-based expiration (The origin of this thread)
0002- introduces dynahash pruning feature
0003- implement relcache pruning using 0002
0004- (perhaps) independent from the three above. PoC of
plancache pruning. Details are shown above.

I found up to v3 in this thread so I named this version 4.

Andres suggested back in March (and again privately to me) that given
how much this has changed from the original this CF item should be
marked Returned With Feedback and the current patchset submitted as a
new item.

Does anyone object to that course of action?

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Hello. The previous v4 patchset was just broken.

At Tue, 26 Jun 2018 18:00:03 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in <20180626.180003.127457941.horiguchi.kyotaro@lab.ntt.co.jp>

Hello. I rebased this patchset.

..

The attached is the patch set including this plancache stuff.

0001- catcache time-based expiration (The origin of this thread)
0002- introduces dynahash pruning feature
0003- implement relcache pruning using 0002
0004- (perhaps) independent from the three above. PoC of
plancache pruning. Details are shown above.

I found up to v3 in this thread so I named this version 4.

Somehow the 0004 was merged into the 0003 and applying 0004
results in failure. I removed 0004 part from the 0003 and rebased
and repost it.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

On 2018-Jul-02, Andrew Dunstan wrote:

Andres suggested back in March (and again privately to me) that given how
much this has changed from the original this CF item should be marked
Returned With Feedback and the current patchset submitted as a new item.

Does anyone object to that course of action?

If doing that makes the "CF count" reset back to one for the new
submission, then I object to that course of action. If we really
think this item does not belong into this commitfest, lets punt it to
the next one. However, it seems rather strange to do so this early in
the cycle. Is there really no small item that could be cherry-picked
from this series to be committed standalone?

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Hi,

On 2018-07-02 21:50:36 -0400, Alvaro Herrera wrote:

On 2018-Jul-02, Andrew Dunstan wrote:

Andres suggested back in March (and again privately to me) that given how
much this has changed from the original this CF item should be marked
Returned With Feedback and the current patchset submitted as a new item.

Does anyone object to that course of action?

If doing that makes the "CF count" reset back to one for the new
submission, then I object to that course of action. If we really
think this item does not belong into this commitfest, lets punt it to
the next one. However, it seems rather strange to do so this early in
the cycle. Is there really no small item that could be cherry-picked
from this series to be committed standalone?

Well, I think it should just have been RWFed last cycle. It got plenty
of feedback. So it doesn't seem that strange to me, not to include it in
the "mop-up" CF? Either way, I don't feel strongly about it, I just know
that I won't have energy for the topic in this CF.

Greetings,

Andres Freund

Hi,

Subject: Re: Protect syscache from bloating with negative cache entries

Hello. The previous v4 patchset was just broken.

Somehow the 0004 was merged into the 0003 and applying 0004 results in failure. I
removed 0004 part from the 0003 and rebased and repost it.

I have some questions about syscache and relcache pruning
though they may be discussed at upper thread or out of point.

Can I confirm about catcache pruning?
syscache_memory_target is the max figure per CatCache.
(Any CatCache has the same max value.)
So the total max size of catalog caches is estimated around or
slightly more than # of SysCache array times syscache_memory_target.

If correct, I'm thinking writing down the above estimation to the document
would help db administrators with estimation of memory usage.
Current description might lead misunderstanding that syscache_memory_target
is the total size of catalog cache in my impression.

Related to the above I just thought changing sysycache_memory_target per CatCache
would make memory usage more efficient.
Though I haven't checked if there's a case that each system catalog cache memory usage varies largely,
pg_class cache might need more memory than others and others might need less.
But it would be difficult for users to check each CatCache memory usage and tune it
because right now postgresql hasn't provided a handy way to check them.
Another option is that users only specify the total memory target size and postgres
dynamically change each CatCache memory target size according to a certain metric.
(, which still seems difficult and expensive to develop per benefit)
What do you think about this?

+       /*                                                                           
+        * Set up pruning.                                                           
+        *                                                                           
+        * We have two knobs to control pruning and a hash can share them of         
+        * syscache.                                                                 
+        *                                                                           
+        */                                                                          
+       if (flags & HASH_PRUNABLE)                                                   
+       {                                                                            
+               hctl->prunable = true;                                               
+               hctl->prune_cb = info->prune_cb;                                     
+               if (info->memory_target)                                             
+                       hctl->memory_target = info->memory_target;                   
+               else                                                                 
+                       hctl->memory_target = &cache_memory_target;                  
+               if (info->prune_min_age)                                             
+                       hctl->prune_min_age = info->prune_min_age;                   
+               else                                                                 
+                       hctl->prune_min_age = &cache_prune_min_age;                  
+       }                                                                            
+       else                                                                         
+               hctl->prunable = false;

As you commented here, guc variable syscache_memory_target and
syscache_prune_min_age are used for both syscache and relcache (HTAB), right?

Do syscache and relcache have the similar amount of memory usage?
If not, I'm thinking that introducing separate guc variable would be fine.
So as syscache_prune_min_age.

Regards,
====================
Takeshi Ideriha
Fujitsu Limited

Hello. Thank you for looking this.

At Wed, 12 Sep 2018 05:16:52 +0000, "Ideriha, Takeshi" <ideriha.takeshi@jp.fujitsu.com> wrote in <4E72940DA2BF16479384A86D54D0988A6F197012@G01JPEXMBKW04>

Hi,

Subject: Re: Protect syscache from bloating with negative cache entries

Hello. The previous v4 patchset was just broken.

Somehow the 0004 was merged into the 0003 and applying 0004 results in failure. I
removed 0004 part from the 0003 and rebased and repost it.

I have some questions about syscache and relcache pruning
though they may be discussed at upper thread or out of point.

Can I confirm about catcache pruning?
syscache_memory_target is the max figure per CatCache.
(Any CatCache has the same max value.)
So the total max size of catalog caches is estimated around or
slightly more than # of SysCache array times syscache_memory_target.

Right.

If correct, I'm thinking writing down the above estimation to the document
would help db administrators with estimation of memory usage.
Current description might lead misunderstanding that syscache_memory_target
is the total size of catalog cache in my impression.

Honestly I'm not sure that is the right design. Howerver, I don't
think providing such formula to users helps users, since they
don't know exactly how many CatCaches and brothres live in their
server and it is a soft limit, and finally only few or just one
catalogs can reach the limit.

The current design based on the assumption that we would have
only one extremely-growable cache in one use case.

Related to the above I just thought changing sysycache_memory_target per CatCache
would make memory usage more efficient.

We could easily have per-cache settings in CatCache, but how do
we provide the knobs for them? I can guess only too much
solutions for that.

Though I haven't checked if there's a case that each system catalog cache memory usage varies largely,
pg_class cache might need more memory than others and others might need less.
But it would be difficult for users to check each CatCache memory usage and tune it
because right now postgresql hasn't provided a handy way to check them.

I supposed that this is used without such a means. Someone
suffers syscache bloat just can set this GUC to avoid the
bloat. End.

Apart from that, in the current patch, syscache_memory_target is
not exact at all in the first place to avoid overhead to count
the correct size. The major difference comes from the size of
cache tuple itself. But I came to think it is too much to omit.

As a *PoC*, in the attached patch (which applies to current
master), size of CTups are counted as the catcache size.

It also provides pg_catcache_size system view just to give a
rough idea of how such view looks. I'll consider more on that but
do you have any opinion on this?

=# select relid::regclass, indid::regclass, size from pg_syscache_sizes order by size desc;
relid | indid | size
-------------------------+-------------------------------------------+--------
pg_class | pg_class_oid_index | 131072
pg_class | pg_class_relname_nsp_index | 131072
pg_cast | pg_cast_source_target_index | 5504
pg_operator | pg_operator_oprname_l_r_n_index | 4096
pg_statistic | pg_statistic_relid_att_inh_index | 2048
pg_proc | pg_proc_proname_args_nsp_index | 2048
..

Another option is that users only specify the total memory target size and postgres
dynamically change each CatCache memory target size according to a certain metric.
(, which still seems difficult and expensive to develop per benefit)
What do you think about this?

Given that few caches bloat at once, it's effect is not so
different from the current design.

As you commented here, guc variable syscache_memory_target and
syscache_prune_min_age are used for both syscache and relcache (HTAB), right?

Right, just not to add knobs for unclear reasons. Since ...

Do syscache and relcache have the similar amount of memory usage?

They may be different but would make not so much in the case of
cache bloat.

If not, I'm thinking that introducing separate guc variable would be fine.
So as syscache_prune_min_age.

I implemented that so that it is easily replaceable in case, but
I'm not sure separating them makes significant difference..

Thanks for the opinion, I'll put consideration on this more.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

Hi, thank you for the explanation.

From: Kyotaro HORIGUCHI [mailto:horiguchi.kyotaro@lab.ntt.co.jp]

Can I confirm about catcache pruning?
syscache_memory_target is the max figure per CatCache.
(Any CatCache has the same max value.) So the total max size of
catalog caches is estimated around or slightly more than # of SysCache
array times syscache_memory_target.

Right.

If correct, I'm thinking writing down the above estimation to the
document would help db administrators with estimation of memory usage.
Current description might lead misunderstanding that
syscache_memory_target is the total size of catalog cache in my impression.

Honestly I'm not sure that is the right design. Howerver, I don't think providing such
formula to users helps users, since they don't know exactly how many CatCaches and
brothres live in their server and it is a soft limit, and finally only few or just one catalogs
can reach the limit.

Yeah, I agree with that kind of formula is not suited for the document.
But if users don't know how many catcaches and brothers is used at postgres,
then how about changing syscache_memory_target as total soft limit of catcache,
rather than size limit of individual catcache. Internally syscache_memory_target can
be divided by the number of Syscache and does its work. The total amount would be
easier to understand for users who don't know the detailed contents of catalog caches.

Or if user can tell how many/what kind of catcaches exists, for instance by using
the system view you provided in the previous email, the current design looks good to me.

The current design based on the assumption that we would have only one
extremely-growable cache in one use case.

Related to the above I just thought changing sysycache_memory_target
per CatCache would make memory usage more efficient.

We could easily have per-cache settings in CatCache, but how do we provide the knobs
for them? I can guess only too much solutions for that.

Agreed.

Though I haven't checked if there's a case that each system catalog
cache memory usage varies largely, pg_class cache might need more memory than

others and others might need less.

But it would be difficult for users to check each CatCache memory
usage and tune it because right now postgresql hasn't provided a handy way to

check them.

I supposed that this is used without such a means. Someone suffers syscache bloat
just can set this GUC to avoid the bloat. End.

Yeah, I took the purpose wrong.

Apart from that, in the current patch, syscache_memory_target is not exact at all in
the first place to avoid overhead to count the correct size. The major difference comes
from the size of cache tuple itself. But I came to think it is too much to omit.

As a *PoC*, in the attached patch (which applies to current master), size of CTups are
counted as the catcache size.

It also provides pg_catcache_size system view just to give a rough idea of how such
view looks. I'll consider more on that but do you have any opinion on this?

=# select relid::regclass, indid::regclass, size from pg_syscache_sizes order by size
desc;
relid | indid | size
-------------------------+-------------------------------------------+--
-------------------------+-------------------------------------------+--
-------------------------+-------------------------------------------+--
-------------------------+-------------------------------------------+--
pg_class | pg_class_oid_index | 131072
pg_class | pg_class_relname_nsp_index | 131072
pg_cast | pg_cast_source_target_index | 5504
pg_operator | pg_operator_oprname_l_r_n_index | 4096
pg_statistic | pg_statistic_relid_att_inh_index | 2048
pg_proc | pg_proc_proname_args_nsp_index | 2048
..

Great! I like this view.
One of the extreme idea would be adding all the members printed by CatCachePrintStats(),
which is only enabled with -DCATCACHE_STATS at this moment.
All of the members seems too much for customers who tries to change the cache limit size
But it may be some of the members are useful because for example cc_hits would indicate that current
cache limit size is too small.

Another option is that users only specify the total memory target size
and postgres dynamically change each CatCache memory target size according to a

certain metric.

(, which still seems difficult and expensive to develop per benefit)
What do you think about this?

Given that few caches bloat at once, it's effect is not so different from the current
design.

Yes agreed.

As you commented here, guc variable syscache_memory_target and
syscache_prune_min_age are used for both syscache and relcache (HTAB), right?

Right, just not to add knobs for unclear reasons. Since ...

Do syscache and relcache have the similar amount of memory usage?

They may be different but would make not so much in the case of cache bloat.

If not, I'm thinking that introducing separate guc variable would be fine.
So as syscache_prune_min_age.

I implemented that so that it is easily replaceable in case, but I'm not sure separating
them makes significant difference..

Maybe I was overthinking mixing my development.

Regards,
Takeshi Ideriha

Hello. Thank you for the comment.

At Thu, 4 Oct 2018 04:27:04 +0000, "Ideriha, Takeshi" <ideriha.takeshi@jp.fujitsu.com> wrote in <4E72940DA2BF16479384A86D54D0988A6F1BCB6F@G01JPEXMBKW04>

As a *PoC*, in the attached patch (which applies to current master), size of CTups are
counted as the catcache size.

It also provides pg_catcache_size system view just to give a rough idea of how such
view looks. I'll consider more on that but do you have any opinion on this?

...

Great! I like this view.
One of the extreme idea would be adding all the members printed by CatCachePrintStats(),
which is only enabled with -DCATCACHE_STATS at this moment.
All of the members seems too much for customers who tries to change the cache limit size
But it may be some of the members are useful because for example cc_hits would indicate that current
cache limit size is too small.

The attached introduces four features below. (But the features on
relcache and plancache are omitted).

1. syscache stats collector (in 0002)

Records syscache status consists of the same columns above and
"ageclass" information. We could somehow triggering a stats
report with signal but we don't want take/send/write the
statistics in signal handler. Instead, it is turned on by setting
track_syscache_usage_interval to a positive number in
milliseconds.

2. pg_stat_syscache view. (in 0002)

This view shows catcache statistics. Statistics is taken only on
the backends where syscache tracking is active.

pid | application_name | relname | cache_name | size | ageclass | nentries
------+------------------+----------------+-----------------------------------+----------+-------------------------+---------------------------
9984 | psql | pg_statistic | pg_statistic_relid_att_inh_index | 12676096 | {30,60,600,1200,1800,0} | {17660,17310,55870,0,0,0}

Age class is the basis of catcache truncation mechanism and shows
the distribution based on elapsed time since last access. As I
didn't came up an appropriate way, it is represented as two
arrays. Ageclass stores maximum age for each class in
seconds. Nentries holds entry numbers correnponding to the same
element in ageclass. In the above example,

age class : # of entries in the cache
up to 30s : 17660
up to 60s : 17310
up to 600s : 55870
up to 1200s : 0
up to 1800s : 0
more longer : 0

The ageclass is {0, 0.05, 0.1, 1, 2, 3}th multiples of
cache_prune_min_age on the backend.

3. non-transactional GUC setting (in 0003)

It allows setting GUC variable set by the action
GUC_ACTION_NONXACT(the name requires condieration) survive beyond
rollback. It is required by remote guc setting to work
sanely. Without the feature a remote-set value within a trasction
will disappear involved in rollback. The only local interface for
the NONXACT action is set_config(name, value, is_local=false,
is_nonxact = true). pg_set_backend_guc() below works on this
feature.

4. pg_set_backend_guc() function.

Of course syscache statistics recording consumes significant
amount of time so it cannot be turned on usually. On the other
hand since this feature is turned on by GUC, it is needed to grab
the active client connection to turn on/off the feature(but we
cannot). Instead, I provided a means to change GUC variables in
another backend.

pg_set_backend_guc(pid, name, value) sets the GUC variable "name"
on the backend "pid" to "value".

With the above tools, we can inspect catcache statistics of
seemingly bloated process.

A. Find a bloated process pid using ps or something.

B. Turn on syscache stats on the process.
=# select pg_set_backend_guc(9984, 'track_syscache_usage_interval', '10000');

C. Examine the statitics.

=# select pid, relname, cache_name, size from pg_stat_syscache order by size desc limit 3;
pid | relname | cache_name | size
------+--------------+----------------------------------+----------
9984 | pg_statistic | pg_statistic_relid_att_inh_index | 32154112
9984 | pg_cast | pg_cast_source_target_index | 4096
9984 | pg_operator | pg_operator_oprname_l_r_n_index | 4096

=# select * from pg_stat_syscache where cache_name = 'pg_statistic_relid_att_inh_index'::regclass;
-[ RECORD 1 ]---------------------------------
pid | 9984
relname | pg_statistic
cache_name | pg_statistic_relid_att_inh_index
size | 11026176
ntuples | 77950
searches | 77950
hits | 0
neg_hits | 0
ageclass | {30,60,600,1200,1800,0}
nentries | {17630,16950,43370,0,0,0}
last_update | 2018-10-17 15:58:19.738164+09

Another option is that users only specify the total memory target size
and postgres dynamically change each CatCache memory target size according to a

certain metric.

(, which still seems difficult and expensive to develop per benefit)
What do you think about this?

Given that few caches bloat at once, it's effect is not so different from the current
design.

Yes agreed.

As you commented here, guc variable syscache_memory_target and
syscache_prune_min_age are used for both syscache and relcache (HTAB), right?

Right, just not to add knobs for unclear reasons. Since ...

Do syscache and relcache have the similar amount of memory usage?

They may be different but would make not so much in the case of cache bloat.

If not, I'm thinking that introducing separate guc variable would be fine.
So as syscache_prune_min_age.

I implemented that so that it is easily replaceable in case, but I'm not sure separating
them makes significant difference..

Maybe I was overthinking mixing my development.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

Hello, thank you for updating the patch.

From: Kyotaro HORIGUCHI [mailto:horiguchi.kyotaro@lab.ntt.co.jp]
At Thu, 4 Oct 2018 04:27:04 +0000, "Ideriha, Takeshi"
<ideriha.takeshi@jp.fujitsu.com> wrote in
<4E72940DA2BF16479384A86D54D0988A6F1BCB6F@G01JPEXMBKW04>

As a *PoC*, in the attached patch (which applies to current master),
size of CTups are counted as the catcache size.

It also provides pg_catcache_size system view just to give a rough
idea of how such view looks. I'll consider more on that but do you have any opinion

on this?

...

Great! I like this view.
One of the extreme idea would be adding all the members printed by
CatCachePrintStats(), which is only enabled with -DCATCACHE_STATS at this

moment.

All of the members seems too much for customers who tries to change
the cache limit size But it may be some of the members are useful
because for example cc_hits would indicate that current cache limit size is too small.

The attached introduces four features below. (But the features on relcache and
plancache are omitted).

I haven't looked into the code but I'm going to do it later.

Right now It seems to me that focusing on catalog cache invalidation and its stats a quick route
to commit this feature.

1. syscache stats collector (in 0002)

Records syscache status consists of the same columns above and "ageclass"
information. We could somehow triggering a stats report with signal but we don't want
take/send/write the statistics in signal handler. Instead, it is turned on by setting
track_syscache_usage_interval to a positive number in milliseconds.

I agreed. Agecalss is important to tweak the prune_min_age.
Collecting stats is heavy at every stats change

2. pg_stat_syscache view. (in 0002)

This view shows catcache statistics. Statistics is taken only on the backends where
syscache tracking is active.

pid | application_name | relname | cache_name

| size | ageclass | nentries

------+------------------+----------------+-----------------------------------+----------
+-------------------------+---------------------------

9984 | psql | pg_statistic | pg_statistic_relid_att_inh_index |

12676096 | {30,60,600,1200,1800,0} | {17660,17310,55870,0,0,0}

Age class is the basis of catcache truncation mechanism and shows the distribution
based on elapsed time since last access. As I didn't came up an appropriate way, it is
represented as two arrays. Ageclass stores maximum age for each class in seconds.
Nentries holds entry numbers correnponding to the same element in ageclass. In the
above example,

age class : # of entries in the cache
up to 30s : 17660
up to 60s : 17310
up to 600s : 55870
up to 1200s : 0
up to 1800s : 0
more longer : 0

The ageclass is {0, 0.05, 0.1, 1, 2, 3}th multiples of cache_prune_min_age on the
backend.

I just thought that the pair of ageclass and nentries can be represented as
json or multi-dimensional array but in virtual they are all same and can be converted each other
using some functions. So I'm not sure which representaion is better one.

3. non-transactional GUC setting (in 0003)

It allows setting GUC variable set by the action GUC_ACTION_NONXACT(the name
requires condieration) survive beyond rollback. It is required by remote guc setting to
work sanely. Without the feature a remote-set value within a trasction will disappear
involved in rollback. The only local interface for the NONXACT action is
set_config(name, value, is_local=false, is_nonxact = true). pg_set_backend_guc()
below works on this feature.

TBH, I'm not familiar with around this and I may be missing something.
In order to change the other backend's GUC value,
is ignoring transactional behevior always necessary? When transaction of GUC setting
is failed and rollbacked, if the error message is supposeed to be reported I thought
just trying the transaction again is enough.

4. pg_set_backend_guc() function.

Of course syscache statistics recording consumes significant amount of time so it
cannot be turned on usually. On the other hand since this feature is turned on by GUC,
it is needed to grab the active client connection to turn on/off the feature(but we
cannot). Instead, I provided a means to change GUC variables in another backend.

pg_set_backend_guc(pid, name, value) sets the GUC variable "name"
on the backend "pid" to "value".

With the above tools, we can inspect catcache statistics of seemingly bloated process.

A. Find a bloated process pid using ps or something.

B. Turn on syscache stats on the process.
=# select pg_set_backend_guc(9984, 'track_syscache_usage_interval', '10000');

C. Examine the statitics.

=# select pid, relname, cache_name, size from pg_stat_syscache order by size desc
limit 3;
pid | relname | cache_name | size
------+--------------+----------------------------------+----------
9984 | pg_statistic | pg_statistic_relid_att_inh_index | 32154112
9984 | pg_cast | pg_cast_source_target_index | 4096
9984 | pg_operator | pg_operator_oprname_l_r_n_index | 4096

=# select * from pg_stat_syscache where cache_name =
'pg_statistic_relid_att_inh_index'::regclass;
-[ RECORD 1 ]---------------------------------
pid | 9984
relname | pg_statistic
cache_name | pg_statistic_relid_att_inh_index
size | 11026176
ntuples | 77950
searches | 77950
hits | 0
neg_hits | 0
ageclass | {30,60,600,1200,1800,0}
nentries | {17630,16950,43370,0,0,0}
last_update | 2018-10-17 15:58:19.738164+09

The output of this view seems good to me.

I can imagine this use case. Does the use case of setting GUC locally never happen?
I mean can the setting be locally changed?

Regards,
Takeshi Ideriha

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

Thank you for reviewing.

At Thu, 15 Nov 2018 11:02:10 +0000, "Ideriha, Takeshi" <ideriha.takeshi@jp.fujitsu.com> wrote in <4E72940DA2BF16479384A86D54D0988A6F1F4165@G01JPEXMBKW04>

Hello, thank you for updating the patch.

From: Kyotaro HORIGUCHI [mailto:horiguchi.kyotaro@lab.ntt.co.jp]
At Thu, 4 Oct 2018 04:27:04 +0000, "Ideriha, Takeshi"
<ideriha.takeshi@jp.fujitsu.com> wrote in
<4E72940DA2BF16479384A86D54D0988A6F1BCB6F@G01JPEXMBKW04>

As a *PoC*, in the attached patch (which applies to current master),
size of CTups are counted as the catcache size.

It also provides pg_catcache_size system view just to give a rough
idea of how such view looks. I'll consider more on that but do you have any opinion

on this?

...

Great! I like this view.
One of the extreme idea would be adding all the members printed by
CatCachePrintStats(), which is only enabled with -DCATCACHE_STATS at this

moment.

All of the members seems too much for customers who tries to change
the cache limit size But it may be some of the members are useful
because for example cc_hits would indicate that current cache limit size is too small.

The attached introduces four features below. (But the features on relcache and
plancache are omitted).

I haven't looked into the code but I'm going to do it later.

Right now It seems to me that focusing on catalog cache invalidation and its stats a quick route
to commit this feature.

1. syscache stats collector (in 0002)

Records syscache status consists of the same columns above and "ageclass"
information. We could somehow triggering a stats report with signal but we don't want
take/send/write the statistics in signal handler. Instead, it is turned on by setting
track_syscache_usage_interval to a positive number in milliseconds.

I agreed. Agecalss is important to tweak the prune_min_age.
Collecting stats is heavy at every stats change

2. pg_stat_syscache view. (in 0002)

This view shows catcache statistics. Statistics is taken only on the backends where
syscache tracking is active.

pid | application_name | relname | cache_name

| size | ageclass | nentries

------+------------------+----------------+-----------------------------------+----------
+-------------------------+---------------------------

9984 | psql | pg_statistic | pg_statistic_relid_att_inh_index |

12676096 | {30,60,600,1200,1800,0} | {17660,17310,55870,0,0,0}

Age class is the basis of catcache truncation mechanism and shows the distribution
based on elapsed time since last access. As I didn't came up an appropriate way, it is
represented as two arrays. Ageclass stores maximum age for each class in seconds.
Nentries holds entry numbers correnponding to the same element in ageclass. In the
above example,

age class : # of entries in the cache
up to 30s : 17660
up to 60s : 17310
up to 600s : 55870
up to 1200s : 0
up to 1800s : 0
more longer : 0

The ageclass is {0, 0.05, 0.1, 1, 2, 3}th multiples of cache_prune_min_age on the
backend.

I just thought that the pair of ageclass and nentries can be represented as
json or multi-dimensional array but in virtual they are all same and can be converted each other
using some functions. So I'm not sure which representaion is better one.

Multi dimentional array in any style sounds reasonable. Maybe
array is preferable in system views as it is a basic type than
JSON. In the attached, it looks like the follows:

=# select * from pg_stat_syscache where ntuples > 100;
-[ RECORD 1 ]--------------------------------------------------
pid | 1817
relname | pg_class
cache_name | pg_class_oid_index
size | 2048
ntuples | 189
searches | 1620
hits | 1431
neg_hits | 0
ageclass | {{30,189},{60,0},{600,0},{1200,0},{1800,0},{0,0}}
last_update | 2018-11-27 19:22:00.74026+09

3. non-transactional GUC setting (in 0003)

It allows setting GUC variable set by the action GUC_ACTION_NONXACT(the name
requires condieration) survive beyond rollback. It is required by remote guc setting to
work sanely. Without the feature a remote-set value within a trasction will disappear
involved in rollback. The only local interface for the NONXACT action is
set_config(name, value, is_local=false, is_nonxact = true). pg_set_backend_guc()
below works on this feature.

TBH, I'm not familiar with around this and I may be missing something.
In order to change the other backend's GUC value,
is ignoring transactional behevior always necessary? When transaction of GUC setting
is failed and rollbacked, if the error message is supposeed to be reported I thought
just trying the transaction again is enough.

The target backend can be running frequent transaction. The
invoker backend cannot know whether the remote change happend
during a transaction and whether the transaction if any is
committed or aborted, no error message sent to invoker backend.
We could wait for the end of a trasaction but that doesn't work
with long transactions.

Maybe we don't need the feature in GUC system but adding another
similar feature doesn't seem reasonable. This would be useful for
some other tracking features.

4. pg_set_backend_guc() function.

Of course syscache statistics recording consumes significant amount of time so it
cannot be turned on usually. On the other hand since this feature is turned on by GUC,
it is needed to grab the active client connection to turn on/off the feature(but we
cannot). Instead, I provided a means to change GUC variables in another backend.

pg_set_backend_guc(pid, name, value) sets the GUC variable "name"
on the backend "pid" to "value".

With the above tools, we can inspect catcache statistics of seemingly bloated process.

A. Find a bloated process pid using ps or something.

B. Turn on syscache stats on the process.
=# select pg_set_backend_guc(9984, 'track_syscache_usage_interval', '10000');

C. Examine the statitics.

=# select pid, relname, cache_name, size from pg_stat_syscache order by size desc
limit 3;
pid | relname | cache_name | size
------+--------------+----------------------------------+----------
9984 | pg_statistic | pg_statistic_relid_att_inh_index | 32154112
9984 | pg_cast | pg_cast_source_target_index | 4096
9984 | pg_operator | pg_operator_oprname_l_r_n_index | 4096

=# select * from pg_stat_syscache where cache_name =
'pg_statistic_relid_att_inh_index'::regclass;
-[ RECORD 1 ]---------------------------------
pid | 9984
relname | pg_statistic
cache_name | pg_statistic_relid_att_inh_index
size | 11026176
ntuples | 77950
searches | 77950
hits | 0
neg_hits | 0
ageclass | {30,60,600,1200,1800,0}
nentries | {17630,16950,43370,0,0,0}
last_update | 2018-10-17 15:58:19.738164+09

The output of this view seems good to me.

I can imagine this use case. Does the use case of setting GUC locally never happen?
I mean can the setting be locally changed?

Syscahe grows through a life of a backend/session. No other
client cannot connect to it at the same time. So the variable
must be set at the start of a backend using ALTER USER/DATABASE,
or the client itself is obliged to deliberitely turn on the
feature at a convenient time. I suppose that in most use cases
one wants to turn on this feature after he sees another session
is eating memory more and more.

The attached is the rebased version that has multidimentional
ageclass.

Thank you for the comments in the next mail but sorry that I'll
address them later.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

On Tue, Nov 27, 2018 at 11:40 AM Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote:

The attached is the rebased version that has multidimentional
ageclass.

Thank you,

Just for the information, cfbot complains about this patch because:

pgstatfuncs.c: In function ‘pgstat_get_syscache_stats’:
pgstatfuncs.c:1973:8: error: ignoring return value of ‘fread’,
declared with attribute warn_unused_result [-Werror=unused-result]
fread(&cacheid, sizeof(int), 1, fpin);
^
pgstatfuncs.c:1974:8: error: ignoring return value of ‘fread’,
declared with attribute warn_unused_result [-Werror=unused-result]
fread(&last_update, sizeof(TimestampTz), 1, fpin);
^

I'm moving it to the next CF as "Waiting on author", since as far as I
understood you want to address more commentaries from the reviewer.

Hello,

Sorry for delay.
The detailed comments for the source code will be provided later.

I just thought that the pair of ageclass and nentries can be
represented as json or multi-dimensional array but in virtual they are
all same and can be converted each other using some functions. So I'm not sure

which representaion is better one.

Multi dimentional array in any style sounds reasonable. Maybe array is preferable in
system views as it is a basic type than JSON. In the attached, it looks like the follows:

=# select * from pg_stat_syscache where ntuples > 100; -[ RECORD
1 ]--------------------------------------------------
pid | 1817
relname | pg_class
cache_name | pg_class_oid_index
size | 2048
ntuples | 189
searches | 1620
hits | 1431
neg_hits | 0
ageclass | {{30,189},{60,0},{600,0},{1200,0},{1800,0},{0,0}}
last_update | 2018-11-27 19:22:00.74026+09

Thanks, cool. That seems better to me.

3. non-transactional GUC setting (in 0003)

It allows setting GUC variable set by the action
GUC_ACTION_NONXACT(the name requires condieration) survive beyond
rollback. It is required by remote guc setting to work sanely.
Without the feature a remote-set value within a trasction will
disappear involved in rollback. The only local interface for the
NONXACT action is set_config(name, value, is_local=false, is_nonxact = true).

pg_set_backend_guc() below works on this feature.

TBH, I'm not familiar with around this and I may be missing something.
In order to change the other backend's GUC value, is ignoring
transactional behevior always necessary? When transaction of GUC
setting is failed and rollbacked, if the error message is supposeed to
be reported I thought just trying the transaction again is enough.

The target backend can be running frequent transaction. The invoker backend cannot
know whether the remote change happend during a transaction and whether the
transaction if any is committed or aborted, no error message sent to invoker backend.
We could wait for the end of a trasaction but that doesn't work with long transactions.

Maybe we don't need the feature in GUC system but adding another similar feature
doesn't seem reasonable. This would be useful for some other tracking features.

Thank you for the clarification.

4. pg_set_backend_guc() function.

Of course syscache statistics recording consumes significant amount
of time so it cannot be turned on usually. On the other hand since
this feature is turned on by GUC, it is needed to grab the active
client connection to turn on/off the feature(but we cannot). Instead, I provided a

means to change GUC variables in another backend.

pg_set_backend_guc(pid, name, value) sets the GUC variable "name"
on the backend "pid" to "value".

With the above tools, we can inspect catcache statistics of seemingly bloated

process.

A. Find a bloated process pid using ps or something.

B. Turn on syscache stats on the process.
=# select pg_set_backend_guc(9984, 'track_syscache_usage_interval',
'10000');

C. Examine the statitics.

=# select pid, relname, cache_name, size from pg_stat_syscache order
by size desc limit 3;
pid | relname | cache_name | size
------+--------------+----------------------------------+----------
9984 | pg_statistic | pg_statistic_relid_att_inh_index | 32154112
9984 | pg_cast | pg_cast_source_target_index | 4096
9984 | pg_operator | pg_operator_oprname_l_r_n_index | 4096

=# select * from pg_stat_syscache where cache_name =
'pg_statistic_relid_att_inh_index'::regclass;
-[ RECORD 1 ]---------------------------------
pid | 9984
relname | pg_statistic
cache_name | pg_statistic_relid_att_inh_index
size | 11026176
ntuples | 77950
searches | 77950
hits | 0
neg_hits | 0
ageclass | {30,60,600,1200,1800,0}
nentries | {17630,16950,43370,0,0,0}
last_update | 2018-10-17 15:58:19.738164+09

The output of this view seems good to me.

I can imagine this use case. Does the use case of setting GUC locally never happen?
I mean can the setting be locally changed?

Syscahe grows through a life of a backend/session. No other client cannot connect to
it at the same time. So the variable must be set at the start of a backend using ALTER
USER/DATABASE, or the client itself is obliged to deliberitely turn on the feature at a
convenient time. I suppose that in most use cases one wants to turn on this feature
after he sees another session is eating memory more and more.

The attached is the rebased version that has multidimentional ageclass.

Thank you! That's convenient.
How about splitting this non-xact guc and remote guc setting feature as another commit fest entry?
I'm planning to review 001 and 002 patch in more detail and hopefully turn it to 'ready for committer'
and review remote guc feature later.
Related to the feature division why have you discarded pruning of relcache and plancache?
Personally I want relcache one as well as catcache because regarding memory bloat there is some correlation between them.

Regards,
Takeshi Ideriha

From: Ideriha, Takeshi [mailto:ideriha.takeshi@jp.fujitsu.com]
The detailed comments for the source code will be provided later.

Hi,
I'm adding some comments to 0001 and 0002 one.

[0001 patch]

+                       /*                                                                                                                                                  
+                        * Calculate the duration from the time of the last access to the                                                                                   
+                        * "current" time. Since catcacheclock is not advanced within a                                                                                     
+                        * transaction, the entries that are accessed within the current                                                                                    
+                        * transaction won't be pruned.                                                                                                                     
+                        */                                                                                                                                                 
+                       TimestampDifference(ct->lastaccess, catcacheclock, &entry_age, &us); 

+                       /*                                                                                        
+                        * Try to remove entries older than cache_prune_min_age seconds.                          

+ if (entry_age > cache_prune_min_age)

Can you change this comparison between entry_age and cache_prune_min_age
to "entry_age >= cache_prune_min_age"?
That is, I want the feature that entries that are accessed even within the transaction
is pruned in case of cache_prune_min_age = 0
I can hit upon some of my customer who want to always keep memory usage below certain limit as strictly as possible.
This kind of strict user would set cache_prune_min_age to 0 and would not want to exceed the memory target even
if within a transaction.

As I put miscellaneous comments about 0001 patch in some previous email, so please take a look at it.

[0002 patch]
I haven't looked into every detail but here are some comments.

Maybe you would also need to add some sentences to this page:
https://www.postgresql.org/docs/current/monitoring-stats.html

+pgstat_get_syscache_stats(PG_FUNCTION_ARGS)
Function name like 'pg_stat_XXX' would match surrounding code.

When applying patch I found trailing whitespace warning:
../patch/horiguchi_cache/v6_stats/0002-Syscache-usage-tracking-feature.patch:157: trailing whitespace.

../patch/horiguchi_cache/v6_stats/0002-Syscache-usage-tracking-feature.patch:256: trailing whitespace.

../patch/horiguchi_cache/v6_stats/0002-Syscache-usage-tracking-feature.patch:301: trailing whitespace.

../patch/horiguchi_cache/v6_stats/0002-Syscache-usage-tracking-feature.patch:483: trailing whitespace.

../patch/horiguchi_cache/v6_stats/0002-Syscache-usage-tracking-feature.patch:539: trailing whitespace.

Regards,
Takeshi Ideriha

I'm really disappointed by the direction this thread is going in.
The latest patches add an enormous amount of mechanism, and user-visible
complexity, to do something that we learned was a bad idea decades ago.
Putting a limit on the size of the syscaches doesn't accomplish anything
except to add cycles if your cache working set is below the limit, or
make performance fall off a cliff if it's above the limit. I don't think
there's any reason to believe that making it more complicated will avoid
that problem.

What does seem promising is something similar to Horiguchi-san's
original patches all the way back at

/messages/by-id/20161219.201505.11562604.horiguchi.kyotaro@lab.ntt.co.jp

That is, identify usage patterns in which we tend to fill the caches with
provably no-longer-useful entries, and improve those particular cases.
Horiguchi-san identified one such case in that message: negative entries
in the STATRELATTINH cache, caused by the planner probing for stats that
aren't there, and then not cleared when the relevant table gets dropped
(since, by definition, they don't match any pg_statistic entry that gets
deleted). We saw another recent report of the same problem at

/messages/by-id/2114009259.1866365.1544469996900@mail.yahoo.com

so I'd been thinking about ways to fix that case in particular. I came
up with a fix that I think is simpler and a bit more efficient than
what Horiguchi-san proposed originally: rather than trying to reverse-
engineer what to do in low-level cache callbacks, let's have the catalog
manipulation code explicitly send out invalidation commands when the
relevant situations arise. In the attached, heap.c's RemoveStatistics
sends out an sinval message commanding deletion of negative STATRELATTINH
entries that match the OID of the table being deleted. We could use the
same infrastructure to clean out dead RELNAMENSP entries after a schema
deletion, as per Horiguchi-san's second original suggestion; although
I haven't done so here because I'm not really convinced that that's got
an attractive cost-benefit ratio. (In both my patch and Horiguchi-san's,
we have to traverse all entries in the affected cache, so sending out one
of these messages is potentially not real cheap.)

To do this we need to adjust the representation of sinval messages so
that we can have two different kinds of messages that include a cache ID.
Fortunately, because there's padding space available, that's not costly.
0001 below is a simple refactoring patch that converts the message type
ID into a plain enum field that's separate from the cache ID if any.
(I'm inclined to apply this whether or not people like 0002: it makes
the code clearer, more maintainable, and probably a shade faster thanks
to replacing an if-then-else chain with a switch.) Then 0002 adds the
feature of an sinval message type saying "delete negative entries in
cache X that have OID Y in key column Z", and teaches RemoveStatistics
to use that.

Thoughts?

regards, tom lane

From: Tom Lane [mailto:tgl@sss.pgh.pa.us]

I'm really disappointed by the direction this thread is going in.
The latest patches add an enormous amount of mechanism, and user-visible
complexity, to do something that we learned was a bad idea decades ago.
Putting a limit on the size of the syscaches doesn't accomplish anything
except to add cycles if your cache working set is below the limit, or make
performance fall off a cliff if it's above the limit. I don't think there's
any reason to believe that making it more complicated will avoid that
problem.

What does seem promising is something similar to Horiguchi-san's original
patches all the way back at

/messages/by-id/20161219.201505.11562604.horiguc
hi.kyotaro@lab.ntt.co.jp

so I'd been thinking about ways to fix that case in particular.

You're suggesting to go back to the original issue (bloat by negative cache entries) and give simpler solution to it once, aren't you? That may be the way to go.

But the syscache/relcache bloat still remains a problem, when there are many live tables and application connections. Would you agree to solve this in some way? I thought Horiguchi-san's latest patches would solve this and the negative entries. Can we consider that his patch and yours are orthogonal, i.e., we can pursue Horiguchi-san's patch after yours is committed?

(As you said, some parts of Horiguchi-san's patches may be made simpler. For example, the ability to change another session's GUC variable can be discussed in a separate thread.)

I think we need some limit to the size of the relcache, syscache, and plancache. Oracle and MySQL both have it, using LRU to evict less frequently used entries. You seem to be concerned about the LRU management based on your experience, but would it really cost so much as long as each postgres process can change the LRU list without coordination with other backends now? Could you share your experience?

FYI, Oracle provides one parameter, shared_pool_size, that determine the size of a memory area that contains SQL plans and various dictionary objects. Oracle decides how to divide the area among constituents. So it could be possible that one component (e.g. table/index metadata) is short of space, and another (e.g. SQL plans) has free space. Oracle provides a system view to see the free space and hit/miss of each component. If one component suffers from memory shortage, the user increases shared_pool_size. This is similar to what Horiguchi-san is proposing.

MySQL enables fine-tuning of each component. It provides the size parameters for six memory partitions of the dictionary object cache, and the usage statistics of those partitions through the Performance Schema.

tablespace definition cache
schema definition cache
table definition cache
stored program definition cache
character set definition cache
collation definition cache

I wonder whether we can group existing relcache/syscache entries like this.

[MySQL]
14.4 Dictionary Object Cache
https://dev.mysql.com/doc/refman/8.0/en/data-dictionary-object-cache.html
--------------------------------------------------
The dictionary object cache is a shared global cache that stores previously accessed data dictionary objects in memory to enable object reuse and minimize disk I/O. Similar to other cache mechanisms used by MySQL, the dictionary object cache uses an LRU-based eviction strategy to evict least recently used objects from memory.

The dictionary object cache comprises cache partitions that store different object types. Some cache partition size limits are configurable, whereas others are hardcoded.
--------------------------------------------------

8.12.3.1 How MySQL Uses Memory
https://dev.mysql.com/doc/refman/8.0/en/memory-use.html
--------------------------------------------------
table_open_cache
MySQL requires memory and descriptors for the table cache.

table_definition_cache
For InnoDB, table_definition_cache acts as a soft limit for the number of open table instances in the InnoDB data dictionary cache. If the number of open table instances exceeds the table_definition_cache setting, the LRU mechanism begins to mark table instances for eviction and eventually removes them from the data dictionary cache. The limit helps address situations in which significant amounts of memory would be used to cache rarely used table instances until the next server restart.
--------------------------------------------------

Regards
Takayuki Tsunakawa

"Tsunakawa, Takayuki" <tsunakawa.takay@jp.fujitsu.com> writes:

But the syscache/relcache bloat still remains a problem, when there are many live tables and application connections. Would you agree to solve this in some way? I thought Horiguchi-san's latest patches would solve this and the negative entries. Can we consider that his patch and yours are orthogonal, i.e., we can pursue Horiguchi-san's patch after yours is committed?

Certainly, what I've done here doesn't preclude adding some wider solution to
the issue of extremely large catcaches. I think it takes the pressure off
for one rather narrow problem case, and the mechanism could be used to fix
other ones. But if you've got an application that just plain accesses a
huge number of objects, this isn't going to make your life better.

(As you said, some parts of Horiguchi-san's patches may be made simpler. For example, the ability to change another session's GUC variable can be discussed in a separate thread.)

Yeah, that idea seems just bad from here ...

I think we need some limit to the size of the relcache, syscache, and plancache. Oracle and MySQL both have it, using LRU to evict less frequently used entries. You seem to be concerned about the LRU management based on your experience, but would it really cost so much as long as each postgres process can change the LRU list without coordination with other backends now? Could you share your experience?

Well, we *had* an LRU mechanism for the catcaches way back when. We got
rid of it --- see commit 8b9bc234a --- because (a) maintaining the LRU
info was expensive and (b) performance fell off a cliff in scenarios where
the cache size limit was exceeded. You could probably find some more info
about that by scanning the mail list archives from around the time of that
commit, but I'm too lazy to do so right now.

That was a dozen years ago, and it's possible that machine performance
has moved so much since then that the problems are gone or mitigated.
In particular I'm sure that any limit we would want to impose today will
be far more than the 5000-entries-across-all-caches limit that was in use
back then. But I'm not convinced that a workload that would create 100K
cache entries in the first place wouldn't have severe problems if you
tried to constrain it to use only 80K entries. I fear it's just wishful
thinking to imagine that the behavior of a larger cache won't be just
like a smaller one. Also, IIRC some of the problem with the LRU code
was that it resulted in lots of touches of unrelated data, leading to
CPU cache miss problems. It's hard to see how that doesn't get even
worse with a bigger cache.

As far as the relcache goes, we've never had a limit on that, but there
are enough routine causes of relcache flushes --- autovacuum for instance
--- that I'm not really convinced relcache bloat can be a big problem in
production.

The plancache has never had a limit either, which is a design choice that
was strongly influenced by our experience with catcaches. Again, I'm
concerned about the costs of adding a management layer, and the likelihood
that cache flushes will simply remove entries we'll soon have to rebuild.

FYI, Oracle provides one parameter, shared_pool_size, that determine the
size of a memory area that contains SQL plans and various dictionary
objects. Oracle decides how to divide the area among constituents. So
it could be possible that one component (e.g. table/index metadata) is
short of space, and another (e.g. SQL plans) has free space. Oracle
provides a system view to see the free space and hit/miss of each
component. If one component suffers from memory shortage, the user
increases shared_pool_size. This is similar to what Horiguchi-san is
proposing.

Oracle seldom impresses me as having designs we ought to follow.
They have a well-earned reputation for requiring a lot of expertise to
operate, which is not the direction this project should be going in.
In particular, I don't want to "solve" cache size issues by exposing
a bunch of knobs that most users won't know how to twiddle.

regards, tom lane

Hi,

On 2019-01-15 13:32:36 -0500, Tom Lane wrote:

Well, we *had* an LRU mechanism for the catcaches way back when. We got
rid of it --- see commit 8b9bc234a --- because (a) maintaining the LRU
info was expensive and (b) performance fell off a cliff in scenarios where
the cache size limit was exceeded. You could probably find some more info
about that by scanning the mail list archives from around the time of that
commit, but I'm too lazy to do so right now.

That was a dozen years ago, and it's possible that machine performance
has moved so much since then that the problems are gone or mitigated.
In particular I'm sure that any limit we would want to impose today will
be far more than the 5000-entries-across-all-caches limit that was in use
back then. But I'm not convinced that a workload that would create 100K
cache entries in the first place wouldn't have severe problems if you
tried to constrain it to use only 80K entries.

I think that'd be true if you the accesses were truly randomly
distributed - but that's not the case in the cases where I've seen huge
caches. It's usually workloads that have tons of functions, partitions,
... and a lot of them are not that frequently accessed, but because we
have no cache purging mechanism stay around for a long time. This is
often exascerbated by using a pooler to keep connections around for
longer (which you have to, to cope with other limits of PG).

As far as the relcache goes, we've never had a limit on that, but there
are enough routine causes of relcache flushes --- autovacuum for instance
--- that I'm not really convinced relcache bloat can be a big problem in
production.

It definitely is.

The plancache has never had a limit either, which is a design choice that
was strongly influenced by our experience with catcaches.

This sounds a lot of having learned lessons from one bad implementation
and using it far outside of that situation.

Greetings,

Andres Freund

On Tue, Jan 15, 2019 at 01:32:36PM -0500, Tom Lane wrote:

...

FYI, Oracle provides one parameter, shared_pool_size, that determine the
size of a memory area that contains SQL plans and various dictionary
objects. Oracle decides how to divide the area among constituents. So
it could be possible that one component (e.g. table/index metadata) is
short of space, and another (e.g. SQL plans) has free space. Oracle
provides a system view to see the free space and hit/miss of each
component. If one component suffers from memory shortage, the user
increases shared_pool_size. This is similar to what Horiguchi-san is
proposing.

Oracle seldom impresses me as having designs we ought to follow.
They have a well-earned reputation for requiring a lot of expertise to
operate, which is not the direction this project should be going in.
In particular, I don't want to "solve" cache size issues by exposing
a bunch of knobs that most users won't know how to twiddle.

regards, tom lane

+1

Regards,
Ken

From: Tom Lane [mailto:tgl@sss.pgh.pa.us]

Certainly, what I've done here doesn't preclude adding some wider solution
to
the issue of extremely large catcaches.

I'm relieved to hear that.

I think it takes the pressure off
for one rather narrow problem case, and the mechanism could be used to fix
other ones. But if you've got an application that just plain accesses a
huge number of objects, this isn't going to make your life better.

I understand you're trying to solve the problem caused by negative cache entries as soon as possible, because the user is really suffering from it. I feel sympathy with that attitude, because you seem to be always addressing issues that others are reluctant to take. That's one of the reasons I respect you.

Well, we *had* an LRU mechanism for the catcaches way back when. We got
rid of it --- see commit 8b9bc234a --- because (a) maintaining the LRU
info was expensive and (b) performance fell off a cliff in scenarios where
the cache size limit was exceeded. You could probably find some more info
about that by scanning the mail list archives from around the time of that
commit, but I'm too lazy to do so right now.

Oh, in 2006... I'll examine the patch and the discussion to see how the LRU management was done.

That was a dozen years ago, and it's possible that machine performance
has moved so much since then that the problems are gone or mitigated.

I really, really hope so. Even if we see some visible impact by the LRU management, I think that's the debt PostgreSQL had to pay for but doesn't now. Even the single-process MySQL, which doesn't suffer from cache bloat for many server processes, has the ability to limit the cache. And PostgreSQL has many parameters for various memory components such as shared_buffers, wal_buffers, work_mem, etc, so it would be reasonable to also have the limit for the catalog caches. That said, we can avoid the penalty and retain the current performance by disabling the limit (some_size_param = 0).

I think we'll evaluate the impact of LRU management by adding prev and next members to catcache and relcache structures, and putting the entry at the front (or back) of the LRU chain every time the entry is obtained. I think pgbench's select-only mode is enough for evaluation. I'd like to hear if any other workload is more appropriate to see the CPU cache effect.

In particular I'm sure that any limit we would want to impose today will
be far more than the 5000-entries-across-all-caches limit that was in use
back then. But I'm not convinced that a workload that would create 100K
cache entries in the first place wouldn't have severe problems if you
tried to constrain it to use only 80K entries. I fear it's just wishful
thinking to imagine that the behavior of a larger cache won't be just
like a smaller one. Also, IIRC some of the problem with the LRU code
was that it resulted in lots of touches of unrelated data, leading to
CPU cache miss problems. It's hard to see how that doesn't get even
worse with a bigger cache.

As far as the relcache goes, we've never had a limit on that, but there
are enough routine causes of relcache flushes --- autovacuum for instance
--- that I'm not really convinced relcache bloat can be a big problem in
production.

As Andres and Robert mentioned, we want to free less frequently used cache entries. Otherwise, we're now suffering from the bloat to TBs of memory. This is a real, not hypothetical issue...

The plancache has never had a limit either, which is a design choice that
was strongly influenced by our experience with catcaches. Again, I'm
concerned about the costs of adding a management layer, and the likelihood
that cache flushes will simply remove entries we'll soon have to rebuild.

Fortunately, we're not bothered with the plan cache. But I remember you said you were annoyed by PL/pgSQL's plan cache use at Salesforce. Were you able to overcome it somehow?

Oracle seldom impresses me as having designs we ought to follow.
They have a well-earned reputation for requiring a lot of expertise to
operate, which is not the direction this project should be going in.
In particular, I don't want to "solve" cache size issues by exposing
a bunch of knobs that most users won't know how to twiddle.

Oracle certainly seems to be difficult to use. But they seem to be studying other DBMSs to make it simpler to use. I'm sure they also have a lot we should learn, and the cache limit if one of them (although MySQL's per-cache tuning may be better.)

And having limits for various components would be the first step toward the autonomous database; tunable -> auto tuning -> autonomous

Regards
Takayuki Tsunakawa

On Sun, Jan 13, 2019 at 11:41 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Putting a limit on the size of the syscaches doesn't accomplish anything
except to add cycles if your cache working set is below the limit, or
make performance fall off a cliff if it's above the limit.

If you're running on a Turing machine, sure. But real machines have
finite memory, or at least all the ones I use do. Horiguchi-san is
right that this is a real, not theoretical problem. It is one of the
most frequent operational concerns that EnterpriseDB customers have.
I'm not against solving specific cases with more targeted fixes, but I
really believe we need something more. Andres mentioned one problem
case: connection poolers that eventually end up with a cache entry for
every object in the system. Another case is that of people who keep
idle connections open for long periods of time; those connections can
gobble up large amounts of memory even though they're not going to use
any of their cache entries any time soon.

The flaw in your thinking, as it seems to me, is that in your concern
for "the likelihood that cache flushes will simply remove entries
we'll soon have to rebuild," you're apparently unwilling to consider
the possibility of workloads where cache flushes will remove entries
we *won't* soon have to rebuild. Every time that issue gets raised,
you seem to blow it off as if it were not a thing that really happens.
I can't make sense of that position. Is it really so hard to imagine
a connection pooler that switches the same connection back and forth
between two applications with different working sets? Or a system
that keeps persistent connections open even when they are idle? Do
you really believe that a connection that has not accessed a cache
entry in 10 minutes still derives more benefit from that cache entry
than it would from freeing up some memory?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Thu, Jan 17, 2019 at 11:33:35AM -0500, Robert Haas wrote:

The flaw in your thinking, as it seems to me, is that in your concern
for "the likelihood that cache flushes will simply remove entries
we'll soon have to rebuild," you're apparently unwilling to consider
the possibility of workloads where cache flushes will remove entries
we *won't* soon have to rebuild. Every time that issue gets raised,
you seem to blow it off as if it were not a thing that really happens.
I can't make sense of that position. Is it really so hard to imagine
a connection pooler that switches the same connection back and forth
between two applications with different working sets? Or a system
that keeps persistent connections open even when they are idle? Do
you really believe that a connection that has not accessed a cache
entry in 10 minutes still derives more benefit from that cache entry
than it would from freeing up some memory?

Well, I think everyone agrees there are workloads that cause undesired
cache bloat. What we have not found is a solution that doesn't cause
code complexity or undesired overhead, or one that >1% of users will
know how to use.

Unfortunately, because we have not found something we are happy with, we
have done nothing. I agree LRU can be expensive. What if we do some
kind of clock sweep and expiration like we do for shared buffers? I
think the trick is figuring how frequently to do the sweep. What if we
mark entries as unused every 10 queries, mark them as used on first use,
and delete cache entries that have not be used in the past 10 queries.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

On 18/01/2019 08:48, Bruce Momjian wrote:

On Thu, Jan 17, 2019 at 11:33:35AM -0500, Robert Haas wrote:

The flaw in your thinking, as it seems to me, is that in your concern
for "the likelihood that cache flushes will simply remove entries
we'll soon have to rebuild," you're apparently unwilling to consider
the possibility of workloads where cache flushes will remove entries
we *won't* soon have to rebuild. Every time that issue gets raised,
you seem to blow it off as if it were not a thing that really happens.
I can't make sense of that position. Is it really so hard to imagine
a connection pooler that switches the same connection back and forth
between two applications with different working sets? Or a system
that keeps persistent connections open even when they are idle? Do
you really believe that a connection that has not accessed a cache
entry in 10 minutes still derives more benefit from that cache entry
than it would from freeing up some memory?

Well, I think everyone agrees there are workloads that cause undesired
cache bloat. What we have not found is a solution that doesn't cause
code complexity or undesired overhead, or one that >1% of users will
know how to use.

Unfortunately, because we have not found something we are happy with, we
have done nothing. I agree LRU can be expensive. What if we do some
kind of clock sweep and expiration like we do for shared buffers? I
think the trick is figuring how frequently to do the sweep. What if we
mark entries as unused every 10 queries, mark them as used on first use,
and delete cache entries that have not be used in the past 10 queries.

If you take that approach, then this number should be configurable. 
What if I had 12 common queries I used in rotation?

The ARM3 processor cache logic was to simply eject an entry at random,
as the obviously Acorn felt that the silicon required to have a more
sophisticated algorithm would reduce the cache size too much!

I upgraded my Acorn Archimedes that had an 8MHZ bus, from an 8MHz ARM2
to a 25MZ ARM3. that is a clock rate improvement of about 3 times. 
However BASIC programs ran about 7 times faster, which I put down to the
ARM3 having a cache.

Obviously for Postgres this is not directly relevant, but I think it
suggests that it may be worth considering replacing cache items at
random.  As there are no pathological corner cases, and the logic is
very simple.

Cheers,
Gavin

Hello.

At Fri, 18 Jan 2019 11:46:03 +1300, Gavin Flower <GavinFlower@archidevsys.co.nz> wrote in <4e62e6b7-0ffb-54ae-3757-5583fcca38c0@archidevsys.co.nz>

On 18/01/2019 08:48, Bruce Momjian wrote:

On Thu, Jan 17, 2019 at 11:33:35AM -0500, Robert Haas wrote:

The flaw in your thinking, as it seems to me, is that in your concern
for "the likelihood that cache flushes will simply remove entries
we'll soon have to rebuild," you're apparently unwilling to consider
the possibility of workloads where cache flushes will remove entries
we *won't* soon have to rebuild. Every time that issue gets raised,
you seem to blow it off as if it were not a thing that really happens.
I can't make sense of that position. Is it really so hard to imagine
a connection pooler that switches the same connection back and forth
between two applications with different working sets? Or a system
that keeps persistent connections open even when they are idle? Do
you really believe that a connection that has not accessed a cache
entry in 10 minutes still derives more benefit from that cache entry
than it would from freeing up some memory?

Well, I think everyone agrees there are workloads that cause undesired
cache bloat. What we have not found is a solution that doesn't cause
code complexity or undesired overhead, or one that >1% of users will
know how to use.

Unfortunately, because we have not found something we are happy with,
we
have done nothing. I agree LRU can be expensive. What if we do some
kind of clock sweep and expiration like we do for shared buffers? I

So, it doesn't use LRU but a kind of clock-sweep method. If it
finds the size is about to exceed the threshold by
resiz(doubl)ing when the current hash is filled up, it tries to
trim away the entries that are left for a duration corresponding
to usage count. This is not a hard limit but seems to be a good
compromise.

think the trick is figuring how frequently to do the sweep. What if
we
mark entries as unused every 10 queries, mark them as used on first
use,
and delete cache entries that have not be used in the past 10 queries.

As above, it tires pruning at every resizing time. So this adds
complexity to the frequent paths only by setting last accessed
time and incrementing access counter. It scans the whole hash at
resize time but it doesn't add much comparing to resizing itself.

If you take that approach, then this number should be configurable. 
What if I had 12 common queries I used in rotation?

This basically has two knobs. The minimum hash size to do the
pruning and idle time before reaping unused entries, per
catcache.

The ARM3 processor cache logic was to simply eject an entry at random,
as the obviously Acorn felt that the silicon required to have a more
sophisticated algorithm would reduce the cache size too much!

I upgraded my Acorn Archimedes that had an 8MHZ bus, from an 8MHz ARM2
to a 25MZ ARM3. that is a clock rate improvement of about 3 times. 
However BASIC programs ran about 7 times faster, which I put down to
the ARM3 having a cache.

Obviously for Postgres this is not directly relevant, but I think it
suggests that it may be worth considering replacing cache items at
random.  As there are no pathological corner cases, and the logic is
very simple.

Memory was expensive than nowadays by.. about 10^3 times? An
obvious advantage of random reaping is requiring less silicon. I
think we don't need to be so stingy but perhaps clock-sweep is at
the maximum we can pay.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

At Fri, 18 Jan 2019 16:39:29 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in <20190118.163929.229869562.horiguchi.kyotaro@lab.ntt.co.jp>

Hello.

At Fri, 18 Jan 2019 11:46:03 +1300, Gavin Flower <GavinFlower@archidevsys.co.nz> wrote in <4e62e6b7-0ffb-54ae-3757-5583fcca38c0@archidevsys.co.nz>

On 18/01/2019 08:48, Bruce Momjian wrote:

Unfortunately, because we have not found something we are happy with,
we
have done nothing. I agree LRU can be expensive. What if we do some
kind of clock sweep and expiration like we do for shared buffers? I

So, it doesn't use LRU but a kind of clock-sweep method. If it
finds the size is about to exceed the threshold by
resiz(doubl)ing when the current hash is filled up, it tries to
trim away the entries that are left for a duration corresponding
to usage count. This is not a hard limit but seems to be a good
compromise.

think the trick is figuring how frequently to do the sweep. What if
we
mark entries as unused every 10 queries, mark them as used on first
use,
and delete cache entries that have not be used in the past 10 queries.

As above, it tires pruning at every resizing time. So this adds
complexity to the frequent paths only by setting last accessed
time and incrementing access counter. It scans the whole hash at
resize time but it doesn't add much comparing to resizing itself.

If you take that approach, then this number should be configurable. 
What if I had 12 common queries I used in rotation?

This basically has two knobs. The minimum hash size to do the
pruning and idle time before reaping unused entries, per
catcache.

This is the rebased version.

0001: catcache pruning

syscache_memory_target controls per-cache basis minimum size
where this starts pruning.

syscache_prune_min_time controls minimum idle duration until an
catcache entry is removed.

0002: catcache statistics view

track_syscache_usage_interval is the interval statitics of
catcache is collected.

pg_stat_syscache is the view that shows the statistics.

0003: Remote GUC setting

It is independent from the above two, and heavily arguable.

pg_set_backend_config(pid, name, value) changes the GUC <name> on
the backend with <pid> to <value>.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

On Thu, Jan 17, 2019 at 2:48 PM Bruce Momjian <bruce@momjian.us> wrote:

Well, I think everyone agrees there are workloads that cause undesired
cache bloat. What we have not found is a solution that doesn't cause
code complexity or undesired overhead, or one that >1% of users will
know how to use.

Unfortunately, because we have not found something we are happy with, we
have done nothing. I agree LRU can be expensive. What if we do some
kind of clock sweep and expiration like we do for shared buffers? I
think the trick is figuring how frequently to do the sweep. What if we
mark entries as unused every 10 queries, mark them as used on first use,
and delete cache entries that have not be used in the past 10 queries.

I still think wall-clock time is a perfectly reasonable heuristic.
Say every 5 or 10 minutes you walk through the cache. Anything that
hasn't been touched since the last scan you throw away. If you do
this, you MIGHT flush an entry that you're just about to need again,
but (1) it's not very likely, because if it hasn't been touched in
many minutes, the chances that it's about to be needed again are low,
and (2) even if it does happen, it probably won't cost all that much,
because *occasionally* reloading a cache entry unnecessarily isn't
that costly; the big problem is when you do it over and over again,
which can easily happen with a fixed size limit on the cache, and (3)
if somebody does have a workload where they touch the same object
every 11 minutes, we can give them a GUC to control the timeout
between cache sweeps and it's really not that hard to understand how
to set it. And most people won't need to.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:

On Thu, Jan 17, 2019 at 2:48 PM Bruce Momjian <bruce@momjian.us> wrote:

Unfortunately, because we have not found something we are happy with, we
have done nothing. I agree LRU can be expensive. What if we do some
kind of clock sweep and expiration like we do for shared buffers? I
think the trick is figuring how frequently to do the sweep. What if we
mark entries as unused every 10 queries, mark them as used on first use,
and delete cache entries that have not be used in the past 10 queries.

I still think wall-clock time is a perfectly reasonable heuristic.

The easy implementations of that involve putting gettimeofday() calls
into hot code paths, which would be a Bad Thing. But maybe we could
do this only at transaction or statement start, and piggyback on the
gettimeofday() calls that already happen at those times.

regards, tom lane

On 2019-01-18 15:57:17 -0500, Tom Lane wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Thu, Jan 17, 2019 at 2:48 PM Bruce Momjian <bruce@momjian.us> wrote:

Unfortunately, because we have not found something we are happy with, we
have done nothing. I agree LRU can be expensive. What if we do some
kind of clock sweep and expiration like we do for shared buffers? I
think the trick is figuring how frequently to do the sweep. What if we
mark entries as unused every 10 queries, mark them as used on first use,
and delete cache entries that have not be used in the past 10 queries.

I still think wall-clock time is a perfectly reasonable heuristic.

The easy implementations of that involve putting gettimeofday() calls
into hot code paths, which would be a Bad Thing. But maybe we could
do this only at transaction or statement start, and piggyback on the
gettimeofday() calls that already happen at those times.

My proposal for this was to attach a 'generation' to cache entries. Upon
access cache entries are marked to be of the current
generation. Whenever existing memory isn't sufficient for further cache
entries and, on a less frequent schedule, triggered by a timer, the
cache generation is increased and th new generation's "creation time" is
measured. Then generations that are older than a certain threshold are
purged, and if there are any, the entries of the purged generation are
removed from the caches using a sequential scan through the cache.

This outline achieves:
- no additional time measurements in hot code paths
- no need for a sequential scan of the entire cache when no generations
are too old
- both size and time limits can be implemented reasonably cheaply
- overhead when feature disabled should be close to zero

Greetings,

Andres Freund

On Fri, Jan 18, 2019 at 4:23 PM andres@anarazel.de <andres@anarazel.de> wrote:

My proposal for this was to attach a 'generation' to cache entries. Upon
access cache entries are marked to be of the current
generation. Whenever existing memory isn't sufficient for further cache
entries and, on a less frequent schedule, triggered by a timer, the
cache generation is increased and th new generation's "creation time" is
measured. Then generations that are older than a certain threshold are
purged, and if there are any, the entries of the purged generation are
removed from the caches using a sequential scan through the cache.

This outline achieves:
- no additional time measurements in hot code paths
- no need for a sequential scan of the entire cache when no generations
are too old
- both size and time limits can be implemented reasonably cheaply
- overhead when feature disabled should be close to zero

Seems generally reasonable. The "whenever existing memory isn't
sufficient for further cache entries" part I'm not sure about.
Couldn't that trigger very frequently and prevent necessary cache size
growth?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Hi,

On 2019-01-18 19:57:03 -0500, Robert Haas wrote:

On Fri, Jan 18, 2019 at 4:23 PM andres@anarazel.de <andres@anarazel.de> wrote:

My proposal for this was to attach a 'generation' to cache entries. Upon
access cache entries are marked to be of the current
generation. Whenever existing memory isn't sufficient for further cache
entries and, on a less frequent schedule, triggered by a timer, the
cache generation is increased and th new generation's "creation time" is
measured. Then generations that are older than a certain threshold are
purged, and if there are any, the entries of the purged generation are
removed from the caches using a sequential scan through the cache.

This outline achieves:
- no additional time measurements in hot code paths
- no need for a sequential scan of the entire cache when no generations
are too old
- both size and time limits can be implemented reasonably cheaply
- overhead when feature disabled should be close to zero

Seems generally reasonable. The "whenever existing memory isn't
sufficient for further cache entries" part I'm not sure about.
Couldn't that trigger very frequently and prevent necessary cache size
growth?

I'm thinking it'd just trigger a new generation, with it's associated
"creation" time (which is cheap to acquire in comparison to creating a
number of cache entries) . Depending on settings or just code policy we
can decide up to which generation to prune the cache, using that
creation time. I'd imagine that we'd have some default cache-pruning
time in the minutes, and for workloads where relevant one can make
sizing configurations more aggressive - or something like that.

Greetings,

Andres Freund

From: Kyotaro HORIGUCHI [mailto:horiguchi.kyotaro@lab.ntt.co.jp]

0003: Remote GUC setting

It is independent from the above two, and heavily arguable.

pg_set_backend_config(pid, name, value) changes the GUC <name> on the
backend with <pid> to <value>.

Not having looked at the code yet, why did you think this is necessary? Can't we always collect the cache stats? Is it heavy due to some locking in the shared memory, or sending the stats to the stats collector?

Regards
Takayuki Tsunakawa

Hello.

At Fri, 18 Jan 2019 17:09:41 -0800, "andres@anarazel.de" <andres@anarazel.de> wrote in <20190119010941.6ruftewah7t3k3yk@alap3.anarazel.de>

Hi,

On 2019-01-18 19:57:03 -0500, Robert Haas wrote:

On Fri, Jan 18, 2019 at 4:23 PM andres@anarazel.de <andres@anarazel.de> wrote:

My proposal for this was to attach a 'generation' to cache entries. Upon
access cache entries are marked to be of the current
generation. Whenever existing memory isn't sufficient for further cache
entries and, on a less frequent schedule, triggered by a timer, the
cache generation is increased and th new generation's "creation time" is
measured. Then generations that are older than a certain threshold are
purged, and if there are any, the entries of the purged generation are
removed from the caches using a sequential scan through the cache.

This outline achieves:
- no additional time measurements in hot code paths

It is caused at every transaction start time and stored in
TimestampTz in this patch. No additional time measurement exists
already but cache puruing won't happen if a transaction lives for
a long time. Time-driven generation value, maybe with 10s-1min
fixed interval, is a possible option.

- no need for a sequential scan of the entire cache when no generations
are too old

This patch didn't precheck against the oldest generation, but it
can be easily calculated. (But doesn't base on the creation time
but on the last-access time.) (Attached applies over the
v7-0001-Remove-entries-..patch)

Using generation time, entries are purged even if it is recently
accessed. I think last-accessed time is more sutable for the
purpse. On the other hand using last-accessed time, the oldest
generation can be stale by later access.

- both size and time limits can be implemented reasonably cheaply
- overhead when feature disabled should be close to zero

Overhead when disabled is already nothing since scanning is
inhibited when cache_prune_min_age is a negative value.

Seems generally reasonable. The "whenever existing memory isn't
sufficient for further cache entries" part I'm not sure about.
Couldn't that trigger very frequently and prevent necessary cache size
growth?

I'm thinking it'd just trigger a new generation, with it's associated
"creation" time (which is cheap to acquire in comparison to creating a
number of cache entries) . Depending on settings or just code policy we
can decide up to which generation to prune the cache, using that
creation time. I'd imagine that we'd have some default cache-pruning
time in the minutes, and for workloads where relevant one can make
sizing configurations more aggressive - or something like that.

The current patch uses last-accesed time by non-gettimeofday()
method. The genreation is fixed up to 3 and infrequently-accessed
entries are removed sooner. Generation interval is determined by
cache_prune_min_age.

Although this doesn't put a hard cap on memory usage, it is
indirectly and softly limited by the cache_prune_min_age and
cache_memory_target, which determins how large a cache can grow
until pruning happens. They are per-cache basis.

If we prefer to set a budget on all the syschaches (or even
including other caches), it would be more complex.

regares.

--
Kyotaro Horiguchi
NTT Open Source Software Center

Thank you for pointing out the stupidity. (Tom did earlier, though.)

At Mon, 21 Jan 2019 07:12:41 +0000, "Tsunakawa, Takayuki" <tsunakawa.takay@jp.fujitsu.com> wrote in <0A3221C70F24FB45833433255569204D1FB6C78A@G01JPEXMBYT05>

From: Kyotaro HORIGUCHI [mailto:horiguchi.kyotaro@lab.ntt.co.jp]

0003: Remote GUC setting

It is independent from the above two, and heavily arguable.

pg_set_backend_config(pid, name, value) changes the GUC <name> on the
backend with <pid> to <value>.

Not having looked at the code yet, why did you think this is necessary? Can't we always collect the cache stats? Is it heavy due to some locking in the shared memory, or sending the stats to the stats collector?

Yeah, I had a fun making it but I don't think it can be said very
good. I must admit that it is a kind of too-much or something
stupid.

Anyway it needs to scan the whole hash to collect numbers and I
don't see how to elimite the complexity without a penalty on
regular code paths for now. I don't want do that always for the
reason.

An option is an additional PGPROC member and interface functions.

struct PGPROC
{
...
int syscahe_usage_track_interval; /* track interval, 0 to disable */

=# select syscahce_usage_track_add(<pid>, <intvl>[, <repetition>]);
=# select syscahce_usage_track_remove(2134);

Or, just provide an one-shot triggering function.

=# select syscahce_take_usage_track(<pid>);

This can use both a similar PGPROC variable or SendProcSignal()
but the former doesn't fire while idle time unless using timer.

Any thoughts?

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

On Fri, Jan 18, 2019 at 05:09:41PM -0800, Andres Freund wrote:

Hi,

On 2019-01-18 19:57:03 -0500, Robert Haas wrote:

On Fri, Jan 18, 2019 at 4:23 PM andres@anarazel.de <andres@anarazel.de> wrote:

My proposal for this was to attach a 'generation' to cache entries. Upon
access cache entries are marked to be of the current
generation. Whenever existing memory isn't sufficient for further cache
entries and, on a less frequent schedule, triggered by a timer, the
cache generation is increased and th new generation's "creation time" is
measured. Then generations that are older than a certain threshold are
purged, and if there are any, the entries of the purged generation are
removed from the caches using a sequential scan through the cache.

This outline achieves:
- no additional time measurements in hot code paths
- no need for a sequential scan of the entire cache when no generations
are too old
- both size and time limits can be implemented reasonably cheaply
- overhead when feature disabled should be close to zero

Seems generally reasonable. The "whenever existing memory isn't
sufficient for further cache entries" part I'm not sure about.
Couldn't that trigger very frequently and prevent necessary cache size
growth?

I'm thinking it'd just trigger a new generation, with it's associated
"creation" time (which is cheap to acquire in comparison to creating a
number of cache entries) . Depending on settings or just code policy we
can decide up to which generation to prune the cache, using that
creation time. I'd imagine that we'd have some default cache-pruning
time in the minutes, and for workloads where relevant one can make
sizing configurations more aggressive - or something like that.

OK, so it seems everyone likes the idea of a timer. The open questions
are whether we want multiple epochs, and whether we want some kind of
size trigger.

With only one time epoch, if the timer is 10 minutes, you could expire an
entry after 10-19 minutes, while with a new epoch every minute and
10-minute expire, you can do 10-11 minute precision. I am not sure the
complexity is worth it.

For a size trigger, should removal be effected by how many expired cache
entries there are? If there were 10k expired entries or 50, wouldn't
you want them removed if they have not been accessed in X minutes?

In the worst case, if 10k entries were accessed in a query and never
accessed again, what would the ideal cleanup behavior be? Would it
matter if it was expired in 10 or 19 minutes? Would it matter if there
were only 50 entries?

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

From: Kyotaro HORIGUCHI [mailto:horiguchi.kyotaro@lab.ntt.co.jp]

Although this doesn't put a hard cap on memory usage, it is indirectly and
softly limited by the cache_prune_min_age and cache_memory_target, which
determins how large a cache can grow until pruning happens. They are
per-cache basis.

If we prefer to set a budget on all the syschaches (or even including other
caches), it would be more complex.

This is a pure question. How can we answer these questions from users?

* What value can I set to cache_memory_target when I can use 10 GB for the caches and max_connections = 100?
* How much RAM do I need to have for the caches when I set cache_memory_target = 1M?

The user tends to estimate memory to avoid OOM.

Regards
Takayuki Tsunakawa

At Mon, 21 Jan 2019 17:22:55 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in <20190121.172255.226467552.horiguchi.kyotaro@lab.ntt.co.jp>

An option is an additional PGPROC member and interface functions.

struct PGPROC
{
...
int syscahe_usage_track_interval; /* track interval, 0 to disable */

=# select syscahce_usage_track_add(<pid>, <intvl>[, <repetition>]);
=# select syscahce_usage_track_remove(2134);

Or, just provide an one-shot triggering function.

=# select syscahce_take_usage_track(<pid>);

This can use both a similar PGPROC variable or SendProcSignal()
but the former doesn't fire while idle time unless using timer.

The attached is revised version of this patchset, where the third
patch is the remote setting feature. It uses static shared memory.

=# select pg_backend_catcache_stats(<pid>, <millis>);

Activates or changes catcache stats feature on the backend with
PID. (The name should be changed to .._syscache_stats, though.)
It is far smaller than the remote-GUC feature. (It contains a
part that should be in the previous patch. I will fix it later.)

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

On Wed, Jan 23, 2019 at 05:35:02PM +0900, Kyotaro HORIGUCHI wrote:

At Mon, 21 Jan 2019 17:22:55 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in <20190121.172255.226467552.horiguchi.kyotaro@lab.ntt.co.jp>

An option is an additional PGPROC member and interface functions.

struct PGPROC
{
...
int syscahe_usage_track_interval; /* track interval, 0 to disable */

=# select syscahce_usage_track_add(<pid>, <intvl>[, <repetition>]);
=# select syscahce_usage_track_remove(2134);

Or, just provide an one-shot triggering function.

=# select syscahce_take_usage_track(<pid>);

This can use both a similar PGPROC variable or SendProcSignal()
but the former doesn't fire while idle time unless using timer.

The attached is revised version of this patchset, where the third
patch is the remote setting feature. It uses static shared memory.

=# select pg_backend_catcache_stats(<pid>, <millis>);

Activates or changes catcache stats feature on the backend with
PID. (The name should be changed to .._syscache_stats, though.)
It is far smaller than the remote-GUC feature. (It contains a
part that should be in the previous patch. I will fix it later.)

I have a few questions to make sure we have not made the API too
complex. First, for syscache_prune_min_age, that is the minimum age
that we prune, and entries could last twice that long. Is there any
value to doing the scan at 50% of the age so that the
syscache_prune_min_age is the max age? For example, if our age cutoff
is 10 minutes, we could scan every 5 minutes so 10 minutes would be the
maximum age kept.

Second, when would you use syscache_memory_target != 0? If you had
syscache_prune_min_age really fast, e.g. 10 seconds? What is the
use-case for this? You have a query that touches 10k objects, and then
the connection stays active but doesn't touch many of those 10k objects,
and you want it cleaned up in seconds instead of minutes? (I can't see
why you would not clean up all unreferenced objects after _minutes_ of
disuse, but removing them after seconds of disuse seems undesirable.)
What are the odds you would retain the entires you want with a fast
target?

What is the value of being able to change a specific backend's stat
interval? I don't remember any other setting having this ability.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

Thank you for the comments.

At Wed, 23 Jan 2019 18:21:45 -0500, Bruce Momjian <bruce@momjian.us> wrote in <20190123232145.GA8334@momjian.us>

On Wed, Jan 23, 2019 at 05:35:02PM +0900, Kyotaro HORIGUCHI wrote:

At Mon, 21 Jan 2019 17:22:55 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in <20190121.172255.226467552.horiguchi.kyotaro@lab.ntt.co.jp>

An option is an additional PGPROC member and interface functions.

struct PGPROC
{
...
int syscahe_usage_track_interval; /* track interval, 0 to disable */

=# select syscahce_usage_track_add(<pid>, <intvl>[, <repetition>]);
=# select syscahce_usage_track_remove(2134);

Or, just provide an one-shot triggering function.

=# select syscahce_take_usage_track(<pid>);

This can use both a similar PGPROC variable or SendProcSignal()
but the former doesn't fire while idle time unless using timer.

The attached is revised version of this patchset, where the third
patch is the remote setting feature. It uses static shared memory.

=# select pg_backend_catcache_stats(<pid>, <millis>);

Activates or changes catcache stats feature on the backend with
PID. (The name should be changed to .._syscache_stats, though.)
It is far smaller than the remote-GUC feature. (It contains a
part that should be in the previous patch. I will fix it later.)

I have a few questions to make sure we have not made the API too
complex. First, for syscache_prune_min_age, that is the minimum age
that we prune, and entries could last twice that long. Is there any
value to doing the scan at 50% of the age so that the
syscache_prune_min_age is the max age? For example, if our age cutoff
is 10 minutes, we could scan every 5 minutes so 10 minutes would be the
maximum age kept.

(Looking into the patch..) Actually thrice, not twice. It is
because I put significance on the access frequency. I think it is
reasonable that the entries with more frequent access gets longer
life (within a certain limit). The original problem here was
negative caches that are created but never accessed. However,
there's no firm reason for the number of the steps (3). There
might be no difference if the extra life time were up to once of
s_p_m_age or even with no extra time.

Second, when would you use syscache_memory_target != 0?

It is a suggestion upthread, we sometimes want to keep some known
amount of caches despite that expration should be activated.

If you had
syscache_prune_min_age really fast, e.g. 10 seconds? What is the
use-case for this? You have a query that touches 10k objects, and then
the connection stays active but doesn't touch many of those 10k objects,
and you want it cleaned up in seconds instead of minutes? (I can't see
why you would not clean up all unreferenced objects after _minutes_ of
disuse, but removing them after seconds of disuse seems undesirable.)
What are the odds you would retain the entires you want with a fast
target?

Do you asking the reason for the unit? It's just because it won't
be so large even in seconds, to the utmost 3600 seconds. Even
though I don't think such a short dutaion setting is meaningful
in the real world, either I don't think we need to inhibit
that. (Actually it is useful for testing:p) Another reason is
that GUC_UNIT_MIN doesn't seem so common that it is used only by
two variables, log_rotation_age and old_snapshot_threshold.

What is the value of being able to change a specific backend's stat
interval? I don't remember any other setting having this ability.

As mentioned upthread, it takes significant time to take
statistics so I believe no one is willing to turn it on at all
times. As the result it should be useless because it cannot be
turned on on an active backend when it actually gets bloat. So I
wanted to provide a remote switching feture.

I also thought that there's some other features that is useful if
it could be turned on remotely so the remote GUC feature but it
was too complex...

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

On Thu, Jan 24, 2019 at 06:39:24PM +0900, Kyotaro HORIGUCHI wrote:

Second, when would you use syscache_memory_target != 0?

It is a suggestion upthread, we sometimes want to keep some known
amount of caches despite that expration should be activated.

If you had
syscache_prune_min_age really fast, e.g. 10 seconds? What is the
use-case for this? You have a query that touches 10k objects, and then
the connection stays active but doesn't touch many of those 10k objects,
and you want it cleaned up in seconds instead of minutes? (I can't see
why you would not clean up all unreferenced objects after _minutes_ of
disuse, but removing them after seconds of disuse seems undesirable.)
What are the odds you would retain the entires you want with a fast
target?

Do you asking the reason for the unit? It's just because it won't
be so large even in seconds, to the utmost 3600 seconds. Even
though I don't think such a short dutaion setting is meaningful
in the real world, either I don't think we need to inhibit
that. (Actually it is useful for testing:p) Another reason is

We have gone from ignoring the cache bloat problem to designing an API
that even we don't know what value they provide, and if we don't know,
we can be sure our users will not know. Every GUC has a cost, even if
it is not used.

I suggest you go with just syscache_prune_min_age, get that into PG 12,
and we can then reevaluate what we need. If you want to hard-code a
minimum cache size where no pruning will happen, maybe based on the system
catalogs or typical load, that is fine.

that GUC_UNIT_MIN doesn't seem so common that it is used only by
two variables, log_rotation_age and old_snapshot_threshold.

What is the value of being able to change a specific backend's stat
interval? I don't remember any other setting having this ability.

As mentioned upthread, it takes significant time to take
statistics so I believe no one is willing to turn it on at all
times. As the result it should be useless because it cannot be
turned on on an active backend when it actually gets bloat. So I
wanted to provide a remote switching feture.

I also thought that there's some other features that is useful if
it could be turned on remotely so the remote GUC feature but it
was too complex...

Well, I am thinking if we want to do something like this, we should do
it for all GUCs, not just for this one, so I suggest we not do this now
either.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

Bruce Momjian <bruce@momjian.us> writes:

On Thu, Jan 24, 2019 at 06:39:24PM +0900, Kyotaro HORIGUCHI wrote:

I also thought that there's some other features that is useful if
it could be turned on remotely so the remote GUC feature but it
was too complex...

Well, I am thinking if we want to do something like this, we should do
it for all GUCs, not just for this one, so I suggest we not do this now
either.

I will argue hard that we should not do it at all, ever.

There is already a mechanism for broadcasting global GUC changes:
apply them to postgresql.conf (or use ALTER SYSTEM) and SIGHUP.
I do not think we need something that can remotely change a GUC's
value in just one session. The potential for bugs, misuse, and
just plain confusion is enormous, and the advantage seems minimal.

regards, tom lane

On Thu, Jan 24, 2019 at 10:02 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Bruce Momjian <bruce@momjian.us> writes:

On Thu, Jan 24, 2019 at 06:39:24PM +0900, Kyotaro HORIGUCHI wrote:

I also thought that there's some other features that is useful if
it could be turned on remotely so the remote GUC feature but it
was too complex...

Well, I am thinking if we want to do something like this, we should do
it for all GUCs, not just for this one, so I suggest we not do this now
either.

I will argue hard that we should not do it at all, ever.

There is already a mechanism for broadcasting global GUC changes:
apply them to postgresql.conf (or use ALTER SYSTEM) and SIGHUP.
I do not think we need something that can remotely change a GUC's
value in just one session. The potential for bugs, misuse, and
just plain confusion is enormous, and the advantage seems minimal.

I think there might be some merit in being able to activate debugging
or tracing facilities for a particular session remotely, but designing
something that will do that sort of thing well seems like a very
complex problem that certainly should not be sandwiched into another
patch that is mostly about something else. And if we ever get such a
thing I suspect it should be entirely separate from the GUC system.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

From: Robert Haas [mailto:robertmhaas@gmail.com]

On Thu, Jan 24, 2019 at 10:02 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

I will argue hard that we should not do it at all, ever.

There is already a mechanism for broadcasting global GUC changes:
apply them to postgresql.conf (or use ALTER SYSTEM) and SIGHUP.
I do not think we need something that can remotely change a GUC's
value in just one session. The potential for bugs, misuse, and
just plain confusion is enormous, and the advantage seems minimal.

I think there might be some merit in being able to activate debugging
or tracing facilities for a particular session remotely, but designing
something that will do that sort of thing well seems like a very
complex problem that certainly should not be sandwiched into another
patch that is mostly about something else. And if we ever get such a
thing I suspect it should be entirely separate from the GUC system.

+1 for a separate patch for remote session configuration. ALTER SYSTEM + SIGHUP targeted at a particular backend would do if the DBA can log into the database server (so, it can't be used for DBaaS.) It would be useful to have pg_reload_conf(pid).

Regards
Takayuki Tsunakawa

Hi Horiguchi-san, Bruce,

From: Bruce Momjian [mailto:bruce@momjian.us]

I suggest you go with just syscache_prune_min_age, get that into PG 12,
and we can then reevaluate what we need. If you want to hard-code a
minimum cache size where no pruning will happen, maybe based on the system
catalogs or typical load, that is fine.

Please forgive me if I say something silly (I might have got lost.)

Are you suggesting to make the cache size limit system-defined and uncontrollable by the user? I think it's necessary for the DBA to be able to control the cache memory amount. Otherwise, if many concurrent connections access many partitions within a not-so-long duration, then the cache eviction can't catch up and ends up in OOM. How about the following questions I asked in my previous mail?

--------------------------------------------------
This is a pure question. How can we answer these questions from users?

* What value can I set to cache_memory_target when I can use 10 GB for the caches and max_connections = 100?
* How much RAM do I need to have for the caches when I set cache_memory_target = 1M?

The user tends to estimate memory to avoid OOM.
--------------------------------------------------

Regards
Takayuki Tsunakawa

On Fri, Jan 25, 2019 at 08:14:19AM +0000, Tsunakawa, Takayuki wrote:

Hi Horiguchi-san, Bruce,

From: Bruce Momjian [mailto:bruce@momjian.us]

I suggest you go with just syscache_prune_min_age, get that into
PG 12, and we can then reevaluate what we need. If you want to
hard-code a minimum cache size where no pruning will happen, maybe
based on the system catalogs or typical load, that is fine.

Please forgive me if I say something silly (I might have got lost.)

Are you suggesting to make the cache size limit system-defined and
uncontrollable by the user? I think it's necessary for the DBA to
be able to control the cache memory amount. Otherwise, if many
concurrent connections access many partitions within a not-so-long
duration, then the cache eviction can't catch up and ends up in OOM.
How about the following questions I asked in my previous mail?

----------------------------------------------------------------------
This is a pure question. How can we answer these questions from
users?

* What value can I set to cache_memory_target when I can use 10 GB for
* the caches and max_connections = 100? How much RAM do I need to
* have for the caches when I set cache_memory_target = 1M?

The user tends to estimate memory to avoid OOM.

Well, let's walk through this. Suppose the default for
syscache_prune_min_age is 10 minutes, and that we prune all cache
entries unreferenced in the past 10 minutes, or we only prune every 10
minutes if the cache size is larger than some fixed size like 100.

So, when would you change syscache_prune_min_age? If you reference many
objects and then don't reference them at all for minutes, you might want
to lower syscache_prune_min_age to maybe 1 minute. Why would you want
to change the behavior of removing all unreferenced cache items, at
least when there are more than 100? (You called this
syscache_memory_target.)

My point is I can see someone wanting to change syscache_prune_min_age,
but I can't see someone wanting to change syscache_memory_target. Who
would want to keep 5k cache entries that have not been accessed in X
minutes? If we had some global resource manager that would allow you to
control work_mem, maintenance_work_mem, cache size, and set global
limits on their sizes, I can see where maybe it might make sense, but
right now the memory usage of a backend is so fluid that setting some
limit on its size for unreferenced entries just doesn't make sense.

One of my big points is that syscache_memory_target doesn't even
guarantee that the cache will be this size or lower, it only controls
whether the cleanup happens at syscache_prune_min_age intervals.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

At Fri, 25 Jan 2019 08:14:19 +0000, "Tsunakawa, Takayuki" <tsunakawa.takay@jp.fujitsu.com> wrote in <0A3221C70F24FB45833433255569204D1FB70EFB@G01JPEXMBYT05>

Hi Horiguchi-san, Bruce,

From: Bruce Momjian [mailto:bruce@momjian.us]

I suggest you go with just syscache_prune_min_age, get that into PG 12,
and we can then reevaluate what we need. If you want to hard-code a
minimum cache size where no pruning will happen, maybe based on the system
catalogs or typical load, that is fine.

Please forgive me if I say something silly (I might have got lost.)

Are you suggesting to make the cache size limit system-defined and uncontrollable by the user? I think it's necessary for the DBA to be able to control the cache memory amount. Otherwise, if many concurrent connections access many partitions within a not-so-long duration, then the cache eviction can't catch up and ends up in OOM. How about the following questions I asked in my previous mail?

cache_memory_target does the opposit of limiting memory usage. It
keeps some amount of syscahe entries unpruned. It is intended for
sessions on where cache-effective queries runs intermittently.
syscache_prune_min_age also doesn't directly limit the size. It
just eventually prevents infinite memory consumption.

The knobs are not no-brainer at all and don't need tuning in most
cases.

--------------------------------------------------
This is a pure question. How can we answer these questions from users?

* What value can I set to cache_memory_target when I can use 10 GB for the caches and max_connections = 100?
* How much RAM do I need to have for the caches when I set cache_memory_target = 1M?

The user tends to estimate memory to avoid OOM.
--------------------------------------------------

You don't have a direct control on syscache memory usage. When
you find a queriy slowed by the default cache expiration, you can
set cache_memory_taret to keep them for intermittent execution of
a query, or you can increase syscache_prune_min_age to allow
cache live for a longer time.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

At Fri, 25 Jan 2019 07:26:46 +0000, "Tsunakawa, Takayuki" <tsunakawa.takay@jp.fujitsu.com> wrote in <0A3221C70F24FB45833433255569204D1FB70E6B@G01JPEXMBYT05>

From: Robert Haas [mailto:robertmhaas@gmail.com]

On Thu, Jan 24, 2019 at 10:02 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

I will argue hard that we should not do it at all, ever.

There is already a mechanism for broadcasting global GUC changes:
apply them to postgresql.conf (or use ALTER SYSTEM) and SIGHUP.
I do not think we need something that can remotely change a GUC's
value in just one session. The potential for bugs, misuse, and
just plain confusion is enormous, and the advantage seems minimal.

I think there might be some merit in being able to activate debugging
or tracing facilities for a particular session remotely, but designing
something that will do that sort of thing well seems like a very
complex problem that certainly should not be sandwiched into another
patch that is mostly about something else. And if we ever get such a
thing I suspect it should be entirely separate from the GUC system.

It means that we have a lesser copy of the GUC system but can be
set remotely, then some features explicitly register their own
knob on the new system, with the name that I suspenct it should
be the same to the related GUC (for users' convenient).

+1 for a separate patch for remote session configuration.

It sounds reasnable for me. As I said there should be some such
variables.

ALTER SYSTEM + SIGHUP targeted at a particular backend would do
if the DBA can log into the database server (so, it can't be
used for DBaaS.) It would be useful to have
pg_reload_conf(pid).

I don't think it is reasonable. ALTER SYSTEM alters a *system*
configuration which is assumed to be the same on all sessions and
other processes. All sessions start the syscache tracking if
another ALTER SYSTEM for another variable then pg_reload_conf()
come after doing the above. I think the change should persist no
longer than the session-lifetime.

I think that a consensus on backend-targetted remote tuning is
made here:)

A. Let GUC variables settable by a remote session.

A-1. Variables are changed at a busy time (my first patch).
(transaction-awareness of GUC makes this complex)

A-2. Variables are changed when the session is idle (or outside
a transaction).

B. Override some variables via values laid on shared memory. (my
second or the last patch).

Very specific to a target feature. I think it consumes a bit
too large memory.

C. Provide session-specific GUC variable (that overides the global one)

- Add new configuration file "postgresql.conf.<PID>" and
pg_reload_conf() let the session with the PID loads it as if
it is the last included file. All such files are removed at
startup or at the end of the coressponding session.

- Add a new syntax like this:
ALTER SESSION WITH (pid=xxxx)
SET configuration_parameter {TO | =} {value | 'value' | DEFAULT}
RESET configuration_parameter
RESET ALL

- Target variables are marked with GUC_REMOTE.