You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2010/08/11 00:40:51 UTC
[atomic-revprops] status update
For the last few weeks I've been working on the atomic-revprop branch.
(Its goal is port the new svn_fs_change_rev_prop2() to libsvn_ra, which
should allow callers to specify both a value and an optional "previous
value" and have the revprop change atomic; see [1].)
Currently, the API is implemented over all RA layers, and the test (prop 34)
passes.
To extend svnsync to use the new API for acquiring locks,
[1] http://thread.gmane.org/gmane.comp.version-control.subversion.devel/120330/focus=120416
Re: [atomic-revprops] status update
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
On Tue, Aug 10, 2010 at 8:28 PM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> Hyrum K. Wright wrote on Tue, Aug 10, 2010 at 20:10:54 -0500:
>> Do you think the work is sufficiently non-disruptive to be moved back to trunk?
>
> Yes.
>
> Updated plan: I'll do the "marshal error codes in ra_dav" work on trunk,
> re-test/review the status of the ra_dav part of the branch, and merge the
> branch to trunk then.
>
> Thanks for the reminder.
No problem. I was actually alluding to the work as a whole, rather
than just the mashall-error-codes bits. If you think you're done
monkeying with protocols and APIs to the point of causes impediments
to others' work, just go ahead and merge the sucker, says I.
-Hyrum
Re: [atomic-revprops] status update
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Hyrum K. Wright wrote on Tue, Aug 10, 2010 at 20:10:54 -0500:
> Do you think the work is sufficiently non-disruptive to be moved back to trunk?
Yes.
Updated plan: I'll do the "marshal error codes in ra_dav" work on trunk,
re-test/review the status of the ra_dav part of the branch, and merge the
branch to trunk then.
Thanks for the reminder.
Daniel
Re: [atomic-revprops] status update
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
On Tue, Aug 10, 2010 at 7:51 PM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> (hitted "send" too soon)
>
> Daniel Shahaf wrote on Wed, Aug 11, 2010 at 03:40:51 +0300:
>> For the last few weeks I've been working on the atomic-revprop branch.
>>
>> (Its goal is port the new svn_fs_change_rev_prop2() to libsvn_ra, which
>> should allow callers to specify both a value and an optional "previous
>> value" and have the revprop change atomic; see [1].)
>>
>> Currently, the API is implemented over all RA layers, and the test (prop 34)
>> passes.
>>
>> To extend svnsync to use the new API for acquiring locks,
>
> To extend svnsync to use the new API for acquiring locks, I'd like to make
> the API guarantee which error code it returns in case the atomic expectation
> didn't match the actual value (if any) found. This requires some work on
> ra_dav client/server to make them pass the error code on the wire (like
> ra_svn already does) --- per earlier thread this week [2].
>
> Until then, I have a working patch for svnsync, which --- until said ra_dav
> work is implemented --- relies on checking the error message (that's what I
> have available) rather than the error code. That patch is attached to the
> previous mail in this thread. (I'll commit that after the "check the error
> message" hack is removed.)
>
> That's all for now I suppose. A roadmap of the work can be found in
> BRANCH-README [3].
Do you think the work is sufficiently non-disruptive to be moved back to trunk?
-Hyrum
Re: [atomic-revprops] status update
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Julian Foad wrote on Wed, Aug 11, 2010 at 11:33:49 +0100:
> On Wed, 2010-08-11, Stefan Sperling wrote:
> > The BRANCH-README suggest that we add an interface like
> > 'svn propedit --revprop pname pval2 --old-value=pval'.
> > I don't think there is a valid use case for non-atomic revprop changes.
> > Why not just make all revprop changes atomic by default if the client
> > and server are both >= 1.7? Any propset/propedit operation could
> > transparently retrieve the old value before trying to set the new value.
>
> It makes sense for propEDIT to ensure that the old value it got is still
> current when it sets the new value.
It already does this, propedit-cmd.c calls svn_client_revprop_set2(), which
I have patched on the branch to be atomic if the server supports it (and is
already on trunk "best approximation of atomic" for older servers).
> It would be silly for that to be a
> command-line option. It should always do it.
>
(see above)
> But for a propSET command, the old value that the user was looking at
> before performing the set is not something that the command itself can
> get. There's no point in the command retrieving the "current" value at
> the time the propSET command is run, because that's not necessarily the
> value the user was looking at - it's already too late.
Yep.
> There is a case
> for this command taking the expected old value from the user, not that a
> command-line user is likely to provide it, but a script might. The API
> behind it should certainly take this option so that GUIs etc. can use it
> in implementing their own propedit-like functionality via the propset
> API.
>
Yes. I was wondering whether to extend the client API to use the atomic RA
API, but then I discovered it already was doing "best approximation of
atomicity" so I went ahead and patched it up. This reduces the scope of the
BRANCH-README item stsp quoted to just "Should propset-cmd.c expose this
flag?" --- which, re hwright's question, is one of the trivial items that I
won't hold reintegrating the branch on.
> - Julian
>
>
The _client API patches are r983234 and r983237 (both on the branch).
Re: [atomic-revprops] status update
Posted by Julian Foad <ju...@wandisco.com>.
On Wed, 2010-08-11, Stefan Sperling wrote:
> The BRANCH-README suggest that we add an interface like
> 'svn propedit --revprop pname pval2 --old-value=pval'.
> I don't think there is a valid use case for non-atomic revprop changes.
> Why not just make all revprop changes atomic by default if the client
> and server are both >= 1.7? Any propset/propedit operation could
> transparently retrieve the old value before trying to set the new value.
It makes sense for propEDIT to ensure that the old value it got is still
current when it sets the new value. It would be silly for that to be a
command-line option. It should always do it.
But for a propSET command, the old value that the user was looking at
before performing the set is not something that the command itself can
get. There's no point in the command retrieving the "current" value at
the time the propSET command is run, because that's not necessarily the
value the user was looking at - it's already too late. There is a case
for this command taking the expected old value from the user, not that a
command-line user is likely to provide it, but a script might. The API
behind it should certainly take this option so that GUIs etc. can use it
in implementing their own propedit-like functionality via the propset
API.
- Julian
Re: [atomic-revprops] status update
Posted by Stefan Sperling <st...@elego.de>.
On Wed, Aug 11, 2010 at 03:51:38AM +0300, Daniel Shahaf wrote:
> (hitted "send" too soon)
>
> Daniel Shahaf wrote on Wed, Aug 11, 2010 at 03:40:51 +0300:
> > For the last few weeks I've been working on the atomic-revprop branch.
> >
> > (Its goal is port the new svn_fs_change_rev_prop2() to libsvn_ra, which
> > should allow callers to specify both a value and an optional "previous
> > value" and have the revprop change atomic; see [1].)
> >
> > Currently, the API is implemented over all RA layers, and the test (prop 34)
> > passes.
> >
> > To extend svnsync to use the new API for acquiring locks,
>
> To extend svnsync to use the new API for acquiring locks, I'd like to make
> the API guarantee which error code it returns in case the atomic expectation
> didn't match the actual value (if any) found. This requires some work on
> ra_dav client/server to make them pass the error code on the wire (like
> ra_svn already does) --- per earlier thread this week [2].
>
> Until then, I have a working patch for svnsync, which --- until said ra_dav
> work is implemented --- relies on checking the error message (that's what I
> have available) rather than the error code. That patch is attached to the
> previous mail in this thread. (I'll commit that after the "check the error
> message" hack is removed.)
>
> That's all for now I suppose. A roadmap of the work can be found in
> BRANCH-README [3].
Thanks a lot for working on this! The svnsync locking problem is really
annoying, as it breaks replication setups that follow the svnbook.
The BRANCH-README suggest that we add an interface like
'svn propedit --revprop pname pval2 --old-value=pval'.
I don't think there is a valid use case for non-atomic revprop changes.
Why not just make all revprop changes atomic by default if the client
and server are both >= 1.7? Any propset/propedit operation could
transparently retrieve the old value before trying to set the new value.
Stefan
Re: [atomic-revprops] status update
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
(hitted "send" too soon)
Daniel Shahaf wrote on Wed, Aug 11, 2010 at 03:40:51 +0300:
> For the last few weeks I've been working on the atomic-revprop branch.
>
> (Its goal is port the new svn_fs_change_rev_prop2() to libsvn_ra, which
> should allow callers to specify both a value and an optional "previous
> value" and have the revprop change atomic; see [1].)
>
> Currently, the API is implemented over all RA layers, and the test (prop 34)
> passes.
>
> To extend svnsync to use the new API for acquiring locks,
To extend svnsync to use the new API for acquiring locks, I'd like to make
the API guarantee which error code it returns in case the atomic expectation
didn't match the actual value (if any) found. This requires some work on
ra_dav client/server to make them pass the error code on the wire (like
ra_svn already does) --- per earlier thread this week [2].
Until then, I have a working patch for svnsync, which --- until said ra_dav
work is implemented --- relies on checking the error message (that's what I
have available) rather than the error code. That patch is attached to the
previous mail in this thread. (I'll commit that after the "check the error
message" hack is removed.)
That's all for now I suppose. A roadmap of the work can be found in
BRANCH-README [3].
Daniel
(who maintains an offline list of points he should test/review)
>
>
>
> [1] http://thread.gmane.org/gmane.comp.version-control.subversion.devel/120330/focus=120416
[2] http://svn.haxx.se/dev/archive-2010-08/0225.shtml
[3] http://svn.apache.org/repos/asf/subversion/branches/atomic-revprop/BRANCH-README