| 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: | Whole Thread | Raw Message | 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
| 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. |