|From:||Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>|
|To:||Andres Freund <andres(at)anarazel(dot)de>|
|Cc:||Rushabh Lathia <rushabh(dot)lathia(at)gmail(dot)com>, Prabhat Sahu <prabhat(dot)sahu(at)enterprisedb(dot)com>, Peter Geoghegan <pg(at)bowt(dot)ie>, Robert Haas <robertmhaas(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Rafia Sabih <rafia(dot)sabih(at)enterprisedb(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>, Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>, Oleg Golovanov <rentech(at)mail(dot)ru>|
|Subject:||Re: [HACKERS] Parallel Hash take II|
|Views:||Raw Message | Whole Thread | Download mbox|
On Sat, Dec 2, 2017 at 3:46 PM, Andres Freund <andres(at)anarazel(dot)de> wrote:
> Looking at 0004-Add-shared-tuplestores.patch
> - I'd rename mutex to lock. Seems quite possible that we end up with shared
> lockers too.
> - Expand "Simple mechanism for sharing tuples between backends." intro
> comment - that doesn't really seem like a meaningful description of
> the mechanism. Should probably mention that it's similar to
> tuplestores etc...
> - I'm still concerned about the chunking mechanism. How about this
> sketch of an alternative solution?
> Chunks are always the same length. To avoid having to read the length
> from disk while holding a lock, introduce continuation chunks which
> store the amount of space needed by the overlarge tuple at the
> start. The reading process stays largely the same, except that if a
> backend reads a chunk that's a continuation, it reads the length of
> the continuation and skips ahead. That could mean that more than one
> backend read continuation chunks, but the window is small and there's
> normally not goign to be that many huge tuples (otherwise things are
> slow anyway).
I've also included a simple test harness that can be used to drive
SharedTuplestore independently of Parallel Hash, but that patch is not
for commit. See example of usage in the commit message.
(Incidentally I noticed that ParallelWorkerNumber is not marked
PGDLLIMPORT so that fails to build on Windows CI.)
> - why are we using a separate hardcoded 32 for sts names? Why not just
> go for NAMEDATALEN or such?
> - I'd replace most of the "should's" in comments with "need".
Another problem I discovered is that v27's way of installing a
different function into ExecProcNode in ExecInitHashJoin() was broken:
it didn't allow for the possibility that there is no DSA area
available due to lack of resources. ExecInitNode() is too soon to
decide. My solution is to provide a way for executor nodes to change
their ExecProcNode functions at any later time, which requires a way
for execProcnode.c to redo any wrapper functions.
|Next Message||Rushabh Lathia||2017-12-12 10:09:29||Re: [HACKERS] Parallel tuplesort (for parallel B-Tree index creation)|
|Previous Message||Amit Langote||2017-12-12 09:43:57||Re: Boolean partitions syntax|