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

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>
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 14:33:37
Message-ID: 21075.1505486017@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

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

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

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2017-09-15 14:52:35 pgsql: Get rid of shared_record_typmod_registry_worker_detach; it doesn
Previous Message Robert Haas 2017-09-15 12:19:42 pgsql: Test coverage for CREATE/ALTER FOREIGN DATA WRAPPER .. HANDLER.

Browse pgsql-hackers by date

  From Date Subject
Next Message Bossart, Nathan 2017-09-15 14:36:33 Re: Support to COMMENT ON DATABASE CURRENT_DATABASE
Previous Message Daniel Gustafsson 2017-09-15 14:19:18 Re: Statement-level rollback