On Apr 13, 2011, at 2:06 AM, Tom Lane wrote:
> "A.M." <agentm@themactionfaction.com> writes:
>> On Apr 11, 2011, at 7:13 PM, Tom Lane wrote:
>>> Robert Haas <robertmhaas@gmail.com> writes:
>>>> I mean I'm not convinced that fcntl() locking will be as reliable.
>
>>> I'm not either. Particularly not on NFS.
>
>> Is there an example of a recent system where fcntl is broken (ignoring NFS)?
>
> Well, the fundamental point is that "ignoring NFS" is not the real
> world. We can't tell people not to put data directories on NFS,
> and even if we did tell them not to, they'd still do it. And NFS
> locking is not trustworthy, because the remote lock daemon can crash
> and restart (forgetting everything it ever knew) while your own machine
> and the postmaster remain blissfully awake.
>
> None of this is to say that an fcntl lock might not be a useful addition
> to what we do already. It is to say that fcntl can't just replace what
> we do already, because there are real-world failure cases that the
> current solution handles and fcntl alone wouldn't.
The goal of this patch is to eliminate SysV shared memory, not to implement NFS-capable locking which, as you point
out,is virtually impossible.
As far as I can tell, in the worst case, my patch does not change how postgresql handles the NFS case. SysV shared
memorywon't work across NFS, so that interlock won't catch, so postgresql is left with looking at a lock file with PID
ofprocess on another machine, so that won't catch either. This patch does not alter the lock file semantics, but merely
augmentsthe file with file locking.
At least with this patch, there is a chance the lock might work across NFS. In the best case, it can allow for
shared-storagepostgresql failover, which is a new feature.
Furthermore, there is an improvement in shared memory handling in that it is unlinked immediately after creation, so
onlythe postmaster and its children have access to it (through file descriptor inheritance). This means shared memory
cannotbe stomped on by any other process.
Considering that possibly working NFS locking is a side-effect of this patch and not its goal and, in the worst
possiblescenario, it doesn't change current behavior, I don't see how this can be a ding against this patch.
Cheers,
M