On Mon, Sep 14, 2020 at 5:50 PM Jeff Davis <pg...@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
