Re: Why are we PageInit'ing buffers in RelationAddExtraBlocks()? - Mailing list pgsql-hackers

From Andres Freund
Subject Re: Why are we PageInit'ing buffers in RelationAddExtraBlocks()?
Date
Msg-id 20190115204428.kpqmk3ix6cenr7gi@alap3.anarazel.de
Whole thread Raw
In response to Re: Why are we PageInit'ing buffers in RelationAddExtraBlocks()?  (Andres Freund <andres@anarazel.de>)
Responses Re: Why are we PageInit'ing buffers in RelationAddExtraBlocks()?  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
Hi,

On 2018-12-20 15:04:11 -0800, Andres Freund wrote:
> On 2018-12-19 16:56:36 -0800, Andres Freund wrote:
> > On 2018-12-19 19:39:33 -0500, Tom Lane wrote:
> > > Robert Haas <robertmhaas@gmail.com> writes:
> > > > On Wed, Dec 19, 2018 at 5:37 PM Andres Freund <andres@anarazel.de> wrote:
> > > >> What's gained by the logic of emitting that warning in VACUUM after a
> > > >> crash? I don't really see any robustness advantages in it.
> > > 
> > > > I don't know.  I am just normally reluctant to change things
> > > > precipitously that are of long tenure.
> > > 
> > > Me too, but I think Andres has a point here.  Those warnings in VACUUM
> > > are ancient, probably predating the introduction of WAL :-(.  At the
> > > time there was good reason to be suspicious of zeroed pages in tables.
> > > Now, though, we have (what we think is) a bulletproof crash recovery
> > > procedure in which possibly-zeroed pages are to be expected; so we're
> > > just causing users unnecessary alarm by warning about them.  I think
> > > it's reasonable to, if not remove the messages entirely, at least
> > > downgrade them to a low DEBUG level.
> > 
> > Yea, I think that'd be reasonable.
> > 
> > I think we ought to add them, as new/zero pages, to the FSM. If we did
> > so both during vacuum and RelationAddExtraBlocks() we'd avoid the
> > redundant writes, and avoid depending on heap layout in
> > RelationAddExtraBlocks().
> > 
> > I wonder if it'd make sense to only log a DEBUG if there's no
> > corresponding FSM entry. That'd obviously not be bulltproof, but it'd
> > reduce the spammyness considerably.
> 
> Here's a prototype patch implementing this change.  I'm not sure the new
> DEBUG message is really worth it?
> 
> 
> 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?

Greetings,

Andres Freund


pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: MERGE SQL statement for PG12
Next
From: Andrew Gierth
Date:
Subject: Re: Ryu floating point output patch