Re: svn commit: r1673170 - in /subversion/trunk/subversion: include/ libsvn_fs/ libsvn_fs_base/ libsvn_fs_fs/ libsvn_fs_x/ libsvn_ra_local/ libsvn_repos/ mod_dav_svn/ svnserve/ tests/libsvn_ra/

On 13 April 2015 at 19:05, Bert Huijben <bert_at_qqmail.nl> wrote:
>> -----Original Message-----
>> From: Ivan Zhakov [mailto:ivan_at_visualsvn.com]
>> Sent: maandag 13 april 2015 17:13
>> To: Bert Huijben
>> Cc: dev_at_subversion.apache.org
>> Subject: Re: svn commit: r1673170 - in /subversion/trunk/subversion: include/
>> libsvn_fs/ libsvn_fs_base/ libsvn_fs_fs/ libsvn_fs_x/ libsvn_ra_local/
>> libsvn_repos/ mod_dav_svn/ svnserve/ tests/libsvn_ra/
>>
>> On 13 April 2015 at 17:41, Bert Huijben <bert_at_qqmail.nl> wrote:
>> >> -----Original Message-----
>> >> From: Ivan Zhakov [mailto:ivan_at_visualsvn.com]
>> >> Sent: maandag 13 april 2015 14:53
>> >> To: dev_at_subversion.apache.org; Bert Huijben
>> >> Subject: Re: svn commit: r1673170 - in /subversion/trunk/subversion:
>> include/
>> >> libsvn_fs/ libsvn_fs_base/ libsvn_fs_fs/ libsvn_fs_x/ libsvn_ra_local/
>> >> libsvn_repos/ mod_dav_svn/ svnserve/ tests/libsvn_ra/
>> >
>> >> The proper solution would be add new DAV property like
>> >> "has-dead-props", advertise it support using capability header and
>> >> then request it from client instead of "deadprop-count".
>> >>
>> >> What do you think?
>> >
>> > The problem is that currently all subversion clients that perform an 'svn ls -v'
>> (including TortoiseSVN)
>> > use the existing request. New clients that know about this problem simply
>> don't ask for this property.
>> > If we do it as you suggest we don't help old clients and we don't help new
>> clients, while old
>> > clients don't have a way to access this integer.
>> >
>> I understand you intention to improve performance for users with older
>> clients, but with introducing such protocol hacks your may end with
>> situation when you need time machine for fix bugs/problems.
>>
>> Your 'svn ls -v' fix (r1673153) is really nice and simple. We could
>> easily backport it to 1.8.x and 1.9.x. Users who experience this
>> problem should upgrade to newer version.
>>
>>
>> > I think we should define a new thing (capability, header, property, whatever)
>> if we ever decide
>> > that we are interested in the integer. Given that today we aren't even
>> interested in the boolean
>> > value (but do spend a whole lot of server CPU obtaining it - Minutes in my
>> tescases), I
>> > don't think it is likely that we ever want the real value, as the other ra layers
>> don't provide
>> > the value either.
>> Command line client doesn't interested in it, but we expose has_props
>> through API and clients like TortoiseSVN uses it.
>>
>> >
>> > The best solution would be to do as you suggested, but in that case we need a
>> time machine (or
>> > otherwise we should do nothing at all and keep mod_dav as slow as it is
>> today).
>> >
>> >
>> > In this already bad case I think it is better to tell new servers that we want the
>> real value
>> > in the future instead of now spending minutes of server CPU and IO time on a
>> request that
>> > could end in seconds, when nobody is interested in the result :(
>> >
>> I think that making server side to report bogus data (99 as dead prop
>> count) is protocol violation. I'm -1 on it. I think we should
>> implement proper server side fix or remove it since we already fixed
>> command line client.
>
> (Not answering any question here, or trying to convince you... just writing down my reasoning)
>
> I like to think that mod_dav implements just the RA api when we have is_svn_client as TRUE (or HTTPv2 when applicable), while we try to provide a good read-only DAV client when it is not. (It is only a writable DAV client after explicitly enabling autoversioning)
>
>
> So in general I try to make mod_dav behave as the other ra layers, instead of just trying to map everything on DAV and leaving it there.
>
> In this case that would make ra_dav -like on the log report- get the worst behavior of all ra layers, while it is in my opinion the most popular/mostly used ra layer.
>
>
>
> I don't see which minority of users (if any) we help by aiming for DAV compatibility over svn ra layer usability/performance here.
>
> (BTW: I don't think we ever documented what the deadprop-count value in the
> subversion xmlns represents... It just happens to be what it is today, and is just
> used as a boolean since r858587, committed in 2006 to fix issue 2151 "'svn ls'
> is slow over ra_dav")
Yes, you're right: deadprop-count is not documented, but many parts of
DAV protocol is not documented :(

In this case I'm fine to change deadprop-count semantic to be just a
boolean with 0/1 and behave the same for *all* clients. And document
it properly. What do you think?

-- 
Ivan Zhakov