branch locking mk2.

[John A Meinel]

Robey Pointer wrote:
> 
> On 14 Feb 2006, at 7:08, John A Meinel wrote:
> 
>> Robey Pointer wrote:
>>>
>>> After staring sideways at fancy_rename, I think this is high-level
>>> functionality that should *use* Transport instead of being *used by*
>>> Transport.  I think instead of passing in rename_func & unlink_func,
>>> fancy_rename really just wants to be passed a Transport which knows how
>>> to do these lower-level operations.  We could call it osutils.replace
>>> (or something) and not have it in Transport at all.  Does that make
>>> sense?
>>
>> The whole reason fancy_rename exists is because on Windows and sftp you
>> cannot rename a file over an existing file. And we had been depending on
>> "os.rename" to support atomic replace like that.
>> Transport.move() was hijacked, because it was effectively os.rename, and
>> we were expecting that behavior.
>>
>> I suppose we could audit the code, and everywhere we want 'rename/move'
>> we switch to 'replace' to make it clear what we are doing.
>>
>> I *don't* think we want to make fancy_rename a higher level than
>> transport. Because then all the clients would need to know if they are
>> using a Transport (or platform) which doesn't support atomic rename, and
>> then they need to activate the fancy rename.
>>
>> It is just meant to give posix-like semantics to the rename function.
>> Obviously it has a failing in that it will move directories around. It
>> was never designed to be used on directories, which probably needs to be
>> corrected.
>>
>> The issue is that simple things like AtomicFile, expect to be able to
>> replace an existing file. Its how we replace all weaves, everything.
>>
>> I don't really care how it is done. But I think people need to realize
>> why fancy_rename exists, and where it is actually used. (It is only used
>> on linux for sftp support, though that also effects the locking issues
>> as well).
>>
>> I really think that we should try to standardize functionality across
>> platforms. If we want a rename which *doesn't* replace, then we should
>> force it to check if the target exists on Linux. And then we can have
>> another function which does replace.
>> Yes there is a race condition there, but we have a race condition for
>> 'replace' on Windows.
> 
> I guess there are a few things going on here.
> 
> The name "fancy_rename" implies (to me) something like rename, but with
> added magic.  It sounds like it's really just an attempt to implement
> "atomic_rename" of files for Transports that don't have it.  Having it
> in osutils further implies that it's related to the local OS.
> 
> What if we moved it into Transport.__init__.py and called it
> "atomic_rename", to emphasize that it's really a base implementation for
> Transports that don't have an atomic rename?  Then Transport.rename (or
> .replace) could promise to always be atomic.
> 
> Another issue is that if file-renaming isn't atomic on some platforms,
> and we're relying on atomic behavior, this seems... bad.  I don't have
> an answer.  But it troubles me.  Over SFTP, you can't even guarantee
> that the race is short -- we may get disconnected from the server in the
> middle of the atomic_rename, and now there's no weave file.  Or the user
> may just have hit ^C.  Scary.  Will the append-only knit stuff make this
> a non-issue?
> 
> robey

Append only weaves would make it much less of an issue, since we would
append to the files that exist, rather than replacing them with new ones.

There will probably be some control files that are still atomically
replaced, like revision-history (as long as that exists).
Also, I don't know how we are trying to keep knit index files intact.
I'm guessing we just expect that adding 1 line will be an atomic action,
as long as we add less than X number of bytes.
I can think of some things we could do (like adding a checksum +
line-length at the beginning of each line, along with a line delimiter
before the line (a simple newline could do)).
Then we can throw out index lines that don't match their checksum. And
the delimiter makes sure that we know where the entries start & stop.

John
=:->


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 249 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060215/c7468fce/attachment.pgp