Horton

Re: WIP: [[Parallel] Shared] Hash

Peter Geoghegan <pg@bowt.ie>

View thread View on postgr.es
From: Peter Geoghegan <pg@bowt.ie>
To: Thomas Munro <thomas.munro@enterprisedb.com>
Cc: Rafia Sabih <rafia.sabih@enterprisedb.com>, Andres Freund <andres@anarazel.de>, Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>, Haribabu Kommi <kommi.haribabu@gmail.com>, Pg Hackers <pgsql-hackers@postgresql.org>, Robert Haas <robertmhaas@gmail.com>
Date: 2017-03-26T05:33:51Z
Lists: pgsql-hackers 
On Sat, Mar 25, 2017 at 7:56 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
> On Sun, Mar 26, 2017 at 1:53 PM, Peter Geoghegan <pg@bowt.ie> wrote:
>> ISTM that your patch now shares a quality with parallel tuplesort: You
>> may now hold files open after an unlink() of the original link/path
>> that they were opened using. As Robert pointed out when discussing
>> parallel tuplesort earlier in the week, that comes with the risk,
>> however small, that the vFD cache will close() the file out from under
>> us during LRU maintenance, resulting in a subsequent open() (at the
>> tail-end of the vFD's lifetime) that fails unexpectedly. It's probably
>> fine to assume that we can sanely close() the file ourselves in fd.c
>> error paths despite a concurrent unlink(), since we never operate on
>> the link itself, and there probably isn't much pressure on each
>> backend's vFD cache. But, is that good enough? I can't say, though I
>> suspect that this particular risk is one that's best avoided.
>>
>> I haven't tested out how much of a problem this might be for your
>> patch, but I do know that resowner.c will call your shared mem segment
>> callback before closing any backend local vFDs, so I can't imagine how
>> it could be that this risk doesn't exist.
>
> I wouldn't have expected anything like that to be a problem, because
> FileClose() doesn't call FileAccess().  So IIUC it wouldn't ever try
> to reopen a kernel fd just to close it.

The concern is that something somewhere does. For example, mdread()
calls FileRead(), which calls FileAccess(), ultimately because of some
obscure catalog access. It's very hard to reason about things like
that.

-- 
Peter Geoghegan

Commits

  1. Add shared tuplestores.

  2. Remove BufFile's isTemp flag.

  3. Provide DSM segment to ExecXXXInitializeWorker functions.

  4. Optimize joins when the inner relation can be proven unique.

  5. Fix failure to use clamp_row_est() for parallel joins.

  6. Document lack of validation when attaching foreign partitions.

  7. Fix inclusions of c.h from .h files.

  8. Fix inclusions of postgres_fe.h from .h files.

  9. Bring plpgsql into line with header inclusion policy.

  10. Document intentional violations of header inclusion policy.

  11. Shut down Gather's children before shutting down Gather itself.

  12. btree: Support parallel index scans.

  13. Add explicit ORDER BY to a few tests that exercise hash-join code.

  14. Revise hash join code so that we can increase the number of batches

  15. Rewrite hash join to use simple linked lists instead of a