From: Tom Lane <tgl@sss.pgh.pa.us>
To: Thomas Munro <thomas.munro@enterprisedb.com>
cc: Andres Freund <andres@anarazel.de>,
        pgsql-committers <pgsql-committers@postgresql.org>
Subject: Re: pgsql: Add support for coordinating record typmods among parallel
 worke
In-reply-to: 
 <CAEepm=0y78KA-UFVA=q8joA4m9tPre5y7VxMy2Jzigo+O6E6MA@mail.gmail.com>
References: <E1dsguX-00056N-9x@gemulon.postgresql.org>
 <13971.1505445491@sss.pgh.pa.us>
 <CAEepm=08r6ssWnM7wOAN=omz=s9DHujsJjJY6SN_PTHayPR0hw@mail.gmail.com>
 <14932.1505447120@sss.pgh.pa.us>
 <CAEepm=13KfNVsjzvdz_u4E_3_joAWfL+k6TNaFkr-=WTT_Ou=A@mail.gmail.com>
 <CAEepm=2q2OoJ5K1Mg2_XaqjSSRbMTAmE4V7fsM5O3yTWWkb2aw@mail.gmail.com>
 <29888.1505451575@sss.pgh.pa.us>
 <CAEepm=0y78KA-UFVA=q8joA4m9tPre5y7VxMy2Jzigo+O6E6MA@mail.gmail.com>
Comments: In-reply-to Thomas Munro <thomas.munro@enterprisedb.com>
	message dated "Fri, 15 Sep 2017 18:47:10 +1200"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <21074.1505486017.1@sss.pgh.pa.us>
Date: Fri, 15 Sep 2017 10:33:37 -0400
Message-ID: <21075.1505486017@sss.pgh.pa.us>
Precedence: bulk
Sender: pgsql-committers-owner@postgresql.org

Thomas Munro <thomas.munro@enterprisedb.com> writes:
> Yeah.  The regress tests in d36f7efb39e1b9613193b2e12717dbe2418ddae5
> tested this stuff in parallel mode, but only a happy case to
> demonstrate blessed RECORD types travelling through tqueue.c correctly
> before and after ripping out the remapping stuff.  The problem here
> was that I do some cleanup in the DSM detach hook of the new session
> segment, and that involved reading data that could point to another
> segment (if the DSA area needed more space).  That can blow up if that
> other segment happens to have been unmapped first in the arbitrary
> order of resource cleanup in an aborting worker.  I do have some
> thoughts on how to solve that general problem, but in this case it's
> completely unnecessary anyway.  The attached patch fixes the problem
> by getting rid of the code that accesses shmem in the detach hook.
> With this patch applied installcheck survives on a cluster with
> force_parallel_worker=regress.

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.)

My inclination is to just leave the local data structures alone.
There's nothing we can do to them that doesn't create more problems
than it solves, if the process continues to use the cache.

			regards, tom lane


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