Re: Race condition between hot standby and restoring a FPW

On 11/13/2014 01:06 AM, Jim Nasby wrote:
> On 11/12/14, 9:47 AM, Tom Lane wrote:
>> Heikki Linnakangas <hlinnakangas@vmware.com> writes:
>>> On 11/12/2014 05:20 PM, Tom Lane wrote:
>>>> On reconsideration I think the "RBM_ZERO returns page already locked"
>>>> alternative may be the less ugly.   That has the advantage that any code
>>>> that doesn't get updated will fail clearly and reliably.
>>
>>> Yeah, I'm leaning to that approach as well. It's made more ugly by the
>>> fact that you sometimes need a cleanup lock on the buffer, so the caller
>>> will somehow need to specify whether to get a cleanup lock or a normal
>>> exclusive lock. Maybe add yet another mode, RBM_ZERO_WITH_CLEANUP_LOCK.
>>> Could also rename RBM_ZERO to e.g. RBM_ZERO_AND_LOCK, to make any code
>>> that's not updated to break even more obviously, at compile-time.
>>
>> Yeah, I was considering suggesting changing the name of the mode too.
>> +1 for solving the lock-type problem with 2 modes.
>>
>> (You could also consider leaving RBM_ZERO in place with its current
>> semantics, but I think what you've shown here is that there is no
>> safe way to use it, so probably that's not what we should do.)

Committed that way. I left a placeholder for RBM_ZERO in back-branches, 
so 3rd party extensions using it continue to work - to the extent that 
they did before - without recompiling. In theory, it's possible that 
someone is using RBM_ZERO while holding an AccessExclusiveLock on the 
relation, or there's something else that ensures that no-one else is 
looking at the relation. I doubt there actually are any extensions out 
there using it, but might as well.

> If we're tweaking modes, can we avoid zeroing the buffer if we're about to dump a full page image into it? Presumably
thatmeans we'd have to keep the page locked until the image is written...
 

Hmm. I suppose we could, as long as the caller knows that is has to 
re-initialize the page. That would probably be ok for all current 
callers. I didn't go ahead with that, however; the zeroing isn't that 
expensive and we would need to rename the mode if it didn't zero the 
page anymore, which would cause some code churn.

- Heikki