You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Arwin Arni <ar...@collab.net> on 2011/04/18 12:35:14 UTC
[PATCH] Add a test in dav-mirror-autocheck.sh to showcase a out-of-date
slave related bug
Hi All,
This patch adds a test case to
subversion/tests/cmdline/dav-mirror-autocheck.sh to showcase the
following bug.
In a master/slave repositories setup where writes are proxied to the
master and reads are handles by the slave repository, if the slave
repository is behind the master by one or more revisions, certain
operations that one would expect to succeed are failing.
Example:
$svn cp -m "Create branch" ^/trunk ^/branches/newbranch (from a working
copy of the slave)
will FAIL if the master has an unsynced commit (not yet synced with
slave) anywhere below ^/branches.
i.e Coder 1 does a commit to branches/some-other-branch (master has
received the commit, slave hasn't yet synced)
Coder 2 tries to create a new branch via above command. It fails.
I'll shortly file an issue in the tigris tracker for this.
Regards,
Arwin Arni
Re: [PATCH] Add a test in dav-mirror-autocheck.sh to showcase a
out-of-date slave related bug
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
On Mon, 18 Apr 2011 22:14 -0400, "Mark Phippard" <ma...@gmail.com> wrote:
> On Mon, Apr 18, 2011 at 7:41 AM, Daniel Shahaf <d....@daniel.shahaf.name>
> wrote:
> > I agree it would be nice if this worked, but given that we have to remain sane to people who open an RA session to the slave before it has
> > synced up I'm afraid it might be tricky to address. (That corresponds to the case of concurrent commits --- two people attempting Mark's two
> > mkdirs concurrently.)
...
> [...] As you know most of our SVN commands involved many HTTP
> requests and the problem here is that some of them are handled by the
> proxy and others are proxied to the master. A solution would likely
> require the proxy to have more awareness of the SVN command being
> executed so that it knew to proxy all of the requests back to the
> master rather than handle any of them off the local replica.
What I'm worried about is someone who works with the RA API directly and expecting the API responses to be consistent with a given view of a repository --- regardless of whether they're working against a DAV mirror or not.
Re: [PATCH] Add a test in dav-mirror-autocheck.sh to showcase a out-of-date
slave related bug
Posted by Kamesh Jayachandran <ka...@collab.net>.
On 04/19/2011 07:44 AM, Mark Phippard wrote:
> On Mon, Apr 18, 2011 at 7:41 AM, Daniel Shahaf<d....@daniel.shahaf.name> wrote:
>> On Mon, 18 Apr 2011 07:08 -0400, "Mark Phippard"<ma...@gmail.com> wrote:
>>> I know why it fails, but I would not expect it to fail as a user, even
>>> with a proxy. I did not look at Arwin's test but it does not require
>>> a WC to show the failure. This also fails:
>>>
>>> $ svn mkdir url://branches/branch1
>>> $ svn mkdir url://branches/branch2
>> Because in the second mkdir, the slave's idea of HEAD is behind master's and user's idea of HEAD.
> As I said, I know why it fails. That does not mean we should not
> identify these problems and look for ways to solve them. If proxy was
> a first class feature it would know you were running a command where
> the all of the HTTP requests should be proxied and this would not
> fail.
>
>> I agree it would be nice if this worked, but given that we have to remain sane to people who open an RA session to the slave before it has
>> synced up I'm afraid it might be tricky to address. (That corresponds to the case of concurrent commits --- two people attempting Mark's two
>> mkdirs concurrently.)
>>
>> Separately: we might want to teach the wc that one repos_url (out of several with the same repos_uuid) is preferred in given circumstances...
>> then we could have '^mirror/' and '^master/' syntaxes.</random-thoughts>
> To reiterate, there is no WC involved here. So a change like that is
> not needed. This problem exists with commands that are run on URL's
> alone. As you know most of our SVN commands involved many HTTP
> requests and the problem here is that some of them are handled by the
> proxy and others are proxied to the master. A solution would likely
> require the proxy to have more awareness of the SVN command being
> executed so that it knew to proxy all of the requests back to the
> master rather than handle any of them off the local replica.
FWIW at [1] I originally sent a patch which would provide a RA API to
set a request header 'SVN-ACTION' indicating high level svn operation.
[1] http://svn.haxx.se/dev/archive-2010-01/0101.shtml
With regards
Kamesh Jayachandran
Re: [PATCH] Add a test in dav-mirror-autocheck.sh to showcase a
out-of-date slave related bug
Posted by Mark Phippard <ma...@gmail.com>.
On Mon, Apr 18, 2011 at 7:41 AM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> On Mon, 18 Apr 2011 07:08 -0400, "Mark Phippard" <ma...@gmail.com> wrote:
>> I know why it fails, but I would not expect it to fail as a user, even
>> with a proxy. I did not look at Arwin's test but it does not require
>> a WC to show the failure. This also fails:
>>
>> $ svn mkdir url://branches/branch1
>> $ svn mkdir url://branches/branch2
>
> Because in the second mkdir, the slave's idea of HEAD is behind master's and user's idea of HEAD.
As I said, I know why it fails. That does not mean we should not
identify these problems and look for ways to solve them. If proxy was
a first class feature it would know you were running a command where
the all of the HTTP requests should be proxied and this would not
fail.
> I agree it would be nice if this worked, but given that we have to remain sane to people who open an RA session to the slave before it has
> synced up I'm afraid it might be tricky to address. (That corresponds to the case of concurrent commits --- two people attempting Mark's two
> mkdirs concurrently.)
>
> Separately: we might want to teach the wc that one repos_url (out of several with the same repos_uuid) is preferred in given circumstances...
> then we could have '^mirror/' and '^master/' syntaxes. </random-thoughts>
To reiterate, there is no WC involved here. So a change like that is
not needed. This problem exists with commands that are run on URL's
alone. As you know most of our SVN commands involved many HTTP
requests and the problem here is that some of them are handled by the
proxy and others are proxied to the master. A solution would likely
require the proxy to have more awareness of the SVN command being
executed so that it knew to proxy all of the requests back to the
master rather than handle any of them off the local replica.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: [PATCH] Add a test in dav-mirror-autocheck.sh to showcase a
out-of-date slave related bug
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
On Mon, 18 Apr 2011 07:08 -0400, "Mark Phippard" <ma...@gmail.com> wrote:
> I know why it fails, but I would not expect it to fail as a user, even
> with a proxy. I did not look at Arwin's test but it does not require
> a WC to show the failure. This also fails:
>
> $ svn mkdir url://branches/branch1
> $ svn mkdir url://branches/branch2
Because in the second mkdir, the slave's idea of HEAD is behind master's and user's idea of HEAD.
I agree it would be nice if this worked, but given that we have to remain sane to people who open an RA session to the slave before it has synced up I'm afraid it might be tricky to address. (That corresponds to the case of concurrent commits --- two people attempting Mark's two mkdirs concurrently.)
Separately: we might want to teach the wc that one repos_url (out of several with the same repos_uuid) is preferred in given circumstances... then we could have '^mirror/' and '^master/' syntaxes. </random-thoughts>
Daniel
(who also had thoughts about ^key/ syntax from ~/.s/config)
Re: [PATCH] Add a test in dav-mirror-autocheck.sh to showcase a
out-of-date slave related bug
Posted by Mark Phippard <ma...@gmail.com>.
On Mon, Apr 18, 2011 at 6:51 AM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> On Mon, 18 Apr 2011 16:05 +0530, "Arwin Arni" <ar...@collab.net> wrote:
>> Hi All,
>>
>> This patch adds a test case to
>> subversion/tests/cmdline/dav-mirror-autocheck.sh to showcase the
>> following bug.
>>
>> In a master/slave repositories setup where writes are proxied to the
>> master and reads are handles by the slave repository, if the slave
>> repository is behind the master by one or more revisions, certain
>> operations that one would expect to succeed are failing.
>>
>
> Why are you expecting it to succeed?
>
> Consider:
>
> % svn co ^/ wc
> % cd wc
> % svn mkdir -mm ^/branches/foo
> % svn cp trunk branches/bar
> % svn ci -mm
>
> which should fail with 'out of date'.
>
> Therefore I think your case should fail as well.
I know why it fails, but I would not expect it to fail as a user, even
with a proxy. I did not look at Arwin's test but it does not require
a WC to show the failure. This also fails:
$ svn mkdir url://branches/branch1
$ svn mkdir url://branches/branch2
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: [PATCH] Add a test in dav-mirror-autocheck.sh to showcase a
out-of-date slave related bug
Posted by Peter Samuelson <pe...@p12n.org>.
[Daniel Shahaf]
> when the slave hasn't synced up yet, I assume (not knowing the proxy
> code) that the editor would be opened against a revision N1, such
> that on master branches/ has been touched in some revision N2 > N1.
> Therefore, as far as master is concerned, the error is proper.
> (consistent with the FS design)
Yes, but why is this only a problem with a write-through proxy?
Shouldn't the same race exist between two clients talking to the _same_
master server? (Not just cp and mkdir, but anything that on the client
side would have been considered a tree conflict.)
I don't know the exact sequence used here - is there a server-side
lock during the commit that prevents these conflicts? If so, the
write-through proxy should really perform the same actions on the
server that cause this lock to perform the same function.
--
Peter Samuelson | org-tld!p12n!peter | http://p12n.org/
Re: [PATCH] Add a test in dav-mirror-autocheck.sh to showcase a
out-of-date slave related bug
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
On Mon, 18 Apr 2011 13:51 +0300, "Daniel Shahaf" <d....@daniel.shahaf.name> wrote:
> On Mon, 18 Apr 2011 16:05 +0530, "Arwin Arni" <ar...@collab.net> wrote:
> > Hi All,
> >
> > This patch adds a test case to
> > subversion/tests/cmdline/dav-mirror-autocheck.sh to showcase the
> > following bug.
> >
> > In a master/slave repositories setup where writes are proxied to the
> > master and reads are handles by the slave repository, if the slave
> > repository is behind the master by one or more revisions, certain
> > operations that one would expect to succeed are failing.
> >
>
> Why are you expecting it to succeed?
>
> Consider:
>
> % svn co ^/ wc
> % cd wc
> % svn mkdir -mm ^/branches/foo
> % svn cp trunk branches/bar
> % svn ci -mm
>
> which should fail with 'out of date'.
>
> Therefore I think your case should fail as well.
In more words...
when the slave hasn't synced up yet, I assume (not knowing the proxy code) that the editor would be opened against a revision N1, such that on master branches/ has been touched in some revision N2 > N1. Therefore, as far as master is concerned, the error is proper. (consistent with the FS design)
Not sure if there's a good fix here... (could try to open an editor against N2 on the slave?)
Re: [PATCH] Add a test in dav-mirror-autocheck.sh to showcase a
out-of-date slave related bug
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
On Mon, 18 Apr 2011 16:05 +0530, "Arwin Arni" <ar...@collab.net> wrote:
> Hi All,
>
> This patch adds a test case to
> subversion/tests/cmdline/dav-mirror-autocheck.sh to showcase the
> following bug.
>
> In a master/slave repositories setup where writes are proxied to the
> master and reads are handles by the slave repository, if the slave
> repository is behind the master by one or more revisions, certain
> operations that one would expect to succeed are failing.
>
Why are you expecting it to succeed?
Consider:
% svn co ^/ wc
% cd wc
% svn mkdir -mm ^/branches/foo
% svn cp trunk branches/bar
% svn ci -mm
which should fail with 'out of date'.
Therefore I think your case should fail as well.
>
> Example:
>
> $svn cp -m "Create branch" ^/trunk ^/branches/newbranch (from a working
> copy of the slave)
>
> will FAIL if the master has an unsynced commit (not yet synced with
> slave) anywhere below ^/branches.
>
> i.e Coder 1 does a commit to branches/some-other-branch (master has
> received the commit, slave hasn't yet synced)
> Coder 2 tries to create a new branch via above command. It fails.
>
> I'll shortly file an issue in the tigris tracker for this.
>
> Regards,
> Arwin Arni
>
> Email had 2 attachments:
> + out_of_date_slave_commit_test.patch.txt
> 2k (text/plain)
> + out_of_date_slave_commit_test.log.txt
> 1k (text/plain)
Re: [PATCH] Add a test in dav-mirror-autocheck.sh to showcase a out-of-date
slave related bug
Posted by Kamesh Jayachandran <ka...@collab.net>.
Thanks, Committed this patch in r1095662.
With regards
Kamesh Jayachandran
On 04/18/2011 04:05 PM, Arwin Arni wrote:
> Hi All,
>
> This patch adds a test case to
> subversion/tests/cmdline/dav-mirror-autocheck.sh to showcase the
> following bug.
>
> In a master/slave repositories setup where writes are proxied to the
> master and reads are handles by the slave repository, if the slave
> repository is behind the master by one or more revisions, certain
> operations that one would expect to succeed are failing.
>
>
> Example:
>
> $svn cp -m "Create branch" ^/trunk ^/branches/newbranch (from a
> working copy of the slave)
>
> will FAIL if the master has an unsynced commit (not yet synced with
> slave) anywhere below ^/branches.
>
> i.e Coder 1 does a commit to branches/some-other-branch (master has
> received the commit, slave hasn't yet synced)
> Coder 2 tries to create a new branch via above command. It fails.
>
> I'll shortly file an issue in the tigris tracker for this.
>
> Regards,
> Arwin Arni
Re: [PATCH] Add a test in dav-mirror-autocheck.sh to showcase a out-of-date
slave related bug
Posted by Arwin Arni <ar...@collab.net>.
On Monday 18 April 2011 04:05 PM, Arwin Arni wrote:
>
> I'll shortly file an issue in the tigris tracker for this.
>
Filed Issue #3860 and set the target milestone to 1.7-consider
Regards,
Arwin Arni