Re: logtape.c stats don't account for unused "prefetched" block numbers

On Tue, Sep 1, 2020 at 4:36 PM Alvaro Herrera <alvherre(at)2ndquadrant(dot)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