Re: partitioning code reorganization - Mailing list pgsql-hackers

From Alvaro Herrera
Subject Re: partitioning code reorganization
Date
Msg-id 20180417201850.geekn5pxceho5ti2@alvherre.pgsql
Whole thread Raw
In response to Re: partitioning code reorganization  (Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>)
Responses Re: partitioning code reorganization
List pgsql-hackers
Amit Langote wrote:

> 0001-Make-copying-of-cached-partitioning-info-more-con.patch
> 0002-Cache-all-partitioning-info-under-one-memory-cont.patch
> 0003-Cache-partsupfunc-separately-from-PartitionKey.patch

I'd rather not do these patches now, unless there is some pressing
reason to (eg. some bug crops up).  I know Tom dislikes some of the code
being used for relcache, but I don't know of any actual bugs; my
proposal is put these patches to sleep until pg12 opens.

> I think going the way of 0001 might seem counter to what we may *really*
> want to do in this regard, which is to make it so that we can use (keep
> around) the pointer to partition info in relcache, instead of copying the
> information in it piece by piece for every query.  Robert's email from a
> couple of months ago (that he also recently mentioned) brought this up wrt
> to relcache data usage within the planner:

I hadn't seen that discussion, thanks.

Some braindump follows.

Re 0002 though I didn't carefully review the patch, I worry that an
error partway some of these routines would leave you with either a
long-lived memory leak (or permanent!), or dangling pointers in the
relcache entry.  If that is so, you can probably fix it by having a
memcxt callback that resets the other partition-related members from the
relcache entry being processed, so that they are rebuilt; then reset the
memcxt. (make the memcxt always exist, for simplicity).

The other thing I was interested in was to build the partitioning info
on demand rather than immediately when the relcache entry is created.
I think there must be tons of places where we want the relcache entry
for other reasons and we don't want to waste time building partition
desc/key uselessly.

Another line of thought.  Instead of copying anything, did you consider
the idea of using refcounted structs?  Rough sketch: in
RelationGetPartitionDesc, build struct if relcache doesn't have one,
otherwise just increment pincount in existing struct; create new
ReleasePartitionDesc which decrements pincount, and if pincount is 0 and
not referenced from relcache entry, free the struct (memcxt delete?); if
relcache inval is received, detach partitiondesc from relcache entry and
build a new one.  So old pointers continue to work, until the release.
Maybe need to get ResourceOwner involved to make sure we don't
accidentally lose track of a PartitionDesc with pincout != 0.

This is all pg12 anyway.

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


pgsql-hackers by date:

Previous
From: Heikki Linnakangas
Date:
Subject: Re: pgsql: Store 2PC GID in commit/abort WAL recs for logicaldecoding
Next
From: Jonathan Rudenberg
Date:
Subject: Re: [sqlsmith] Unpinning error in parallel worker