Re: Hot standby, RestoreBkpBlocks and cleanup locks

On Fri, 2009-01-09 at 18:30 +0200, Heikki Linnakangas wrote:

> The hot standby patch has some hacks to decide which full-page-images 
> can be restored holding an exclusive lock and which ones need a 
> vacuum-strength lock. It's not very pretty as is, as mentioned in 
> comments too.

Agreed!

> How about we refactor things so that redo-functions are responsible for 
> calling RestoreBkpBlocks? The redo function can then pass an argument 
> indicating what kind of lock is required. We should also change 
> XLogReadBufferExtended so that it doesn't lock the page; the caller 
> knows better what kind of a lock it needs. That makes it more analogous 
> with ReadBufferExtended too, although I think we should keep 
> XLogReadBuffer() unchanged for now.

Much better idea, thanks. I felt a new rmgr function was overkill but
couldn't think of how to do this.

> See attached patch. One shortfall of this patch is that you can pass 
> only one argument to RestoreBkpBlocks, but there can multiple backup 
> blocks in one WAL record. That's OK in current usage, though.

If we're doing this because of cleanup locks, then I'd say we don't
currently need a cleanup lock with any WAL record that uses multiple
backup blocks. So we can just document that so anybody adding such a
record in the future will be careful.

So all seems good.

Would you want to push ResolveRedoVisibilityConflicts() down into the
rmgrs as well and make reachedSafeStartPoint a global? That is only
called for cleanup records.

-- Simon Riggs           www.2ndQuadrant.comPostgreSQL Training, Services and Support