From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, pgsql-committers <pgsql-committers(at)postgresql(dot)org> |
Subject: | Re: pgsql: Add support for coordinating record typmods among parallel worke |
Date: | 2017-09-15 18:14:36 |
Message-ID: | 20170915181436.u6j27rx65giyntmk@alap3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers pgsql-hackers |
On 2017-09-15 10:33:37 -0400, Tom Lane wrote:
> Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Eisentraut | 2017-09-15 18:29:52 | pgsql: Apply pg_get_serial_sequence() to identity column sequences as w |
Previous Message | Andres Freund | 2017-09-15 18:07:08 | Re: pgsql: Add support for coordinating record typmods among parallel worke |
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2017-09-15 18:23:11 | Re: SendRowDescriptionMessage() is slow for queries with a lot of columns |
Previous Message | Jeff Janes | 2017-09-15 18:09:13 | Re: CLUSTER command progress monitor |