Re: logtape.c stats don't account for unused "prefetched" block numbers
Peter Geoghegan <pg@bowt.ie>
From: Peter Geoghegan <pg@bowt.ie>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Jeff Davis <pgsql@j-davis.com>,
PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2020-09-02T00:24:27Z
Lists: pgsql-hackers
On Tue, Sep 1, 2020 at 4:36 PM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> This open item hasn't received any replies. I think Peter knows how to
> fix it already, but no patch has been posted ... It'd be good to get a
> move on it.
I picked this up again today.
It's not obvious what we should do. It's true that the instrumentation
doesn't accurately reflect the on-disk temp file overhead. That is, it
doesn't agree with the high watermark temp file size I see in the
pgsql_tmp directory, which is a clear regression compared to earlier
releases (where tuplesort was the only user of logtape.c). But it's
also true that we need to use somewhat more temp file space for a
tuplesort in Postgres 13, because we use the preallocation stuff for
tuplesort -- though probably without getting any benefit for it.
I haven't figured out how to correct the accounting just yet. In fact,
I'm not sure that this isn't some kind of leak of blocks from the
freelist, which shouldn't happen at all. The code is complicated
enough that I wasn't able to work that out in the couple of hours I
spent on it today. I can pick it up again tomorrow.
BTW, this MaxAllocSize freeBlocksLen check is wrong -- doesn't match
the later repalloc allocation:
if (lts->nFreeBlocks >= lts->freeBlocksLen)
{
/*
* If the freelist becomes very large, just return and leak this free
* block.
*/
if (lts->freeBlocksLen * 2 > MaxAllocSize)
return;
lts->freeBlocksLen *= 2;
lts->freeBlocks = (long *) repalloc(lts->freeBlocks,
lts->freeBlocksLen * sizeof(long));
}
--
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