Re: logtape.c stats don't account for unused "prefetched" block numbers
Peter Geoghegan <pg@bowt.ie>
From: Peter Geoghegan <pg@bowt.ie>
To: Jeff Davis <pgsql@j-davis.com>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2020-09-15T01:56:50Z
Lists: pgsql-hackers
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
Commits
-
Change LogicalTapeSetBlocks() to use nBlocksWritten.
- 6e146a663536 13.0 landed
- c8aeaf3ab31e 14.0 landed
-
HashAgg: release write buffers sooner by rewinding tape.
- 42a46f5a76ec 13.0 landed
- 3bd35d4f516a 14.0 landed
-
logtape.c: do not preallocate for tapes when sorting
- 93106d71a18a 13.0 landed
- 075896496394 14.0 landed
-
Fix bogus MaxAllocSize check in logtape.c.
- 4a4f3bf983b4 13.0 landed
- 0852006a946a 14.0 landed
-
Avoid fragmentation of logical tapes when writing concurrently.
- 896ddf9b3cd7 13.0 cited
-
Logical Tape Set: use min heap for freelist.
- c02fdc922301 13.0 cited
-
Don't create "holes" in BufFiles, in the new logtape code.
- 7ac4a389a7db 10.0 cited