From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Thomas Munro <thomas.munro@enterprisedb.com>
Cc: pgsql-committers <pgsql-committers@postgresql.org>
Subject: Re: pgsql: Add support for coordinating record typmods among parallel worke
Date: Fri, 15 Sep 2017 11:14:36 -0700
Message-ID: <20170915181436.u6j27rx65giyntmk@alap3.anarazel.de> (raw)
In-Reply-To: <21075.1505486017@sss.pgh.pa.us>
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>
<21075.1505486017@sss.pgh.pa.us>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-committers>
On 2017-09-15 10:33:37 -0400, Tom Lane wrote:
> 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.)
How'd it reuse it after running the detach hook in workers?
> 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.
The problem is that RecordCacheArray[n] can point into shared
memory. Which means that the pushed change leaves around danging
pointers into shared memory - unless I'm missing something (didn't yet
have coffee, had to run to the store).
Greetings,
Andres Freund
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-committers@postgresql.org
Cc: andres@anarazel.de, tgl@sss.pgh.pa.us, thomas.munro@enterprisedb.com
Subject: Re: pgsql: Add support for coordinating record typmods among parallel worke
In-Reply-To: <20170915181436.u6j27rx65giyntmk@alap3.anarazel.de>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox