Re: Inadequate thought about buffer locking during hot standby replay

On 2012-11-10 16:24:22 -0500, Tom Lane wrote:
> Simon Riggs <simon@2ndQuadrant.com> writes:
> >> One thing that seems a bit annoying is the use of zero-based backup
> >> block indexes in RestoreBackupBlock, when most (not all) of the callers
> >> are referencing macros with one-based names (XLR_BKP_BLOCK_1 etc).
> >> That's a bug waiting to happen.  We could address it by changing
> >> RestoreBackupBlock to accept a one-based argument, but what I'm thinking
> >> of doing instead is getting rid of the one-based convention entirely;
> >> that is, remove XLR_BKP_BLOCK_1 through XLR_BKP_BLOCK_4 and instead have
> >> all code use the XLR_SET_BKP_BLOCK() macro, which is zero-based.  One
> >> advantage of doing that is that it'll force inspection of all code
> >> related to this.
>
> > I wouldn't do that in a back branch, but I can see why its a good idea.
> 
> If any of this stuff were getting used by external modules, changing it
> would be problematic ... but fortunately, it isn't, because we lack
> support for plug-in rmgrs.  So I'm not worried about back-patching the
> change, and would prefer to keep the 9.x branches in sync.

XLR_BKP_BLOCK_* might be used by things like pg_lesslog and its surely
used by xlogdump. Not sure if either are worth that much attention, but
it seems worth noticing that such a change will break stuff.

Greetings,

Andres Freund

-- 
 Andres Freund	                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services