[MERGE] Faster LocalTransport.put() and append() methods.

[Martin Pool]

On 14/06/2006, at 7:16 PM, Robert Collins wrote:

> On Wed, 2006-06-14 at 13:36 +1000, Martin Pool wrote:
>
>> Since you're creating the file with O_EXCL I don't see any need to
>> include the PID.  Instead, if that fails with EEXIST then just e.g.
>> add a sequence number and try again.
>
> O_EXCL is documented as broken on NFS. if use a static suffix - i.e.
> filename + '.bzrtmp', if someone has a file that looks like that,  
> we may
> overwrite it unintentionally - *depending on the manner of the
> brokenness for NFS*.

But you may overwrite something called foo.$pid.tmp too. ??

> If its 'race conditions with two clients writing the same file'  
> broken -
> then I'm happy to remove the pid. If its 'O_EXCL is simply ignored on
> some implementations' then I think the pid is a not-too-bad price to
> pay.

I think your patch had the problem that if the file happens to exist  
(e.g. a previous run crashed) then the open will fail, which would be  
unfortunate.  Including the pid makes it less likely but it can still  
happen.  So you do need to catch that case, and I would suggest just  
retrying.

On local filesystems, O_EXCL will be safe.  On NFS, even including  
the pid is not enough (and I don't think the atomic rename is  
guaranteed either).  So I don't see what the pid really gains you.

> And thread safety may be an issue too. I think calculating the umask
> when the module is first used, and then chmodding IFF its needed is a
> reasonable compromise.

That's OK with me.

-- 
Martin