On Mon, Sep 14, 2020 at 5:50 PM Jeff Davis <pgsql@j-davis.com> wrote:
> Yes, it was apparently an oversight. Patch attached.
This is closer to how logical tapes are used within tuplesort.c. I
notice that this leads to about a 50% reduction in temp file usage for
a test case involving very little work_mem (work_mem is set to 64).
But it doesn't seem to make as much difference with more work_mem. It
probably has something to do with recursion during spilling.
> RC1 was just stamped, are we in a sensitive time or is it still
> possible to backport this to REL_13_STABLE?
Testing indicates that this still doesn't make "nBlocksWritten ==
nBlocksAllocated" when the instrumentation is used for
HashAggs-that-spill.
I'm not sure what I was talking about earlier when I connected this
with the main/instrumentation issue, since preallocation used by
logtape.c to help HashAggs-that-spill necessarily reserves blocks
without writing them out for a while (the fires in California have
made it difficult to be productive). You might write blocks out as
zero blocks first, and then only write the real data later
(overwriting the zero blocks). But no matter how the writes among
tapes are interlaced, the fact is that nBlocksAllocated can exceed
nBlocksWritten by at least one block per active tape.
If we really wanted to ensure "nBlocksWritten == nBlocksAllocated",
wouldn't it be necessary for LogicalTapeSetBlocks() to go through the
remaining preallocated blocks from each tape and count the number of
blocks "logically preallocated" (by ltsGetPreallocBlock()) but not yet
"physically preallocated" (by being written out as zero blocks within
ltsWriteBlock())? That count would have to be subtracted, because
nBlocksAllocated includes logically preallocated blocks, without
regard for whether they've been physically preallocated. But we only
know the difference by checking against nBlocksWritten, so we might as
well just use my patch from earlier. (I'm not arguing that we should,
I'm just pointing out the logical though perhaps absurd conclusion.)
--
Peter Geoghegan
