Re: [HACKERS] Parallel tuplesort (for parallel B-Tree index creation)

On Tue, Nov 14, 2017 at 1:41 AM, Rushabh Lathia
<rushabh(dot)lathia(at)gmail(dot)com> wrote:
> Thanks Peter and Thomas for the review comments.

No problem. More feedback:

* I don't really see much need for this:

+ elog(LOG, "Worker for create index %d", parallel_workers);

You can just use trace_sort, and observe the actual behavior of the
sort that way.

* As I said before, you should remove the header comments within nbtsort.c.

* This should just say "write routines":

+ * This is why write/recycle routines don't need to know about offsets at
+ * all.

* You didn't point out the randomAccess restriction in tuplesort.h.

* I can't remember why I added the Valgrind suppression at this point.
I'd remove it until the reason becomes clear, which may never happen.
The regression tests should still pass without Valgrind warnings.

* You can add back comments removed from above LogicalTapeTell(). I
made these changes because it looked like we should close out the
possibility of doing a tell during the write phase, as unified tapes
actually would make that hard (no one does what it describes anyway).
But now, unified tapes are a distinct case to frozen tapes in a way
that they weren't before, so there is no need to make it impossible.

I also think you should replace "Assert(lt->frozen)" with
"Assert(lt->offsetBlockNumber == 0L)", for the same reason.

--
Peter Geoghegan