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