You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Ben Collins-Sussman <su...@collab.net> on 2004/12/02 14:27:25 UTC
on-the-fly lock checking
Here's a heads-up for folks following what's going on in the locking
branch.
After a bunch of discussion yesterday, cmpilato and kfogel feel that
it was a mistake to put on-the-fly lock checking directly into public
fs routines. (BDB is doing this now, FSFS isn't quite there yet.)
It turns out that we have a precedent for this, one which I
misunderstood: it's the way in which the filesystem does
out-of-dateness checks. Apparently only svn_fs_commit_txn() actually
does out-of-dateness checks. All of the other on-the-fly
out-of-dateness checks are being done by *servers* -- specifically, in
the commit editors implemented by the servers.
I guess the rationale here is that a libsvn_fs transaction tree is
supposed to represent a "generic scratch space". libsvn_fs isn't in
the business of telling users what they may or may not assemble in
their scratch space; if users want to drop out-of-date files in there
and edit them, that's their own business. Who knows if the user even
plans to commit the transaction? (Remember that in svn 1.0,
transaction trees were created just to do updates!) libsvn_fs only
enforces out-of-dateness if the user tries to merge the tree into
HEAD.
In that same vein, libsvn_fs shouldn't be in the business of
preventing users from editing locked files in their tree. libsvn_fs
only does enforcement of *both* out-of-dateness and locks when the
user tries to commit.
So I'm going to rip the on-the-fly lock checking out of BDB's public
fs funcs, and fitz going to avoid coding that feature in FSFS.
Meanwhile, I'm going to create some libsvn_repos wrappers around the 8
or so fs-write routines. These wrappers will do on-the-fly lock
checking, and will be optionally available for use by both server
processes.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: on-the-fly lock checking
Posted by Ben Collins-Sussman <su...@collab.net>.
On Dec 2, 2004, at 5:16 PM, Greg Stein wrote:
> On Fri, Dec 03, 2004 at 12:02:37AM +0100, Branko ??ibej wrote:
>> Ben Collins-Sussman wrote:
>>
>>> So I'm going to rip the on-the-fly lock checking out of BDB's public
>>> fs funcs, and fitz going to avoid coding that feature in FSFS.
>>> Meanwhile, I'm going to create some libsvn_repos wrappers around the
>>> 8
>>> or so fs-write routines. These wrappers will do on-the-fly lock
>>> checking, and will be optionally available for use by both server
>>> processes.
>>
>> I'm on the verge of suggesting we deprecate all the FS wrappers in
>> libsvn_repos, even the new ones...what a mess...
>
> +1 ... I never really liked those darned wrappers.
>
Would either of you guys like to elaborate?
cmpilato believes that it's "inevitable" that all fs funcs be wrapped
in libsvn_repos. But I'd like to know why you guys are unhappy.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: on-the-fly lock checking
Posted by Greg Stein <gs...@lyra.org>.
On Fri, Dec 03, 2004 at 12:02:37AM +0100, Branko ??ibej wrote:
> Ben Collins-Sussman wrote:
>
> >So I'm going to rip the on-the-fly lock checking out of BDB's public
> >fs funcs, and fitz going to avoid coding that feature in FSFS.
> >Meanwhile, I'm going to create some libsvn_repos wrappers around the 8
> >or so fs-write routines. These wrappers will do on-the-fly lock
> >checking, and will be optionally available for use by both server
> >processes.
>
> I'm on the verge of suggesting we deprecate all the FS wrappers in
> libsvn_repos, even the new ones...what a mess...
+1 ... I never really liked those darned wrappers.
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: on-the-fly lock checking
Posted by Andy Whitcroft <ap...@shadowen.org>.
C. Michael Pilato wrote:
> I began a rebuttal. Then I actually applied thought. Added a pinch
> of sanity check from Ben Collins-Sussman. And the response is that
> there is no rebuttal to be had.
>
> I suppose we can do much of this right now. Ben can rev create_txn(),
> adding two parameters (one for on-the-fly lock checking, one for
> out-of-dateness). We can implement the on-the-fly lock checking in
> the FS. We can document that we don't do OOD checks yet, and return
> FEATURE_NOT_IMPLEMENTED if that boolean is TRUE.
> svn_repos_begin_txn_for_commit/update() can pass FALSE for OOD
> checking parameter (now and forever), and we can start deprecating
> functions.
Can we not at least use a flags mask or something ... more and more
params for each 'modifier' seems over kill.
-apw
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: on-the-fly lock checking
Posted by Ben Collins-Sussman <su...@collab.net>.
On Dec 5, 2004, at 2:37 PM, Greg Hudson wrote:
> On Sun, 2004-12-05 at 13:45, C. Michael Pilato wrote:
>> Comfort to Branko: there's no interface for clients to modify
>> transation properties.
>
> Although it would be nice if there were. But at such a time as we add
> one, we could disallow a section of the property namespace for our
> internal use.
>
Isn't "svn:" already reserved for internal use, both for versioned
*and* unversioned properties? I thought that users were already
forbidden from inventing properties in that namespace. So I was simply
going to dream up a new svn: transaction property for internal use.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: on-the-fly lock checking
Posted by Greg Hudson <gh...@MIT.EDU>.
On Sun, 2004-12-05 at 13:45, C. Michael Pilato wrote:
> Comfort to Branko: there's no interface for clients to modify
> transation properties.
Although it would be nice if there were. But at such a time as we add
one, we could disallow a section of the property namespace for our
internal use.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: on-the-fly lock checking
Posted by "C. Michael Pilato" <cm...@collab.net>.
Branko Čibej <br...@xbc.nu> writes:
> > cmpilato and I were thinking that perhaps svn_fs_begin_txn2() can
> > save the 'flags' value as a temporary svn: revprop. and then
> > svn_fs_commit_txn() can quietly remove it.
>
> I don't see anything fundamentally wrong with that. I would use a
> revprop name that can't be set from the client, though (i.e., that
> doesn't pass our prop name validation). We want to be careful that a
> client can't change that revprop's value.
Minor correction to Ben's comment: this is a temporary *transaction*
property.
Comfort to Branko: there's no interface for clients to modify
transation properties.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: on-the-fly lock checking
Posted by Branko Čibej <br...@xbc.nu>.
Ben Collins-Sussman wrote:
>
> On Dec 3, 2004, at 6:48 PM, Branko Čibej wrote:
>
>>> I suppose we can do much of this right now. Ben can rev create_txn(),
>>> adding two parameters (one for on-the-fly lock checking, one for
>>> out-of-dateness). We can implement the on-the-fly lock checking in
>>> the FS. We can document that we don't do OOD checks yet, and return
>>> FEATURE_NOT_IMPLEMENTED if that boolean is TRUE.
>>> svn_repos_begin_txn_for_commit/update() can pass FALSE for OOD
>>> checking parameter (now and forever), and we can start deprecating
>>> functions.
>>>
>> One bitmap parameter would suffice, I think; we may have other kinds
>> of checks later on, and this way we don't have to rev create_txn
>> every time. Otherwise, +1. This will give us a much cleaner API in
>> the long run.
>>
>
> So I've added a 'flags' parameter to svn_fs_begin_txn2().
>
> But guess what? This new variable needs to be attached to the
> transaction. Remember that fs users can constantly open/close the
> transaction (like mod_dav_svn, for example). But that means changing
> the definition of a txn_t... which means a db schema change. Ick!
Hmmm, yes, good point.
> cmpilato and I were thinking that perhaps svn_fs_begin_txn2() can save
> the 'flags' value as a temporary svn: revprop. and then
> svn_fs_commit_txn() can quietly remove it.
I don't see anything fundamentally wrong with that. I would use a
revprop name that can't be set from the client, though (i.e., that
doesn't pass our prop name validation). We want to be careful that a
client can't change that revprop's value.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: on-the-fly lock checking
Posted by Ben Collins-Sussman <su...@collab.net>.
On Dec 3, 2004, at 6:48 PM, Branko Čibej wrote:
>> I suppose we can do much of this right now. Ben can rev create_txn(),
>> adding two parameters (one for on-the-fly lock checking, one for
>> out-of-dateness). We can implement the on-the-fly lock checking in
>> the FS. We can document that we don't do OOD checks yet, and return
>> FEATURE_NOT_IMPLEMENTED if that boolean is TRUE.
>> svn_repos_begin_txn_for_commit/update() can pass FALSE for OOD
>> checking parameter (now and forever), and we can start deprecating
>> functions.
>>
> One bitmap parameter would suffice, I think; we may have other kinds
> of checks later on, and this way we don't have to rev create_txn every
> time. Otherwise, +1. This will give us a much cleaner API in the long
> run.
>
So I've added a 'flags' parameter to svn_fs_begin_txn2().
But guess what? This new variable needs to be attached to the
transaction. Remember that fs users can constantly open/close the
transaction (like mod_dav_svn, for example). But that means changing
the definition of a txn_t... which means a db schema change. Ick!
cmpilato and I were thinking that perhaps svn_fs_begin_txn2() can save
the 'flags' value as a temporary svn: revprop. and then
svn_fs_commit_txn() can quietly remove it.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: on-the-fly lock checking
Posted by Branko Čibej <br...@xbc.nu>.
C. Michael Pilato wrote:
>I began a rebuttal. Then I actually applied thought. Added a pinch
>of sanity check from Ben Collins-Sussman. And the response is that
>there is no rebuttal to be had.
>
>
Ah, I'm glad you agree. :-)
>I suppose we can do much of this right now. Ben can rev create_txn(),
>adding two parameters (one for on-the-fly lock checking, one for
>out-of-dateness). We can implement the on-the-fly lock checking in
>the FS. We can document that we don't do OOD checks yet, and return
>FEATURE_NOT_IMPLEMENTED if that boolean is TRUE.
>svn_repos_begin_txn_for_commit/update() can pass FALSE for OOD
>checking parameter (now and forever), and we can start deprecating
>functions.
>
>
One bitmap parameter would suffice, I think; we may have other kinds of
checks later on, and this way we don't have to rev create_txn every
time. Otherwise, +1. This will give us a much cleaner API in the long run.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: on-the-fly lock checking
Posted by "C. Michael Pilato" <cm...@collab.net>.
I began a rebuttal. Then I actually applied thought. Added a pinch
of sanity check from Ben Collins-Sussman. And the response is that
there is no rebuttal to be had.
I suppose we can do much of this right now. Ben can rev create_txn(),
adding two parameters (one for on-the-fly lock checking, one for
out-of-dateness). We can implement the on-the-fly lock checking in
the FS. We can document that we don't do OOD checks yet, and return
FEATURE_NOT_IMPLEMENTED if that boolean is TRUE.
svn_repos_begin_txn_for_commit/update() can pass FALSE for OOD
checking parameter (now and forever), and we can start deprecating
functions.
Branko Čibej <br...@xbc.nu> writes:
> The FS wrappers today are doing two things:
>
> * running hooks
> * checking out-of-datendess
>
> We obviously don't need wrappers for running hooks; as I told Ben the
> other day on IRC, a set of callbacks would serve much better (/and/
> we'd be able to finally implement read hooks).
>
> The out-of datedness check is obviously a performance optimisation,
> and any way I look at it, it doetn's belong in libsvn_repos. I mean,
> it's a bit funny to have libsvn_repos do some of the out-of-date
> checks, and libsvn_fs do others. The argument that a user can use a fs
> txn as a scratch area, so the fs shouldn't do out-of-date checks until
> commit...well, it holds water up to a point (even though IIRC there
> are not /right now/ any such applications). Now, I think you'll agree
> that it makes no sense to theck out-of-detedness for some FS
> operations but not others /within a single txn/. So the decision
> whether the checks should be performed always or just at commit canbe
> made at txn creation time, and is a parameter of the transaction. Ten
> all FS functions that take a txn parameter can decide whether to
> perform the checks or not. The FS api remains unchanged (except for
> create_txn, which gets an extra flag); the repos wrappers go away, and
> everybody's happy.
>
> (s/out-of-date/lock/g in the above, same argument applies)
>
> -- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: on-the-fly lock checking
Posted by Branko Čibej <br...@xbc.nu>.
C. Michael Pilato wrote:
>Branko Čibej <br...@xbc.nu> writes:
>
>
>
>>Ben Collins-Sussman wrote:
>>
>>
>>
>>>So I'm going to rip the on-the-fly lock checking out of BDB's public
>>>fs funcs, and fitz going to avoid coding that feature in FSFS.
>>>Meanwhile, I'm going to create some libsvn_repos wrappers around the 8
>>>or so fs-write routines. These wrappers will do on-the-fly lock
>>>checking, and will be optionally available for use by both server
>>>processes.
>>>
>>>
>>I'm on the verge of suggesting we deprecate all the FS wrappers in
>>libsvn_repos, even the new ones...what a mess...
>>
>>
>
>Nonsense. There's nothing messy about any of it. The filesystem
>library has an API, and does its thing. The repository library wraps
>the filesystem API to do things that are flatly wrong for the
>filesystem API to do. If you can find a way to wrap the FS without
>using wrappers, I'm all ears (or eyes).
>
>
The FS wrappers today are doing two things:
* running hooks
* checking out-of-datendess
We obviously don't need wrappers for running hooks; as I told Ben the
other day on IRC, a set of callbacks would serve much better (/and/ we'd
be able to finally implement read hooks).
The out-of datedness check is obviously a performance optimisation, and
any way I look at it, it doetn's belong in libsvn_repos. I mean, it's a
bit funny to have libsvn_repos do some of the out-of-date checks, and
libsvn_fs do others. The argument that a user can use a fs txn as a
scratch area, so the fs shouldn't do out-of-date checks until
commit...well, it holds water up to a point (even though IIRC there are
not /right now/ any such applications). Now, I think you'll agree that
it makes no sense to theck out-of-detedness for some FS operations but
not others /within a single txn/. So the decision whether the checks
should be performed always or just at commit canbe made at txn creation
time, and is a parameter of the transaction. Ten all FS functions that
take a txn parameter can decide whether to perform the checks or not.
The FS api remains unchanged (except for create_txn, which gets an extra
flag); the repos wrappers go away, and everybody's happy.
(s/out-of-date/lock/g in the above, same argument applies)
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: on-the-fly lock checking
Posted by "C. Michael Pilato" <cm...@collab.net>.
Branko Čibej <br...@xbc.nu> writes:
> Ben Collins-Sussman wrote:
>
> > So I'm going to rip the on-the-fly lock checking out of BDB's public
> > fs funcs, and fitz going to avoid coding that feature in FSFS.
> > Meanwhile, I'm going to create some libsvn_repos wrappers around the 8
> > or so fs-write routines. These wrappers will do on-the-fly lock
> > checking, and will be optionally available for use by both server
> > processes.
>
> I'm on the verge of suggesting we deprecate all the FS wrappers in
> libsvn_repos, even the new ones...what a mess...
Nonsense. There's nothing messy about any of it. The filesystem
library has an API, and does its thing. The repository library wraps
the filesystem API to do things that are flatly wrong for the
filesystem API to do. If you can find a way to wrap the FS without
using wrappers, I'm all ears (or eyes).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: on-the-fly lock checking
Posted by Branko Čibej <br...@xbc.nu>.
Ben Collins-Sussman wrote:
> So I'm going to rip the on-the-fly lock checking out of BDB's public
> fs funcs, and fitz going to avoid coding that feature in FSFS.
> Meanwhile, I'm going to create some libsvn_repos wrappers around the 8
> or so fs-write routines. These wrappers will do on-the-fly lock
> checking, and will be optionally available for use by both server
> processes.
I'm on the verge of suggesting we deprecate all the FS wrappers in
libsvn_repos, even the new ones...what a mess...
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org