Andres Freund <and...@anarazel.de> writes: >> Looking at the surrounding code made me wonder about the wisdom of >> entering empty pages as all-visible and all-frozen into the VM. That'll >> mean we'll never re-discover them on a primary, after promotion. There's >> no mechanism to add such pages to the FSM on a standby (in contrast to >> e.g. pages where tuples are modified), as vacuum will never visit that >> page again. Now obviously it has the advantage of avoiding >> re-processing the page in the next vacuum, but is that really an >> important goal? If there's frequent vacuums, there got to be a fair >> amount of modifications, which ought to lead to re-use of such pages at >> a not too far away point?
> Any comments on the approach in this patch? I agree with the concept of postponing page init till we're actually going to do something with the page. However, the patch seems to need some polish: * There's a comment in RelationAddExtraBlocks, just above where you changed, that * Extend by one page. This should generally match the main-line * extension code in RelationGetBufferForTuple, except that we hold * the relation extension lock throughout. This seems to be falsified by this patch, in that one of the two paths does PageInit and the other doesn't. * s/unitialized pages/uninitialized pages/ * This bit in vacuumlazy seems unnecessarily confusing: + Size freespace = 0; ... + if (GetRecordedFreeSpace(onerel, blkno) == 0) + freespace = BufferGetPageSize(buf) - SizeOfPageHeaderData; + + if (freespace > 0) + { + RecordPageWithFreeSpace(onerel, blkno, freespace); I'd write that as just + if (GetRecordedFreeSpace(onerel, blkno) == 0) + { + Size freespace = BufferGetPageSize(buf) - SizeOfPageHeaderData; + RecordPageWithFreeSpace(onerel, blkno, freespace); I tend to agree that the DEBUG message isn't very necessary, or at least could be lower than DEBUG1. regards, tom lane