[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1143860 - /subversion/trunk/subversion/libsvn_ra_serf/update.c

From: Ivan Zhakov <ivan_at_visualsvn.com>
Date: Tue, 12 Jul 2011 11:58:50 +0400

On Tue, Jul 12, 2011 at 02:46, Bert Huijben <bert_at_qqmail.nl> wrote:
>> >> Log:
>> >> Fix calculation X-SVN-VR-Base request header in ra_serf.
>> >>
>> >> This commit reverts r1142065 and r1143089.
>> >>
>> >> * subversion/libsvn_ra_serf/update.c
>> >>   (report_dir_t): Remove repos_relpath.
>> >>   (report_context_t): Remove root_is_switched.
>> >>   (start_report): Do not update repos_relpath.
>> >>   (end_report): Use only wcprops as source for delta base. Our current
>> >>    protocol doesn't gives us stable way to construct delta base without
>> >>    wcprops. We can optimize it later.
>> >
>> > Whoa!  wcprops again?  Can you explain this?  wcprops only ever existed
>> > because it was too expensive to ask the server to give us a URL to
> describe
>> > a particular object.  But URL generation is *supposed* to be trivial in
>> > HTTPv2, so... what's up?!
>> >
>> We cannot calculate them even over HTTPv2 in case of switched paths
>> (or file externals). Bert implemented some heuristic in r1142065, but
>> as any heuristic method. Really stable way to fix the problem is to
>> include property with repos relative path to update report.
>>
>> The problem that old code actually didn't work: X-SVN-VR-Base request
>> header was simply ignored by server (see r1141845).
>
> [ from the top of this mail]
>>
>> This was no heuristic. This was always guaranteed the right
>> information, because it was obtained from the wc just after obtaining
>> the write lock.
>>
>> If this would be some heuristic ANY UPDATE would be some heuristic as
>> the update itself is driven based on the same information. (Except that
>> it is stored in a tempfile on the server because storing a revision,
>> lock, path, etc for every node can easily be to big to store in ram.
>> The number of switches in a wc is- just like the obtained locks-
>> typically very low)
>>
>
> Any update is started by reporting the revisions and paths in the working
> copy to the repository via a svn_ra_reporter3_t instance.
>
> From the information in this report the server sends the minimal list of
> changes necessary to change the working copy into the state of the target
> revision.
>
> My patch extracted the paths that are switched against their parent from
> this information and store these in a hashtable to generate the stable urls
> we need here.
>
>
> Why did you disable/remove this code?
>
Hi Bert,

Sorry, I had to discuss my commit with you before.

My idea was to stabilize ra_serf code and fix the real regression:
downloading full-texts, instead of deltas. So I reverted code to some
stable and proven state (same as in ra_neon).

You changes seems to be correct and in right way, but I've reverted
them just to return to stable point.

I think we can commit your changes back, but if you'd like to really
optimize performance you also should remove updating wcprops on
checkout/update. What do you think?

-- 
Ivan Zhakov
Received on 2011-07-12 10:00:02 CEST

This is an archived mail posted to the Subversion Dev mailing list.