Horton

Re: WIP: [[Parallel] Shared] Hash

Thomas Munro <thomas.munro@enterprisedb.com>

View thread View on postgr.es
From: Thomas Munro <thomas.munro@enterprisedb.com>
To: Peter Geoghegan <pg@bowt.ie>
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-26T02:56:13Z
Lists: pgsql-hackers 
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.

But... what you said above must be a problem for Windows.  I believe
it doesn't allow files to be unlinked if they are open, and I see that
DSM segments are cleaned up in resowner's phase ==
RESOURCE_RELEASE_BEFORE_LOCKS and files are closed in phase ==
RESOURCE_RELEASE_AFTER_LOCKS.

Hmm.

-- 
Thomas Munro
http://www.enterprisedb.com

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