You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Bert Huijben <be...@qqmail.nl> on 2011/06/17 13:01:03 UTC

RE: svn commit: r1136838 - /subversion/trunk/subversion/libsvn_client/commit_util.c


> -----Original Message-----
> From: rhuijben@apache.org [mailto:rhuijben@apache.org]
> Sent: vrijdag 17 juni 2011 12:29
> To: commits@subversion.apache.org
> Subject: svn commit: r1136838 -
> /subversion/trunk/subversion/libsvn_client/commit_util.c
> 
> Author: rhuijben
> Date: Fri Jun 17 10:28:33 2011
> New Revision: 1136838
> 
> URL: http://svn.apache.org/viewvc?rev=1136838&view=rev
> Log:
> * subversion/libsvn_client/commit_util.c
>   (svn_client__do_commit): Following up on r1136429, r1136530 and
> r1136823,
>     use a more informative error report for the most common ra_serf out of
> date
>     error.
> 
>     The other RA layers report out of date earlier, but ra_serf (and probably
>     neon) commonly reports out of date from here.
>     An improved fix might be to fix serf/neon as this indicates that some files
>     that aren't out of date may already be transfered.

I think this is a HTTPv2 limitation: In HTTPv1 we used to perform a checkout on open_directory() where we (somehow) passed the base_revision and got an out-of-date report.

With HTTPv2 we only pass the base revision to proppatch, so the out of date check for files only works on the MD5 hash we sent for the update.
(So theoretically we might see issues on MD5 collisions)


I think we should at least verify the base_revision somewhere in the PUT code path.

	Bert
 



Re: svn commit: r1136838 - /subversion/trunk/subversion/libsvn_client/commit_util.c

Posted by "C. Michael Pilato" <cm...@collab.net>.
On 06/17/2011 10:34 AM, Bert Huijben wrote:
> But only verifying it in the PUT requests, voids the optimization in our
> libsvn_client commit processing where we delay the puts until we
> completed all restructuring operations.
> 
> In theory the commit can now fail with the last PUT request (after much
> data has already been transferred), while on the other RA layers not a
> single file would be transferred before we find out that some node is out
> of date.

That's a very good point.  But just to be clear, we aren't in much worse of
a situation than we were with HTTPv1, are we?  We did a CHECKOUT on the
parent directory, but that wasn't sufficient to answer out-of-dateness about
the file itself.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


RE: svn commit: r1136838 - /subversion/trunk/subversion/libsvn_client/commit_util.c

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: C. Michael Pilato [mailto:cmpilato@collab.net]
> Sent: vrijdag 17 juni 2011 16:27
> To: Bert Huijben
> Cc: dev@subversion.apache.org
> Subject: Re: svn commit: r1136838 -
> /subversion/trunk/subversion/libsvn_client/commit_util.c
> 
> On 06/17/2011 07:01 AM, Bert Huijben wrote:
> > I think this is a HTTPv2 limitation: In HTTPv1 we used to perform a
> > checkout on open_directory() where we (somehow) passed the
> base_revision
> > and got an out-of-date report.
> >
> > With HTTPv2 we only pass the base revision to proppatch, so the out of
> > date check for files only works on the MD5 hash we sent for the update.
> > (So theoretically we might see issues on MD5 collisions)
> >
> > I think we should at least verify the base_revision somewhere in the PUT
> > code path.
> 
> We are including the base revision in the PUT request, right?  (See
> libsvn_ra_serf/commit.c:setup_put_headers(), for example.)  Are you saying
> that mod_dav_svn isn't verifying it?

Ok, I found the code where we do verify the base revision. (Thanks for the pointer)

But only verifying it in the PUT requests, voids the optimization in our libsvn_client commit processing where we delay the puts until we completed all restructuring operations.

In theory the commit can now fail with the last PUT request (after much data has already been transferred), while on the other RA layers not a single file would be transferred before we find out that some node is out of date.

	Bert



Re: svn commit: r1136838 - /subversion/trunk/subversion/libsvn_client/commit_util.c

Posted by "C. Michael Pilato" <cm...@collab.net>.
On 06/17/2011 07:01 AM, Bert Huijben wrote:
> I think this is a HTTPv2 limitation: In HTTPv1 we used to perform a
> checkout on open_directory() where we (somehow) passed the base_revision
> and got an out-of-date report.
> 
> With HTTPv2 we only pass the base revision to proppatch, so the out of
> date check for files only works on the MD5 hash we sent for the update. 
> (So theoretically we might see issues on MD5 collisions)
> 
> I think we should at least verify the base_revision somewhere in the PUT
> code path.

We are including the base revision in the PUT request, right?  (See
libsvn_ra_serf/commit.c:setup_put_headers(), for example.)  Are you saying
that mod_dav_svn isn't verifying it?

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand