Re: pgsql: Add support for coordinating record typmods among parallel worke

From: Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andres Freund <andres(at)anarazel(dot)de>, pgsql-committers <pgsql-committers(at)postgresql(dot)org>
Subject: Re: pgsql: Add support for coordinating record typmods among parallel worke
Date: 2017-09-15 20:56:45
Message-ID: CAEepm=09Z-H1_C9g-eXWcPig1NH_rtfjOH73FjD4JxM5b7yz0A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

On Sat, Sep 16, 2017 at 7:07 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
>> On 2017-09-15 10:33:37 -0400, Tom Lane wrote:
>>> I don't much like your proposed comment; the only way that this code
>>> is even approximately correct is if we're exiting the process and
>>> will never touch the RecordCacheArray again. (Otherwise, it risks
>>> reassigning a previously used local typmod.)
>
>> How'd it reuse it after running the detach hook in workers?
>
> Resetting NextRecordTypmod to zero means that we could reuse a typmod
> that was previously used. Maybe that's safe because no memory of the
> old record definition exists anywhere else in the process, but I'm
> not exactly convinced of that. For that matter, I'm not exactly
> convinced that nothing else is holding on to an actual pointer to the
> record tupdesc, making it moot whether or not we flush typcache.c's
> pointer. (I'm pretty sure that plpgsql, for one, might hold onto
> tupdesc pointers it gets from the typcache. That's why they have
> refcounts in the first place.) There's never previously been any
> expectation that record typmod registrations weren't good for the life
> of the process, so I don't trust any of the assumptions this code
> wants to make.
>
> It's moot as long as we're not reusing workers, anyway: nothing is
> going to touch any of the record-tupdesc data before process exit.

Thanks for straightening this out. I think that what I was trying to
do (bugs aside) is probably the right direction, but you're quite
right that there are plenty more problems to solve before it could
actually work, so my forward looking but incomplete code looks a bit
goofy in hindsight. I have this vague notion that we should
eventually hunt down and separate all forms of caching that are
dependent on the current session (references to session-specific
typmods being the only example I can think of right now) and separate
them from caching that is OK between different sessions (since reused
backends might as well benefit from prewarmed caches where it makes
sense). For example, if plpgsql is holding onto RECORD TupleDescs or
typmods it would need to stop doing that when it receives a
session-going-away callback, or store its state in some data structure
connected to the current session, or <insert hand waving>.

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

In response to

Browse pgsql-committers by date

  From Date Subject
Next Message Alvaro Herrera 2017-09-15 22:40:43 pgsql: src/test/ldap: Fix test function in Linux port
Previous Message Andres Freund 2017-09-15 19:16:16 Re: pgsql: Add support for coordinating record typmods among parallel worke

Browse pgsql-hackers by date

  From Date Subject
Next Message Nico Williams 2017-09-15 21:03:35 Re: COMMIT TRIGGERs, take n, implemented with CONSTRAINT TRIGGERS
Previous Message Andres Freund 2017-09-15 20:49:10 Re: More efficient truncation of pg_stat_activity query strings