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

From: Peter Geoghegan <pg(at)bowt(dot)ie>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, Rushabh Lathia <rushabh(dot)lathia(at)gmail(dot)com>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Corey Huinker <corey(dot)huinker(at)gmail(dot)com>
Subject: Re: [HACKERS] Parallel tuplesort (for parallel B-Tree index creation)
Date: 2018-02-06 18:33:45
Views: Raw Message | Whole Thread | Download mbox
Lists: pgsql-hackers

On Mon, Feb 5, 2018 at 1:45 PM, Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
>> So, I guess another option might be to call VALGRIND_MAKE_MEM_DEFINED
>> on the buffer. "We know what we're doing, trust us!"
>> In some ways, that seems better than inserting a suppression, because
>> it only affects the memory in the buffer.
> I think that that would also work, and would be simpler, but also
> slightly inferior to using the proposed suppression. If there is
> garbage in logtape.c buffers, we still generally don't want to do
> anything important on the basis of those values. We make one exception
> with the suppression, which is a pretty typical kind of exception to
> make -- don't worry if we write() poisoned bytes, since those are
> bound to be alignment related.
> OTOH, as I've said we are generally bound to write some kind of
> logtape.c garbage, which will almost certainly not be of the
> uninitialized memory variety. So, while I feel that the suppression is
> better, the advantage is likely microscopic.

Attached patch does it to the tail of the buffer, as Tom suggested on
the -committers thread.

Note that there is one other place in logtape.c that can write a
partial block like this: LogicalTapeRewindForRead(). I haven't
bothered to do anything there, since it cannot possibly be affected by
this issue for the same reason that serial sorts cannot be -- it's
code that is only used by a tuplesort that really needs to spill to
disk, and merge multiple runs (or for tapes that have already been
frozen, that are expected to never reallocate logtape.c buffers).

Peter Geoghegan

Attachment Content-Type Size
0001-Mark-logtape.c-buffer-s-tail-as-defined-to-Valgrind.patch text/x-patch 2.0 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2018-02-06 18:50:28 Re: Query running for very long time (server hanged) with parallel append
Previous Message Andres Freund 2018-02-06 17:06:09 Re: Cancelling parallel query leads to segfault