Re: pgsql: Change the way pre-reading in external sort's merge phase works.

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: Peter Geoghegan <pg(at)heroku(dot)com>
Cc: pgsql-committers <pgsql-committers(at)postgresql(dot)org>
Subject: Re: pgsql: Change the way pre-reading in external sort's merge phase works.
Date: 2016-10-06 06:24:05
Message-ID: 76571c58-bd72-eb61-6618-cb6521b12c12@iki.fi
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

On 10/06/2016 04:23 AM, Peter Geoghegan wrote:
> I noticed a simple oversight in this patch. It looks like you missed
> one place where state->maxTapes ought to be replaced with
> numInputTapes -- the loop that calls LogicalTapeAssignReadBufferSize()
> needs that changed too, in order to continue to respect workMem as a
> budget.

Nope. See the comment above that loop:

> /*
> * Set the buffers for the tapes.
> *
> * In a multi-phase merge, the tape that is initially used as an output
> * tape, will later be rewound and read from, and should also use a large
> * buffer at that point. So we must loop up to maxTapes, not just
> * numInputTapes!
> *
> * If there are fewer runs than tapes, we will set the buffer size also
> * for tapes that will go completely unused, but that's harmless.
> * LogicalTapeAssignReadBufferSize() doesn't allocate the buffer
> * immediately, it just sets the size that will be used, when the tape is
> * rewound for read, and the tape isn't empty.
> */
> for (tapenum = 0; tapenum < state->maxTapes; tapenum++)
> {
> int64 numBlocks = blocksPerTape + (tapenum < remainder ? 1 : 0);
>
> LogicalTapeAssignReadBufferSize(state->tapeset, tapenum,
> numBlocks * BLCKSZ);
> }

- Heikki

In response to

Browse pgsql-committers by date

  From Date Subject
Next Message Heikki Linnakangas 2016-10-06 06:52:06 pgsql: Fix excessive memory consumption in the new sort pre-reading cod
Previous Message Tom Lane 2016-10-06 03:04:50 pgsql: Remove -Wl,-undefined,dynamic_lookup in macOS build.