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

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/

From: Bert Huijben <bert_at_qqmail.nl>
Date: Mon, 13 Apr 2015 21:53:55 +0200

> -----Original Message-----
> From: Ivan Zhakov [mailto:ivan_at_visualsvn.com]
> Sent: maandag 13 april 2015 18:55
> 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 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?

I'm 100% ok with that solution.
(Still waiting on other responses before I'll write a patch to implement this)

Do you know where we document these properties?
(Or have a suggestion on where we should document them)

I was trying to find these earlier today, but failed. I had to resolve to using annotate/blame to find out the history of this code.

        Bert
Received on 2015-04-13 21:56:43 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.