On Fri, Sep 4, 2009 at 7:44 AM, C. Michael Pilato<cmpilato_at_collab.net> wrote:
> Julian Foad wrote:
>> <http://subversion.tigris.org/issues/show_bug.cgi?id=2507>
>>
>> This is a problem with repository locks on deleted paths. I would like
>> to progress this issue because it bites our customers quite often and
>> although work-arounds are possible it is quite a nuisance.
>>
>>
>> The problematic behaviour:
>>
>> The repository (FS layer) tries to prevent locking a nonexistent path,
>> but if you delete a locked file and commit with --no-unlock, a
>> repository lock remains at the deleted path.
>>
>> Then the server throws an error if you later try to
>> * delete the parent directory, or
>> * re-create a node at the locked path.
>>
>> Note that while it is theoretically useful to be able to lock a
>> nonexistent path, that is not a supported feature. The Subversion client
>> does not attempt to keep track of locks on paths that do not exist.
>>
>> In issue #2507 another problem is mentioned. If a locked file is moved
>> on the client side, it is then not locked at its new path. The user
>> expects the lock to be moved to the new path. This should be filed as a
>> separate issue as it is independent of this issue (though related) and
>> has complexities of its own.
>>
>>
>> Work-arounds:
>>
>> Nanually remove the lock with "svn unlock --force", and re-try the
>> commit. If several paths are involved, this is unsatisfactory because
>> the user only finds out about one more path for each failed commit, by
>> looking at the error message.
>>
>> Avoid using "--no-unlock". Unsatisfactory if the commit contains other
>> files that are not being deleted.
>>
>>
>> Behavioural change required:
>>
>> The files in <notes/locking/> do not mention locking of non-existent
>> paths or specify how --no-unlock should behave with deletes (or moves).
>>
>> I propose the following behaviour.
>>
>> Subversion should, at commit time, even with "--no-unlock",
>> - delete the lock when deleting or moving a locked file;
>> - fail the commit if the lock deletion is refused by the server.
>>
>>
>> Work required:
>>
>> The behaviour should be enforced in libsvn_fs. In the issue, C-Mike says
>> that appears not to be easy.
>>
>> The behaviour could perhaps be implemented in the client side. Needs
>> investigation to see whether that would be practical or useful.
>>
>>
>> Comments?
>
> What if the lock deletion succeeds, but the commit fails? Re-lock, hoping
> that succeeds and that someone else hasn't already locked the file in the
> meantime? Essentially, you'd like to guarantee atomicity across two
> operations, but you haven't the framework to pull it off.
>
> As a matter of policy, we decided to disallow locks of non-existent paths in
> the repository. The FS layer is failing to maintain this policy in this
> (and apparently the move, too) scenario. That's why my instinct is to fix
> it in the FS layer. I ... just ... don't ... know ... how ... yet.
How about doing it in the opposite way?
Instead of making sure that the locks get cleaned up atomically with
commits (which as you say is hard), can't we just ignore locks on
paths which don't exist? (This is focused more on the deletion than
the move case, maybe.) That is, make attempts to ADD locked paths or
to delete the parent directory of a locked nonexistent (as of the root
in question) path not fail.
Problems here include:
- The lock then ends up existing and relevant in the "re-create
locked path" case. We could make the server break the lock at this
point, or we could just make people force unlock, which is certainly
easier on an existing file.
- What happens if you try to delete A/B_at_100 when a lock exists on
A/B/C and A/B/C was created in r102? Well, I'd argue that this isn't
a problem because you wouldn't be able to delete the not-up-to-date
A/B anyway.
--dave
--
glasser_at_davidglasser.net | langtonlabs.org | flickr.com/photos/glasser/
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2392706
Received on 2009-09-09 02:25:25 CEST