Re: StandbyAcquireAccessExclusiveLock doesn't necessarily

On 8 September 2018 at 00:37, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Commit 37c54863c removed the code in StandbyAcquireAccessExclusiveLock
> that checked the return value of LockAcquireExtended. AFAICS this was
> flat out wrong, because it's still passing reportMemoryError = false
> to LockAcquireExtended, meaning there are still cases where
> LOCKACQUIRE_NOT_AVAIL will be returned. In such a situation, the
> startup process would believe it had acquired exclusive lock even
> though it hadn't, with potentially dire consequences.
>
> While we could certainly put back a test there, it's not clear to me
> that it could do anything more useful than erroring out, at least
> not without largely reverting 37c54863c.
>
> So my inclination is to remove the reportMemoryError = false parameter,
> and just let an error happen in the unlikely situation that we hit OOM
> for the lock table.
>
> That would allow this code to not use LockAcquireExtended at all.
> Indeed, I'd be rather tempted to remove that parameter from
> LockAcquireExtended altogether, as I don't believe it's either
> particularly useful, or at all well tested, or even testable.
>

I've never seen an out of memory on the lock table and that seems even less
likely since changes in 9.2.

So no problem removing that.

Are you looking for a patch to backpatch, or just a change for the future?
Changing the parameter in a backpatch seems more trouble than its worth.

--
Simon Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/>
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services