Allow table AM's to cache stuff in relcache

Started by Heikki Linnakangasover 7 years ago4 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

won't retrysuccessCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t40777
psql -h localhost -U postgres

Built from patchset v1 (message #1), July 28, 2026 at 04:08 AM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t40777_1 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t40777_1 && git checkout t40777_1

Patchset v1 (message #1) is on t40777_1

Jump to latest
#1Heikki Linnakangas
heikki.linnakangas@enterprisedb.com

Index AM's can cache stuff in RelationData->rd_amcache. In the zedstore
table AM we've been hacking on, I'd like to also use rd_amcache to cache
some information, but that's not currently possible, because rd_amcache
can only be used by index AMs, not table AMs.

Attached patch allows rd_amcache to also be used by table AMs.

While working on this, I noticed that the memory management of relcache
entries is quite complicated. Most stuff that's part of a relcache entry
is allocated in CacheMemoryContext. But some fields have a dedicated
memory context to hold them, like rd_rulescxt for rules and rd_pdcxt for
partition information. And indexes have rd_indexcxt to hold all kinds of
index support info.

In the patch, I documented that rd_amcache must be allocated in
CacheMemoryContext, or in rd_indexcxt if it's an index. It works, but
it's a bit weird. It would nice to have one memory context in every
relcache entry, to hold all the stuff related to it, including
rd_amcache. In other words, it would be nice if we had "rd_indexcxt" for
tables, too, not just indexes. That would allow tracking memory usage
more accurately, if you're debugging an out of memory situation for example.

However, the special contexts like rd_rulescxt and rd_pdcxt would still
be needed, because of the way RelationClearRelation preserves them, when
rebuilding the relcache entry for an open relation. So I'm not sure how
much it would really simplify things. Also, there's some overhead for
having extra memory contexts, and some people already complain that the
relcache uses too much memory.

Alternatively, we could document that rd_amcache should always be
allocated in CacheMemoryContext, even for indexes. That would make the
rule for pg_amcache straightforward. There's no particular reason why
rd_amcache has to be allocated in rd_indexcxt, except for how it's
accounted for in memory context dumps.

- Heikki

Attachments:

t40777_1
0001-Allow-table-AM-s-to-use-rd_amcache-too.patchtext/x-patch; name=0001-Allow-table-AM-s-to-use-rd_amcache-too.patchDownload+20-11
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Heikki Linnakangas (#1)
Re: Allow table AM's to cache stuff in relcache

Heikki Linnakangas <hlinnaka@iki.fi> writes:

Index AM's can cache stuff in RelationData->rd_amcache. In the zedstore
table AM we've been hacking on, I'd like to also use rd_amcache to cache
some information, but that's not currently possible, because rd_amcache
can only be used by index AMs, not table AMs.
Attached patch allows rd_amcache to also be used by table AMs.

Seems reasonable.

In the patch, I documented that rd_amcache must be allocated in
CacheMemoryContext, or in rd_indexcxt if it's an index. It works, but
it's a bit weird.

Given the way the patch is implemented, it doesn't really matter which
context it's in, does it? The retail pfree is inessential but also
harmless, if rd_amcache is in rd_indexcxt. So we could take out the
"must". I think it's slightly preferable to use rd_indexcxt if available,
to reduce the amount of loose junk in CacheMemoryContext.

It would nice to have one memory context in every
relcache entry, to hold all the stuff related to it, including
rd_amcache. In other words, it would be nice if we had "rd_indexcxt" for
tables, too, not just indexes. That would allow tracking memory usage
more accurately, if you're debugging an out of memory situation for example.

We had some discussion related to that in the "hyrax
vs. RelationBuildPartitionDesc" thread. I'm not quite sure where
we'll settle on that, but some redesign seems inevitable.

regards, tom lane

#3Julien Rouhaud
rjuju123@gmail.com
In reply to: Tom Lane (#2)
Re: Allow table AM's to cache stuff in relcache

On Fri, Jun 14, 2019 at 5:40 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Heikki Linnakangas <hlinnaka@iki.fi> writes:

Index AM's can cache stuff in RelationData->rd_amcache. In the zedstore
table AM we've been hacking on, I'd like to also use rd_amcache to cache
some information, but that's not currently possible, because rd_amcache
can only be used by index AMs, not table AMs.
Attached patch allows rd_amcache to also be used by table AMs.

Seems reasonable.

+1.

In the patch, I documented that rd_amcache must be allocated in
CacheMemoryContext, or in rd_indexcxt if it's an index. It works, but
it's a bit weird.

Given the way the patch is implemented, it doesn't really matter which
context it's in, does it? The retail pfree is inessential but also
harmless, if rd_amcache is in rd_indexcxt. So we could take out the
"must". I think it's slightly preferable to use rd_indexcxt if available,
to reduce the amount of loose junk in CacheMemoryContext.

I agree that for indexes the context used won't make much difference.
But IMHO avoiding some bloat in CacheMemoryContext is a good enough
reason to document using rd_indexcxt when available.

It would nice to have one memory context in every
relcache entry, to hold all the stuff related to it, including
rd_amcache. In other words, it would be nice if we had "rd_indexcxt" for
tables, too, not just indexes. That would allow tracking memory usage
more accurately, if you're debugging an out of memory situation for example.

We had some discussion related to that in the "hyrax
vs. RelationBuildPartitionDesc" thread. I'm not quite sure where
we'll settle on that, but some redesign seems inevitable.

There wasn't any progress on this since last month, and this patch
won't make the situation any worse. I'll mark this patch as ready for
committer, as it may save some time for people working on custom table
AM.

#4Heikki Linnakangas
heikki.linnakangas@enterprisedb.com
In reply to: Julien Rouhaud (#3)
Re: Allow table AM's to cache stuff in relcache

On 12/07/2019 16:07, Julien Rouhaud wrote:

On Fri, Jun 14, 2019 at 5:40 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Heikki Linnakangas <hlinnaka@iki.fi> writes:

In the patch, I documented that rd_amcache must be allocated in
CacheMemoryContext, or in rd_indexcxt if it's an index. It works, but
it's a bit weird.

Given the way the patch is implemented, it doesn't really matter which
context it's in, does it? The retail pfree is inessential but also
harmless, if rd_amcache is in rd_indexcxt. So we could take out the
"must". I think it's slightly preferable to use rd_indexcxt if available,
to reduce the amount of loose junk in CacheMemoryContext.

I agree that for indexes the context used won't make much difference.
But IMHO avoiding some bloat in CacheMemoryContext is a good enough
reason to document using rd_indexcxt when available.

Right, it doesn't really matter whether an index AM uses
CacheMemoryContext or rd_indexctx, the code works either way. I think
it's better to give clear advice though, one way or another. Otherwise,
different index AM's can end up doing it differently for no particular
reason, which seems confusing.

It would nice to have one memory context in every
relcache entry, to hold all the stuff related to it, including
rd_amcache. In other words, it would be nice if we had "rd_indexcxt" for
tables, too, not just indexes. That would allow tracking memory usage
more accurately, if you're debugging an out of memory situation for example.

We had some discussion related to that in the "hyrax
vs. RelationBuildPartitionDesc" thread. I'm not quite sure where
we'll settle on that, but some redesign seems inevitable.

There wasn't any progress on this since last month, and this patch
won't make the situation any worse. I'll mark this patch as ready for
committer, as it may save some time for people working on custom table
AM.

Pushed, thanks for the review! As Tom noted, some redesign here seems
inevitable, but this patch shouldn't get in the way of that, so no need
to hold this back for the redesign.

- Heikki