You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2012/05/16 12:46:34 UTC
[DISCUSS] delete ra_neon
Short query: is this possible?
Long query:
Hyrum has been doing some heavy lifting on moving our code to Ev2 for
commits. That work can be merged soonish, but a strong argument can be made
for "only when all RA providers have an Ev2 commit editor" (because
otherwise poor-perf shims are used). ra_local already has the needed
editor, and I've been doing work on ra_serf to stabilize a base for writing
the Ev2 editor. Somebody is going to need to write an editor for ra_svn. My
query/suggestion is to delete ra_neon rather than needing to write an
editor for it.
We can also choose to not move to an Ev2 commit editor at this time.
Somebody could also write the editor. We could also state the shim usage is
acceptable (not that scary, actually). My proposed option is to delete
ra_neon rather than write a lot of code for it.
So as a discussion point, is deletion possible? What are the potential
blockers? If we don't do this for 1.8, would we *ever* do this? What are
the requirements?
We have some issues filed against ra_serf. Which of them should be
considered as blockers?
A following question asks about API compatibility. I believe we can simply
sub ra_serf into any loader/query for ra_neon. Hard linkage to the ra_neon
library may also need a solution (it becomes a shim?).
Finally, another answer is to abandon ra_serf entirely, or to sign up for
dual HTTP provider maintenance in perpetuity.
I think that I have laid out our options, but have likely missed some.
Please advise.
Thoughts? Where/how do we proceed?
Cheers,
-g
Re: [DISCUSS] delete ra_neon
Posted by Greg Stein <gs...@gmail.com>.
On Wed, May 16, 2012 at 10:40 AM, C. Michael Pilato <cm...@collab.net> wrote:
> On 05/16/2012 10:13 AM, Greg Stein wrote:
>> On Wed, May 16, 2012 at 9:43 AM, C. Michael Pilato <cm...@collab.net> wrote:
>>> but: How hard would it be to temporarily maintain two commit paths -- one
>>> that drives an Ev2 editor, and one that drives Ev1? The call to
>>> svn_ra_get_commit_editorN() would return NOT_IMPLEMENTED from ra_neon (and
>>> ra_svn), and the client would detect this and call the previous
>>> Ev1-returning API and drive it accordingly?
>>
>> Certainly doable. And then it leads to a new question: "how long do we
>> support these two commit drive code paths?", and it does not provide
>> an answer for ra_neon itself.
>
> We support two commit drive code paths until we need only support one. (Duh?)
My point was to get rid of double code paths (ie. ra_serf and
ra_neon). And then you suggest a different dual path :-P ... of
course, it is a smaller maintenance footprint, so maybe it is still
better.
> The question of whether to drop ra_neon is independent, in my mind, from
> this Ev2 matter. Subversion must continue to work for our users. That's
> not optional.
Definitely othogonal. Agreed. It is simply that the Ev2 work is making
the pain more acute.
> Ev2 is optional, infinitely delay-able. Clearly, we want to move forward
> with Ev2 because of the additional benefit we believe it will bring to our
> users, so we've an incentive to remove barriers to that progress. One such
> barrier could be "the cost of coding Ev2 support in ra_neon". I kinda doubt
> it, but I'm willing to defer to those closer to the problem.
It's a SMOP. Sure, we can do Ev2 for ra_neon, or we could rely on the
shims. We might be able to find a blended approach such as a custom
shim that avoids the big issues, yet defers to the Ev1 commit editor
for the bulk of the work.
>
> At any rate, I suggest we make the call on ra_neon/ra_serf first. Now.
Right. Thus my intent to start the discussion. Give it a few days or
so, see where it goes.
>...
> If ra_serf covers the feature set of ra_neon, I'm fine with ditching
> ra_neon.
I believe it does.
Philip has raised the traffic size issue (or alternatively CPU to
enable mod_deflate); dunno whether he would characterize that as a
blocker, however. There is also an issue (3981) that seems to cause
hangs over https; not sure how prevalent that is, or if we can narrow
it down. I suspect the traffic size can be corrected for checkouts by
switch to Depth:1 PROPFIND requests. The hang that philip observes
seems repeatable, so fixable.
> There may come a day when I (or Ivan, or someone ...) re-adds the
> non-skelta mode back to our DAV layer as an option if good technical reason
> to do so is established, and should that day come, I trust that no one will
> stand in the way of that for non-technical reasons.
I remain dubious, but that's just another discussion about tradeoffs.
I dunno that it has specific bearing right now.
>...
So back to the original discussion, I see two points:
1) conceptually, is anybody opposed to deleting ra_neon?
2) what blockers to this path exist?
And related: is there any support for keeping ra_neon? I haven't seen
any calls for that on this thread, other than to mention the
possibility.
Thanks,
-g
Re: [DISCUSS] delete ra_neon
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 05/16/2012 10:13 AM, Greg Stein wrote:
> On Wed, May 16, 2012 at 9:43 AM, C. Michael Pilato <cm...@collab.net> wrote:
>> but: How hard would it be to temporarily maintain two commit paths -- one
>> that drives an Ev2 editor, and one that drives Ev1? The call to
>> svn_ra_get_commit_editorN() would return NOT_IMPLEMENTED from ra_neon (and
>> ra_svn), and the client would detect this and call the previous
>> Ev1-returning API and drive it accordingly?
>
> Certainly doable. And then it leads to a new question: "how long do we
> support these two commit drive code paths?", and it does not provide
> an answer for ra_neon itself.
We support two commit drive code paths until we need only support one. (Duh?)
The question of whether to drop ra_neon is independent, in my mind, from
this Ev2 matter. Subversion must continue to work for our users. That's
not optional.
Ev2 is optional, infinitely delay-able. Clearly, we want to move forward
with Ev2 because of the additional benefit we believe it will bring to our
users, so we've an incentive to remove barriers to that progress. One such
barrier could be "the cost of coding Ev2 support in ra_neon". I kinda doubt
it, but I'm willing to defer to those closer to the problem.
At any rate, I suggest we make the call on ra_neon/ra_serf first. Now.
If the call is "keep 'em both", then we do so, and we stop subconsciously
granting first-class citizenship to one or the other of our DAV RA modules.
If the call is "drop ra_neon", then we do so. Any features ra_serf lacks
that ra_neon offers (and which Subversion uses) become hard blockers to
Subversion 1.8.
If the call is "drop ra_serf", then we do so. Any features ra_neon lacks
that ra_serf offers (and which Subversion uses) become hard blockers to
Subversion 1.8.
The Ev2 stuff will be managed in accordance with the preexisting policies of
this community: trunk is stable, period. If we keep ra_neon, then Hyrum
must ensure that his Ev2 stuff continues to function with all RA layers
before it lands on trunk (through shims, or a dual commit drive approach, or
whatever). Let's not overcomplicate this.
If ra_serf covers the feature set of ra_neon, I'm fine with ditching
ra_neon. There may come a day when I (or Ivan, or someone ...) re-adds the
non-skelta mode back to our DAV layer as an option if good technical reason
to do so is established, and should that day come, I trust that no one will
stand in the way of that for non-technical reasons. That's about the only
semi-contentious thing I can recall in terms of the general approach that
ra_serf takes. Cool?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: [DISCUSS] delete ra_neon
Posted by Greg Stein <gs...@gmail.com>.
On Wed, May 16, 2012 at 9:43 AM, C. Michael Pilato <cm...@collab.net> wrote:
> On 05/16/2012 09:33 AM, Hyrum K Wright wrote:
>> On Wed, May 16, 2012 at 8:15 AM, C. Michael Pilato <cm...@collab.net> wrote:
>>> ...
>>> What to do about the commit Ev2 stuff? Perhaps you or Hyrum can help us
>>> understand the scope of the performance cost paid by employing editor shims?
>>
>> The primary performance cost paid by Ev2 is that the entire edit is
>> queued up during translation, and then replayed upon completion.
>...
> Are the shims required at all?
They are required for any RA provider that does not offer an Ev2
commit editor. At the moment, ra_local is the only RA layer which
supports an Ev2 commit editor. ra_serf will provide one in the near
term. (and I hand-wave / hope that somebody will step up to code
something for ra_svn)
> I mean, my recollection of the commit
> process is that the client does a bunch of local investigation
> (harvest_committables, etc.), but the actual drive of the commit editor is
> handled in a single path-driver callback func. I don't have a sense of how
> much of that initial state-gathering stuff you had to change to get Ev2
> commit drives working, so the following question could just be ridiculous,
The gathering is independent of the commit drive. Bert has revamped
the gathering completely [on trunk], and Hyrum rebuilt the commit
drive on the ev2-export branch.
> but: How hard would it be to temporarily maintain two commit paths -- one
> that drives an Ev2 editor, and one that drives Ev1? The call to
> svn_ra_get_commit_editorN() would return NOT_IMPLEMENTED from ra_neon (and
> ra_svn), and the client would detect this and call the previous
> Ev1-returning API and drive it accordingly?
Certainly doable. And then it leads to a new question: "how long do we
support these two commit drive code paths?", and it does not provide
an answer for ra_neon itself.
Cheers,
-g
Re: [DISCUSS] delete ra_neon
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 05/16/2012 09:33 AM, Hyrum K Wright wrote:
> On Wed, May 16, 2012 at 8:15 AM, C. Michael Pilato <cm...@collab.net> wrote:
>> ...
>> What to do about the commit Ev2 stuff? Perhaps you or Hyrum can help us
>> understand the scope of the performance cost paid by employing editor shims?
>
> The primary performance cost paid by Ev2 is that the entire edit is
> queued up during translation, and then replayed upon completion. Some
> of this intermediate queuing happens in memory, other bits happen on
> disk, but suffice it to say that for large commits, such intermediate
> storage may be prohibitive. I don't think it would be a problem for
> day-to-day work, but for large merges which are common in some
> organizations I know of, such a performance penalty would be
> prohibitive.
>
> Greg's been hacking around in the shims, and has reduced this cost
> somewhat, but there is still a O(n) additional resource use cost to
> consumers of the shim code.
Are the shims required at all? I mean, my recollection of the commit
process is that the client does a bunch of local investigation
(harvest_committables, etc.), but the actual drive of the commit editor is
handled in a single path-driver callback func. I don't have a sense of how
much of that initial state-gathering stuff you had to change to get Ev2
commit drives working, so the following question could just be ridiculous,
but: How hard would it be to temporarily maintain two commit paths -- one
that drives an Ev2 editor, and one that drives Ev1? The call to
svn_ra_get_commit_editorN() would return NOT_IMPLEMENTED from ra_neon (and
ra_svn), and the client would detect this and call the previous
Ev1-returning API and drive it accordingly?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: [DISCUSS] delete ra_neon
Posted by Hyrum K Wright <hy...@wandisco.com>.
On Wed, May 16, 2012 at 8:15 AM, C. Michael Pilato <cm...@collab.net> wrote:
>...
> What to do about the commit Ev2 stuff? Perhaps you or Hyrum can help us
> understand the scope of the performance cost paid by employing editor shims?
The primary performance cost paid by Ev2 is that the entire edit is
queued up during translation, and then replayed upon completion. Some
of this intermediate queuing happens in memory, other bits happen on
disk, but suffice it to say that for large commits, such intermediate
storage may be prohibitive. I don't think it would be a problem for
day-to-day work, but for large merges which are common in some
organizations I know of, such a performance penalty would be
prohibitive.
Greg's been hacking around in the shims, and has reduced this cost
somewhat, but there is still a O(n) additional resource use cost to
consumers of the shim code.
-Hyrum
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/
Re: [DISCUSS] delete ra_neon
Posted by Mark Phippard <ma...@gmail.com>.
On Wed, May 16, 2012 at 9:25 AM, Greg Stein <gs...@gmail.com> wrote:
> [avoiding responding, in favor of collecting feedback, but one item...]
>
> On Wed, May 16, 2012 at 9:15 AM, C. Michael Pilato <cm...@collab.net> wrote:
>>...
>> Having not reviewed the issues myself, I can only submit the following
>> litmus test of ra_serf readiness: can it do everything that our current
>> user base expects ra_neon to do? I don't see any reason to make it more
>
> I think Mark tracks this. iirc, serf does not do "smart card" authn,
> or somesuch.
No, it wasn't me. I do recall some people posting items like this but
I do not remember who.
Interestingly, GMail is suggesting I add Ivan to this email and that
is also what I was thinking. I recall that VisualSVN advertises smart
card support so he might have some info.
On Windows, most of the SVN binaries are supporting Smart Cards via
OpenSSL. It can be built in a way that it works with the Windows
CryptoAPI which adds smart card support. This all happens in the SSL
negotiation outside of Subversion so it should work with Serf as well
as Neon. On Unix, I have no idea how people do it, except I do recall
Joe Orton or someone posting about using Neon with pakchois or some
other library that does this. AFAIK, this requires doing a custom
build of the stack for your specific smart card implementation.
FWIW, a downside of using OpenSSL to do this is that our servers file
is ignored so you cannot configure a specific cert in that file as you
could if OpenSSL is not compiled to use that API. We have a few
customers this is a problem with because they use Smart Cards for some
stuff on their LAN, but for SVN they are using client certs. They are
always prompted to insert their Smart Card and that is not what they
want. TortoiseSVN applies a custom patch to OpenSSL to allow this to
be turned on/off at runtime but that approach does not work for the
command line binaries.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: [DISCUSS] delete ra_neon
Posted by Greg Stein <gs...@gmail.com>.
[avoiding responding, in favor of collecting feedback, but one item...]
On Wed, May 16, 2012 at 9:15 AM, C. Michael Pilato <cm...@collab.net> wrote:
>...
> Having not reviewed the issues myself, I can only submit the following
> litmus test of ra_serf readiness: can it do everything that our current
> user base expects ra_neon to do? I don't see any reason to make it more
I think Mark tracks this. iirc, serf does not do "smart card" authn,
or somesuch.
Mark?
Re: [DISCUSS] delete ra_neon
Posted by Mark Phippard <ma...@gmail.com>.
On Wed, May 16, 2012 at 9:46 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:
> On Wed, May 16, 2012 at 5:15 PM, C. Michael Pilato <cm...@collab.net> wrote:
>> On 05/16/2012 06:46 AM, Greg Stein wrote:
>>> Short query: is this possible?
>>
>> I know of no convincing reason why we cannot someday delete libsvn_ra_neon.
>> I am very much in favor of getting back to a single DAV provider. I
>> believe that ra_serf offers us the best path forward along those lines, not
>> because it is demonstrably "better", but because it has our developer
>> inertia and we've a tighter cross-community connection with the Serf project
>> than we do the Neon project.
>>
>>> We have some issues filed against ra_serf. Which of them should be
>>> considered as blockers?
>>
>> Having not reviewed the issues myself, I can only submit the following
>> litmus test of ra_serf readiness: can it do everything that our current
>> user base expects ra_neon to do? I don't see any reason to make it more
>> complicated than that. I strongly wish to *not* have to tell any user,
>> "Sorry, you can't upgrade to Subversion 1.8 because we decided to drop
>> support for that Windows Kerberos NTLM Snozwaggle 8.2 support you were using
>> in ra_neon that ra_serf doesn't offer." Until that time, ra_neon must remain.
>>
> Serf has full Kerberos/NTLM implementation on Windows and *nix. So
> it's not issue. Probably implementation has some bugs, but I've used
> it for along time without serious issues.
>
> I didn't tested but most likely serf doesn't support OpenSSL
> authentication using client certificates stored smart cards. But this
> is different issue than Kerberos/NTLM.
We have some customers that use Smart Cards and I know they work on
Windows by letting OpenSSL handle it. I do not know what these
customers are doing on Unix. I assume they have a way of getting an
SSL client cert and use that instead? Here is an old thread where Joe
helped a user get a smart card working on Red Hat:
http://svn.haxx.se/users/archive-2008-03/0980.shtml
It is hard to tell whether there are many people successfully doing
this. AFAIK, there are no out of the box solutions for this where,
for example, you simply need to install RHEL 6.1 with the RPM's it
provides and this will all just work.
As long as Serf + OpenSSL still works on Windows for smart cards, I am
guessing the lack of support on Unix is not a big step backwards.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: [DISCUSS] delete ra_neon
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Wed, May 16, 2012 at 5:15 PM, C. Michael Pilato <cm...@collab.net> wrote:
> On 05/16/2012 06:46 AM, Greg Stein wrote:
>> Short query: is this possible?
>
> I know of no convincing reason why we cannot someday delete libsvn_ra_neon.
> I am very much in favor of getting back to a single DAV provider. I
> believe that ra_serf offers us the best path forward along those lines, not
> because it is demonstrably "better", but because it has our developer
> inertia and we've a tighter cross-community connection with the Serf project
> than we do the Neon project.
>
>> We have some issues filed against ra_serf. Which of them should be
>> considered as blockers?
>
> Having not reviewed the issues myself, I can only submit the following
> litmus test of ra_serf readiness: can it do everything that our current
> user base expects ra_neon to do? I don't see any reason to make it more
> complicated than that. I strongly wish to *not* have to tell any user,
> "Sorry, you can't upgrade to Subversion 1.8 because we decided to drop
> support for that Windows Kerberos NTLM Snozwaggle 8.2 support you were using
> in ra_neon that ra_serf doesn't offer." Until that time, ra_neon must remain.
>
Serf has full Kerberos/NTLM implementation on Windows and *nix. So
it's not issue. Probably implementation has some bugs, but I've used
it for along time without serious issues.
I didn't tested but most likely serf doesn't support OpenSSL
authentication using client certificates stored smart cards. But this
is different issue than Kerberos/NTLM.
--
Ivan Zhakov
Re: [DISCUSS] delete ra_neon
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 05/16/2012 06:46 AM, Greg Stein wrote:
> Short query: is this possible?
I know of no convincing reason why we cannot someday delete libsvn_ra_neon.
I am very much in favor of getting back to a single DAV provider. I
believe that ra_serf offers us the best path forward along those lines, not
because it is demonstrably "better", but because it has our developer
inertia and we've a tighter cross-community connection with the Serf project
than we do the Neon project.
> We have some issues filed against ra_serf. Which of them should be
> considered as blockers?
Having not reviewed the issues myself, I can only submit the following
litmus test of ra_serf readiness: can it do everything that our current
user base expects ra_neon to do? I don't see any reason to make it more
complicated than that. I strongly wish to *not* have to tell any user,
"Sorry, you can't upgrade to Subversion 1.8 because we decided to drop
support for that Windows Kerberos NTLM Snozwaggle 8.2 support you were using
in ra_neon that ra_serf doesn't offer." Until that time, ra_neon must remain.
What to do about the commit Ev2 stuff? Perhaps you or Hyrum can help us
understand the scope of the performance cost paid by employing editor shims?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: [DISCUSS] delete ra_neon
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 05/19/2012 03:20 PM, Justin Erenkrantz wrote:
> On Sat, May 19, 2012 at 10:34 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>> I'm still in favor to include properties in update REPORT (Mike's
>> approach). PROPFINDs are not cached by proxies and working copy. There
>> are relatively small, so there is no real benefit to fetch them using
>> separate request.
>
> If you do that, then you have to store all of the properties in-memory
> for the entire checkout or serialize on to disk or something
> similar...ugh.
Yeah, we still have the cache-the-props-till-we-can-use-them issue with this
approach. Only a full-fledged "send-all" REPORT mode lets us avoid that.
> I'd also be curious how much extra time it takes to include the
> properties in the REPORT response - it's not going to be free and may
> not even be cheap. =( -- justin
Nah, my timings seemed to indicate that including the properties in the
REPORT response costs next-to-nothing. The server-side editor is already
feeding them to mod_dav_svn, it's just that in "skelta" mode, mod_dav_svn
drops the data and assumes the client will PROPFIND for it again later.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: [DISCUSS] delete ra_neon
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Sat, May 19, 2012 at 10:34 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:
> I'm still in favor to include properties in update REPORT (Mike's
> approach). PROPFINDs are not cached by proxies and working copy. There
> are relatively small, so there is no real benefit to fetch them using
> separate request.
If you do that, then you have to store all of the properties in-memory
for the entire checkout or serialize on to disk or something
similar...ugh.
I'd also be curious how much extra time it takes to include the
properties in the REPORT response - it's not going to be free and may
not even be cheap. =( -- justin
Re: [DISCUSS] delete ra_neon
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Sat, May 19, 2012 at 8:28 PM, Justin Erenkrantz
<ju...@erenkrantz.com> wrote:
> On Sat, May 19, 2012 at 9:19 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>> As far I know, Greg current approach is to make node operation atomic
>> and allow each node modified only in one call during the edit.
>
> Yah, that seems to be the consensus. If that's the case, yes, you are
> right and we would have to store the properties in memory within
> ra_serf for all of the properties for every entry in a directory. So,
> the memory usage for larger directories could go up...but, it'd only
> be the properties not the fulltext content - so, it shouldn't be too
> awful, but it'll quite likely be noticeable. Depending upon how we do
> the request parallelization, it's possible we could have a bunch of
> directories in-flight further increasing the overhead as we'd have
> multiple PROPFIND responses for large directories sitting in memory
> waiting for their fulltext to arrive.
>
> My take is that'll be a fair tradeoff, but I'd be happy to hear what
> others think. -- justin
I'm still in favor to include properties in update REPORT (Mike's
approach). PROPFINDs are not cached by proxies and working copy. There
are relatively small, so there is no real benefit to fetch them using
separate request.
--
Ivan Zhakov
Re: [DISCUSS] delete ra_neon
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Sat, May 19, 2012 at 9:19 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:
> As far I know, Greg current approach is to make node operation atomic
> and allow each node modified only in one call during the edit.
Yah, that seems to be the consensus. If that's the case, yes, you are
right and we would have to store the properties in memory within
ra_serf for all of the properties for every entry in a directory. So,
the memory usage for larger directories could go up...but, it'd only
be the properties not the fulltext content - so, it shouldn't be too
awful, but it'll quite likely be noticeable. Depending upon how we do
the request parallelization, it's possible we could have a bunch of
directories in-flight further increasing the overhead as we'd have
multiple PROPFIND responses for large directories sitting in memory
waiting for their fulltext to arrive.
My take is that'll be a fair tradeoff, but I'd be happy to hear what
others think. -- justin
Re: [DISCUSS] delete ra_neon
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Sat, May 19, 2012 at 8:12 PM, Justin Erenkrantz
<ju...@erenkrantz.com> wrote:
> On Sat, May 19, 2012 at 8:15 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>> PROPFIND per directory has another problem: we have to save properties
>> for all children of directory in memory.
>
> This ultimately depends upon how Ev2 structures it - if we can't feed
> the editor with the properties until the individual children are
> processed, then yes, we have to stash it in memory (within the RA
> layer) until we process the nodes. However, if we can deliver all of
> the properties via Ev2 as soon as the PROPFIND response comes in, we
> don't have that concern - ultimately, that makes it libsvn_wc's issue
> about how to handle the atomicity/incompleteness issue - which I
> *think* would be far more solvable...but YMMV. -- justin
As far I know, Greg current approach is to make node operation atomic
and allow each node modified only in one call during the edit.
--
Ivan Zhakov
Re: [DISCUSS] delete ra_neon
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Sat, May 19, 2012 at 8:15 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:
> PROPFIND per directory has another problem: we have to save properties
> for all children of directory in memory.
This ultimately depends upon how Ev2 structures it - if we can't feed
the editor with the properties until the individual children are
processed, then yes, we have to stash it in memory (within the RA
layer) until we process the nodes. However, if we can deliver all of
the properties via Ev2 as soon as the PROPFIND response comes in, we
don't have that concern - ultimately, that makes it libsvn_wc's issue
about how to handle the atomicity/incompleteness issue - which I
*think* would be far more solvable...but YMMV. -- justin
Re: [DISCUSS] delete ra_neon
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Sat, May 19, 2012 at 12:42 AM, C. Michael Pilato <cm...@collab.net> wrote:
> On 05/18/2012 04:15 PM, Ivan Zhakov wrote:
>> On Fri, May 18, 2012 at 12:49 AM, Greg Stein <gs...@gmail.com> wrote:
>>> On Thu, May 17, 2012 at 4:47 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>>>> ...
>>>> I meant one PROPFIND per directory. I didn't see where PROPFIND with
>>>> "1" depth is created. It used only in svn_ra_serf__get_dir() but not
>>>> in update driver code.
>>>
>>> Today, we do a PROPFIND per node. I want to change it to
>>> per-directory. IOW, on my todo list. (there is an open issue about it)
>>>
>> This is future plan, but Mike said that we currently send PROPFIND
>> with depth "1" for each added directory.
>
> I was referring to Greg's future plan using present-tense verbs. Sorry for
> the confusion.
>
> Today, we send one depth-zero PROPFIND per node (files and directories both).
>
> I have a patch which reduces this to one depth-zero PROPFIND per directory
> (because file props are delivered inline in the REPORT response).
>
> Greg was suggesting that we not bother with my patch, because he wants to
> also reduce this to one PROPFIND per directory, but those PROPFINDs would be
> depth-one. So, all properties would still be delivered via PROPFINDs (as
> opposed to inline in the REPORT respones) as are today, but we'd use one
> (larger, depth-one) PROPFIND per directory instead of one PROPFIND per
> directory-and-file.
>
PROPFIND per directory has another problem: we have to save properties
for all children of directory in memory.
--
Ivan Zhakov
Re: [DISCUSS] delete ra_neon
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Fri, May 18, 2012 at 1:42 PM, C. Michael Pilato <cm...@collab.net> wrote:
> On 05/18/2012 04:15 PM, Ivan Zhakov wrote:
>> On Fri, May 18, 2012 at 12:49 AM, Greg Stein <gs...@gmail.com> wrote:
>>> On Thu, May 17, 2012 at 4:47 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>>>> ...
>>>> I meant one PROPFIND per directory. I didn't see where PROPFIND with
>>>> "1" depth is created. It used only in svn_ra_serf__get_dir() but not
>>>> in update driver code.
>>>
>>> Today, we do a PROPFIND per node. I want to change it to
>>> per-directory. IOW, on my todo list. (there is an open issue about it)
>>>
>> This is future plan, but Mike said that we currently send PROPFIND
>> with depth "1" for each added directory.
>
> I was referring to Greg's future plan using present-tense verbs. Sorry for
> the confusion.
>
> Today, we send one depth-zero PROPFIND per node (files and directories both).
>
> I have a patch which reduces this to one depth-zero PROPFIND per directory
> (because file props are delivered inline in the REPORT response).
>
> Greg was suggesting that we not bother with my patch, because he wants to
> also reduce this to one PROPFIND per directory, but those PROPFINDs would be
> depth-one. So, all properties would still be delivered via PROPFINDs (as
> opposed to inline in the REPORT respones) as are today, but we'd use one
> (larger, depth-one) PROPFIND per directory instead of one PROPFIND per
> directory-and-file.
>
> And if this fails to clear things up, too bad -- I quit and we'll just talk
> about in Berlin. :-P
Unless I'm completely blind, I'm not seeing the patch on-list. But,
yay, for imaginary patches! =)
I wouldn't be terribly concerned about mergeinfo or property sizes -
the data has to get sent to the client *somehow* - it's just a
question of what's the most efficient way to send it. It's not the RA
layer's issue if the properties are big...IOW, we should resolve that
somewhere else. =P
Apropos of my other post to dev@ I just sent, I'm curious how your
patch handles the editor drive issues. Would we be leaking handles if
we set the properties like that? -- justin
Re: [DISCUSS] delete ra_neon
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 05/18/2012 04:15 PM, Ivan Zhakov wrote:
> On Fri, May 18, 2012 at 12:49 AM, Greg Stein <gs...@gmail.com> wrote:
>> On Thu, May 17, 2012 at 4:47 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>>> ...
>>> I meant one PROPFIND per directory. I didn't see where PROPFIND with
>>> "1" depth is created. It used only in svn_ra_serf__get_dir() but not
>>> in update driver code.
>>
>> Today, we do a PROPFIND per node. I want to change it to
>> per-directory. IOW, on my todo list. (there is an open issue about it)
>>
> This is future plan, but Mike said that we currently send PROPFIND
> with depth "1" for each added directory.
I was referring to Greg's future plan using present-tense verbs. Sorry for
the confusion.
Today, we send one depth-zero PROPFIND per node (files and directories both).
I have a patch which reduces this to one depth-zero PROPFIND per directory
(because file props are delivered inline in the REPORT response).
Greg was suggesting that we not bother with my patch, because he wants to
also reduce this to one PROPFIND per directory, but those PROPFINDs would be
depth-one. So, all properties would still be delivered via PROPFINDs (as
opposed to inline in the REPORT respones) as are today, but we'd use one
(larger, depth-one) PROPFIND per directory instead of one PROPFIND per
directory-and-file.
And if this fails to clear things up, too bad -- I quit and we'll just talk
about in Berlin. :-P
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: [DISCUSS] delete ra_neon
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Fri, May 18, 2012 at 12:49 AM, Greg Stein <gs...@gmail.com> wrote:
> On Thu, May 17, 2012 at 4:47 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>>...
>> I meant one PROPFIND per directory. I didn't see where PROPFIND with
>> "1" depth is created. It used only in svn_ra_serf__get_dir() but not
>> in update driver code.
>
> Today, we do a PROPFIND per node. I want to change it to
> per-directory. IOW, on my todo list. (there is an open issue about it)
>
This is future plan, but Mike said that we currently send PROPFIND
with depth "1" for each added directory.
--
Ivan Zhakov
Re: [DISCUSS] delete ra_neon
Posted by Greg Stein <gs...@gmail.com>.
On Thu, May 17, 2012 at 4:47 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>...
> I meant one PROPFIND per directory. I didn't see where PROPFIND with
> "1" depth is created. It used only in svn_ra_serf__get_dir() but not
> in update driver code.
Today, we do a PROPFIND per node. I want to change it to
per-directory. IOW, on my todo list. (there is an open issue about it)
Cheers,
-g
Re: [DISCUSS] delete ra_neon
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Fri, May 18, 2012 at 12:35 AM, C. Michael Pilato <cm...@collab.net> wrote:
> On 05/17/2012 04:26 PM, Ivan Zhakov wrote:
>> On Thu, May 17, 2012 at 8:02 PM, C. Michael Pilato <cm...@collab.net> wrote:
>>> On 05/17/2012 11:45 AM, Greg Stein wrote:
>>>>
>>>> On May 17, 2012 10:27 AM, "C. Michael Pilato" <cmpilato@collab.net
>>>> <ma...@collab.net>> wrote:
>>>>> ...
>>>>> No, I haven't yet taken a crack at removing the HEAD request. That will
>>>>> significantly help the fetch-from-pristine-cache situation, but this is a
>>>>> parallel line of optimization that needs to occur.
>>>>
>>>> Yup. Doable; not worried about it. Along with Depth:1 PROPFINDs. I'd like to
>>>> try the latter before patching. From your test, we see the checkout goes
>>>> from 1915 down to 203 PROPFINDs. I can make that happen against unpatched
>>>> servers.
>>>
>>> Yup, agreed that would make sense. Those 203 PROPFINDs would be bigger, but
>>> as I think we've established, the real cost here is in the turnaround, not
>>> the processing of any one request/response.
>>>
>> May be stupid question: why do we need these 203 PROPFINDs for each directory?
>
> It's not 203 PROPFINDs per directory. It's one PROPFIND per directory (at
> depth:1 ... gets all the props for that directory and its immediate children).
>
I meant one PROPFIND per directory. I didn't see where PROPFIND with
"1" depth is created. It used only in svn_ra_serf__get_dir() but not
in update driver code.
--
Ivan Zhakov
Re: [DISCUSS] delete ra_neon
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 05/17/2012 04:26 PM, Ivan Zhakov wrote:
> On Thu, May 17, 2012 at 8:02 PM, C. Michael Pilato <cm...@collab.net> wrote:
>> On 05/17/2012 11:45 AM, Greg Stein wrote:
>>>
>>> On May 17, 2012 10:27 AM, "C. Michael Pilato" <cmpilato@collab.net
>>> <ma...@collab.net>> wrote:
>>>> ...
>>>> No, I haven't yet taken a crack at removing the HEAD request. That will
>>>> significantly help the fetch-from-pristine-cache situation, but this is a
>>>> parallel line of optimization that needs to occur.
>>>
>>> Yup. Doable; not worried about it. Along with Depth:1 PROPFINDs. I'd like to
>>> try the latter before patching. From your test, we see the checkout goes
>>> from 1915 down to 203 PROPFINDs. I can make that happen against unpatched
>>> servers.
>>
>> Yup, agreed that would make sense. Those 203 PROPFINDs would be bigger, but
>> as I think we've established, the real cost here is in the turnaround, not
>> the processing of any one request/response.
>>
> May be stupid question: why do we need these 203 PROPFINDs for each directory?
It's not 203 PROPFINDs per directory. It's one PROPFIND per directory (at
depth:1 ... gets all the props for that directory and its immediate children).
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: [DISCUSS] delete ra_neon
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Thu, May 17, 2012 at 8:02 PM, C. Michael Pilato <cm...@collab.net> wrote:
> On 05/17/2012 11:45 AM, Greg Stein wrote:
>>
>> On May 17, 2012 10:27 AM, "C. Michael Pilato" <cmpilato@collab.net
>> <ma...@collab.net>> wrote:
>>>...
>>> No, I haven't yet taken a crack at removing the HEAD request. That will
>>> significantly help the fetch-from-pristine-cache situation, but this is a
>>> parallel line of optimization that needs to occur.
>>
>> Yup. Doable; not worried about it. Along with Depth:1 PROPFINDs. I'd like to
>> try the latter before patching. From your test, we see the checkout goes
>> from 1915 down to 203 PROPFINDs. I can make that happen against unpatched
>> servers.
>
> Yup, agreed that would make sense. Those 203 PROPFINDs would be bigger, but
> as I think we've established, the real cost here is in the turnaround, not
> the processing of any one request/response.
>
May be stupid question: why do we need these 203 PROPFINDs for each directory?
--
Ivan Zhakov
Re: [DISCUSS] delete ra_neon
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 05/17/2012 11:45 AM, Greg Stein wrote:
>
> On May 17, 2012 10:27 AM, "C. Michael Pilato" <cmpilato@collab.net
> <ma...@collab.net>> wrote:
>>...
>> No, I haven't yet taken a crack at removing the HEAD request. That will
>> significantly help the fetch-from-pristine-cache situation, but this is a
>> parallel line of optimization that needs to occur.
>
> Yup. Doable; not worried about it. Along with Depth:1 PROPFINDs. I'd like to
> try the latter before patching. From your test, we see the checkout goes
> from 1915 down to 203 PROPFINDs. I can make that happen against unpatched
> servers.
Yup, agreed that would make sense. Those 203 PROPFINDs would be bigger, but
as I think we've established, the real cost here is in the turnaround, not
the processing of any one request/response.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: [DISCUSS] delete ra_neon
Posted by Greg Stein <gs...@gmail.com>.
On May 17, 2012 10:27 AM, "C. Michael Pilato" <cm...@collab.net> wrote:
>...
> No, I haven't yet taken a crack at removing the HEAD request. That will
> significantly help the fetch-from-pristine-cache situation, but this is a
> parallel line of optimization that needs to occur.
Yup. Doable; not worried about it. Along with Depth:1 PROPFINDs. I'd like
to try the latter before patching. From your test, we see the checkout goes
from 1915 down to 203 PROPFINDs. I can make that happen against unpatched
servers.
Cheers,
-g
Re: [DISCUSS] delete ra_neon
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 05/17/2012 09:48 AM, Mark Phippard wrote:
> On Thu, May 17, 2012 at 9:45 AM, C. Michael Pilato <cm...@collab.net> wrote:
>> On 05/17/2012 03:43 AM, Justin Erenkrantz wrote:
>>> Again, the outstanding optimization to do on the ra_serf side is to
>>> reduce the number of PROPFIND requests and make that not proportional
>>> to the number of files - most likely either to the number of
>>> directories or only when custom properties are set. -- justin
>>
>> I've got a patch almost complete that:
>>
>> 1. teaches mod_dav_svn to send non-custom properties across the wire even in
>> skelta-mode, with a flag to say "Hey, there are some custom ones here, too!"
>>
>> 2. teaches ra_serf to assume that if the server is sending actual props
>> across the wire for a given file/dir, it shouldn't perform the PROPFIND
>> unless specifically told to do so by the server.
>>
>> I was toying with this a couple of week's ago when I made the
>> "fetch-from-the-pristine-cache" changes. I'll polish it up and post ASAP.
>
> Will this avoid the HEAD request too? I assume so.
No, I haven't yet taken a crack at removing the HEAD request. That will
significantly help the fetch-from-pristine-cache situation, but this is a
parallel line of optimization that needs to occur.
I tweaked my patch to send all svn: properties across the wire in the
REPORT, leaving only truly custom properties to be manually fetched. (Or so
I thought ... it looks like directories are getting PROPFINDs regardless ...
haven't looked into that yet.) I then ran three clean checkouts against my
local server, grabbing an old Subversion trunk directory from a local
repository. Here are the timings:
UNPATCHED SERVER, UNPATCHED CLIENT
real 13.74
user 1.86
sys 4.00
***
real 11.88
user 2.09
sys 3.18
***
real 12.60
user 2.08
sys 3.34
There were 1915 PROPFINDs in each of the three runs.
PATCHED SERVER, UNPATCHED CLIENT
real 13.83
user 2.06
sys 3.40
***
real 12.88
user 2.12
sys 3.00
***
real 11.22
user 1.96
sys 2.92
There were 1915 PROPFINDs in each of the three runs.
PATCHED SERVER, PATCHED CLIENT
real 10.27
user 1.80
sys 3.13
***
real 7.68
user 1.74
sys 3.82
***
real 7.14
user 1.76
sys 2.65
There was only 203 PROPFINDs (for directories)
To be clear, a "patched server" is one that does the inline property thing
regardless of whether the client is prepared to handle it. So, in the
patched server/unpatched client situation, the server is sending all those
properties across the wire but the client is still turning around and
PROPFINDing the same properties again. It appears that the "waste" in this
situation (including the properties in the REPORT response only to have them
ignored) is negligible -- the real cost here is truly in the PROPFINDs.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: [DISCUSS] delete ra_neon
Posted by Mark Phippard <ma...@gmail.com>.
On Thu, May 17, 2012 at 9:45 AM, C. Michael Pilato <cm...@collab.net> wrote:
> On 05/17/2012 03:43 AM, Justin Erenkrantz wrote:
>> Again, the outstanding optimization to do on the ra_serf side is to
>> reduce the number of PROPFIND requests and make that not proportional
>> to the number of files - most likely either to the number of
>> directories or only when custom properties are set. -- justin
>
> I've got a patch almost complete that:
>
> 1. teaches mod_dav_svn to send non-custom properties across the wire even in
> skelta-mode, with a flag to say "Hey, there are some custom ones here, too!"
>
> 2. teaches ra_serf to assume that if the server is sending actual props
> across the wire for a given file/dir, it shouldn't perform the PROPFIND
> unless specifically told to do so by the server.
>
> I was toying with this a couple of week's ago when I made the
> "fetch-from-the-pristine-cache" changes. I'll polish it up and post ASAP.
Will this avoid the HEAD request too? I assume so.
I was playing around with trunk last night after Greg fixed the XML
parsing bug I reported. Using that change you committed to avoid
fetching pristines, I saw checkout of a branch (for which I already
had all of the pristines) dropped from 2:50 to 1:35. The client still
sent the server a HEAD and PROPFIND for each file so getting rid of
all of those requests would have undoubtedly took a lot of additional
time off the command.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: [DISCUSS] delete ra_neon
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 05/17/2012 09:45 AM, C. Michael Pilato wrote:
> On 05/17/2012 03:43 AM, Justin Erenkrantz wrote:
>> Again, the outstanding optimization to do on the ra_serf side is to
>> reduce the number of PROPFIND requests and make that not proportional
>> to the number of files - most likely either to the number of
>> directories or only when custom properties are set. -- justin
>
> I've got a patch almost complete that:
>
> 1. teaches mod_dav_svn to send non-custom properties across the wire even in
> skelta-mode, with a flag to say "Hey, there are some custom ones here, too!"
>
> 2. teaches ra_serf to assume that if the server is sending actual props
> across the wire for a given file/dir, it shouldn't perform the PROPFIND
> unless specifically told to do so by the server.
>
> I was toying with this a couple of week's ago when I made the
> "fetch-from-the-pristine-cache" changes. I'll polish it up and post ASAP.
I just remembered why I abandoned this. My patch discriminated between
"user-visible" and "non user-visible" properties -- the "entry props" versus
everything else. I bailed when I realized that most of the files in our
tree have user-visible properties set on them (svn:eol-style, etc.), so
ra_serf would be dispatching PROPFINDs for those anyway.
So I though, well, what if I include all "svn:*" properties in the REPORT,
and only do PROPFINDs for truly custom properties. But then I realized
that, in all likeliness, the biggest property that ever gets passed around
through a Subversion system is one of our own: svn:mergeinfo. Next largest
is probably svn:ignore.
From there, I found myself wondering if there was meaningful (yet easily
maintainable and explainable) way to divvy up what gets sent across the wire
in the REPORT, and what must be explicitly PROP...FOUND. ;-)
Is it "include all svn:* properties whose values are smaller than X"?
Perhaps, "include (all) properties iff their combined value size is smaller
than Y"?
So, my patch has the necessary plumbing in place to make these sorts of
decisions. It just doesn't know what decision to make.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: [DISCUSS] delete ra_neon
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 05/17/2012 03:43 AM, Justin Erenkrantz wrote:
> Again, the outstanding optimization to do on the ra_serf side is to
> reduce the number of PROPFIND requests and make that not proportional
> to the number of files - most likely either to the number of
> directories or only when custom properties are set. -- justin
I've got a patch almost complete that:
1. teaches mod_dav_svn to send non-custom properties across the wire even in
skelta-mode, with a flag to say "Hey, there are some custom ones here, too!"
2. teaches ra_serf to assume that if the server is sending actual props
across the wire for a given file/dir, it shouldn't perform the PROPFIND
unless specifically told to do so by the server.
I was toying with this a couple of week's ago when I made the
"fetch-from-the-pristine-cache" changes. I'll polish it up and post ASAP.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: [DISCUSS] delete ra_neon
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Wed, May 16, 2012 at 12:12 PM, Philip Martin
<ph...@wandisco.com> wrote:
> I do. Look at the numbers in the issue. I compare neon and serf with
> mod-deflate. I compare neon and serf without mod-deflate. I compare
> neon and serf over https.
I maintain that's still not a fair comparison - but, let me elaborate
on that point as it goes into why I think we should move away from
ra_neon. In my mind, the overall goal of any RA layer should be to
leverage the network as efficiently as possible - but, ra_neon makes
it far harder to do that as everything is far too tightly coupled.
ra_serf is intentionally designed to work with other parts of httpd -
that leads to the avoidance of creating a custom fulltext compression
scheme. Obviously, when going against a known fulltext to both sides,
svndiff makes a lot of sense; and ra_serf/mod_dav_svn (and ra_neon
too) will still happily use that when the client indicates it has a
prior version. The test case in the issue is about the fresh checkout
case where there is no mutually known fulltext - here, svndiff isn't
as useful and actually hurts ra_neon quite a bit.
Since ra_neon does not have that code reuse as a goal, it
unconditionally relies upon svn_txdelta_to_svndiff3 which is a wrapper
around zlib and then base64-encodes the whole svndiff3 output before
sending it on the wire. Therefore, ra_neon can't avoid compressing
the fulltext on the wire unless you set SVNCompressionLevel to 0. So,
what we see on the wire with ra_neon is always the compressed fulltext
base64-encoded - so, of course, ra_neon's traffic will be smaller than
if we disable mod_deflate for ra_serf. To be clear, in this scenario,
ra_neon is still doing a zlib pass while ra_serf is not doing any
compression. However, keep in mind that ra_neon is actually *helped*
from a network-traffic perspective by mod_deflate as
double-compression mitigates the base64 space penalty...but, that's
still sub-optimal from perfect network conditions as we are deflating
a base64 representation of something that is already deflated. By
removing this indirection and inefficiency, I think we can ultimately
get closer to the ideal network conditions with ra_serf than with
ra_neon.
Also, keep in mind that svn and mod_deflate have different default
zlib compression level defaults (5 vs 6). Part of any skew could be
that SVN defaults to 5 and mod_deflate defaults to 6...so, it may not
be quite symmetric. As Daniel pointed out, most web servers today are
not going to be CPU-bound - they are typically network and I/O bound.
And, many many large sites will do deflate at the edge anyway...so,
they would incur the zlib penalty elsewhere.
Again, the outstanding optimization to do on the ra_serf side is to
reduce the number of PROPFIND requests and make that not proportional
to the number of files - most likely either to the number of
directories or only when custom properties are set. -- justin
Re: [DISCUSS] delete ra_neon
Posted by Philip Martin <ph...@wandisco.com>.
Justin Erenkrantz <ju...@erenkrantz.com> writes:
> On Wed, May 16, 2012 at 11:59 AM, Philip Martin
> <ph...@wandisco.com> wrote:
>> Those look like the numbers with mod-deflate in the path. Although
>> mod-deflate reduces the traffic it pushes up the server CPU because serf
>> is pushing so much more data through the compressor. It's not much of a
>> selling point that serf only increases traffic by 20% if it also
>> increases CPU.
>
> IIRC, ra_neon is using a compressor too (native svndiff stuff with the
> base64/XML-escaping) - so, it's not quite fair to say that neon isn't
> doing pretty much the same thing...just in a different code path that
> isn't controlled by mod_deflate. So, I think you need to compare
> apples-to-apples. -- justin
I do. Look at the numbers in the issue. I compare neon and serf with
mod-deflate. I compare neon and serf without mod-deflate. I compare
neon and serf over https.
Serf might be slightly better on CPU but only when the traffic is
doubled. If we enable mod-deflate to reduce the traffic difference
between neon and serf it makes the serf CPU go up much more than the
neon CPU goes up. I can't see any situation where serf is a clear
winner.
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Re: [DISCUSS] delete ra_neon
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Wed, May 16, 2012 at 11:59 AM, Philip Martin
<ph...@wandisco.com> wrote:
> Those look like the numbers with mod-deflate in the path. Although
> mod-deflate reduces the traffic it pushes up the server CPU because serf
> is pushing so much more data through the compressor. It's not much of a
> selling point that serf only increases traffic by 20% if it also
> increases CPU.
IIRC, ra_neon is using a compressor too (native svndiff stuff with the
base64/XML-escaping) - so, it's not quite fair to say that neon isn't
doing pretty much the same thing...just in a different code path that
isn't controlled by mod_deflate. So, I think you need to compare
apples-to-apples. -- justin
Re: [DISCUSS] delete ra_neon
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Sat, May 19, 2012 at 2:37 PM, Daniel Shahaf <da...@elego.de> wrote:
> Tried to extract some info from the logs.. so far found that 60% of the
> requests have Accept-Encoding: gzip, but haven't analysed the remaining
> ones to see how to do they distribute (or even what fraction of them is
> an svn client).
I can't think of any modern browser or downstream packaged svn that
wouldn't have zlib available...pure speculation on my part, but
perhaps SVNKit doesn't utilize deflate? I don't know what Java HTTP
library SVNKit is using - it may use HC which should be pretty
straightforward to enable deflate, but they may just never have cared
about offering that.
The ASF has a lot of Java weenies flying about with Eclipse... *duck* -- justin
Re: [DISCUSS] delete ra_neon
Posted by Daniel Shahaf <da...@elego.de>.
Justin Erenkrantz wrote on Sat, May 19, 2012 at 07:38:19 -0700:
> On Sat, May 19, 2012 at 7:30 AM, Daniel Shahaf <da...@elego.de> wrote:
> >> I suppose most clients do support compression these days.
> >
> > How can we determine how many clients support compression? Is it
> > determined by the presence or absence of the "Accept-Encoding: gzip"
> > header? Are there other ways for a client to indicate "I support
> > compression"?
>
> Yah, Accept-Encoding is the appropriate signal from the client. -- justin
Tried to extract some info from the logs.. so far found that 60% of the
requests have Accept-Encoding: gzip, but haven't analysed the remaining
ones to see how to do they distribute (or even what fraction of them is
an svn client).
Re: [DISCUSS] delete ra_neon
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Sat, May 19, 2012 at 7:30 AM, Daniel Shahaf <da...@elego.de> wrote:
>> I suppose most clients do support compression these days.
>
> How can we determine how many clients support compression? Is it
> determined by the presence or absence of the "Accept-Encoding: gzip"
> header? Are there other ways for a client to indicate "I support
> compression"?
Yah, Accept-Encoding is the appropriate signal from the client. -- justin
Re: [DISCUSS] delete ra_neon
Posted by Daniel Shahaf <da...@elego.de>.
Philip Martin wrote on Fri, May 18, 2012 at 14:15:39 +0100:
> Daniel Shahaf <da...@elego.de> writes:
>
> > Yes, svn.apache.org uses mod_deflate.
>
> And apache 2.2 which means it doesn't have r1103315 which fixes the
> mod_deflate+mod_dav_svn memory leak when a client doesn't support
> compression.
>
> I suppose most clients do support compression these days.
How can we determine how many clients support compression? Is it
determined by the presence or absence of the "Accept-Encoding: gzip"
header? Are there other ways for a client to indicate "I support
compression"?
Re: [DISCUSS] delete ra_neon
Posted by Philip Martin <ph...@wandisco.com>.
Daniel Shahaf <da...@elego.de> writes:
> Yes, svn.apache.org uses mod_deflate.
And apache 2.2 which means it doesn't have r1103315 which fixes the
mod_deflate+mod_dav_svn memory leak when a client doesn't support
compression.
I suppose most clients do support compression these days.
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Re: [DISCUSS] delete ra_neon
Posted by Daniel Shahaf <da...@elego.de>.
Philip Martin wrote on Wed, May 16, 2012 at 19:59:15 +0100:
> Justin Erenkrantz <ju...@erenkrantz.com> writes:
>
> > On Wed, May 16, 2012 at 11:17 AM, Daniel Shahaf <da...@elego.de> wrote:
> >> Perhaps I wasn't clear. The traffic using ra_serf is 2.2 times as much
> >> as using ra_neon; see the (currently) last comment on the issue Philip
> >> links to.
> >
> > I definitely don't see that locally - I only see about a 20-25% gap -
> > which from looking at the traces appears to generally be due to the
> > extra PROPFIND requests.
> >
> > Here's what I see for a checkout of svn trunk -
> > http(s)://svn.apache.org/repos/asf/subversion/trunk
> >
> > Neon (HTTP): 17294674 bytes
> > Neon (HTTPS): 17543641 bytes
> >
> > Serf (HTTP): 21892977 bytes
> > Serf (HTTPS): 19399160 bytes
> >
> > The reason HTTPS for ra_serf is smaller is that Serf and OpenSSL is a
> > pretty optimized path based on network packets... -- justin
>
> Those look like the numbers with mod-deflate in the path. Although
> mod-deflate reduces the traffic it pushes up the server CPU because serf
> is pushing so much more data through the compressor. It's not much of a
> selling point that serf only increases traffic by 20% if it also
> increases CPU.
Yes, svn.apache.org uses mod_deflate.
I believe svn.a.o is IO-bound, not CPU-bound.
Re: [DISCUSS] delete ra_neon
Posted by Philip Martin <ph...@wandisco.com>.
Justin Erenkrantz <ju...@erenkrantz.com> writes:
> On Wed, May 16, 2012 at 11:17 AM, Daniel Shahaf <da...@elego.de> wrote:
>> Perhaps I wasn't clear. The traffic using ra_serf is 2.2 times as much
>> as using ra_neon; see the (currently) last comment on the issue Philip
>> links to.
>
> I definitely don't see that locally - I only see about a 20-25% gap -
> which from looking at the traces appears to generally be due to the
> extra PROPFIND requests.
>
> Here's what I see for a checkout of svn trunk -
> http(s)://svn.apache.org/repos/asf/subversion/trunk
>
> Neon (HTTP): 17294674 bytes
> Neon (HTTPS): 17543641 bytes
>
> Serf (HTTP): 21892977 bytes
> Serf (HTTPS): 19399160 bytes
>
> The reason HTTPS for ra_serf is smaller is that Serf and OpenSSL is a
> pretty optimized path based on network packets... -- justin
Those look like the numbers with mod-deflate in the path. Although
mod-deflate reduces the traffic it pushes up the server CPU because serf
is pushing so much more data through the compressor. It's not much of a
selling point that serf only increases traffic by 20% if it also
increases CPU.
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Re: [DISCUSS] delete ra_neon
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Wed, May 16, 2012 at 11:17 AM, Daniel Shahaf <da...@elego.de> wrote:
> Perhaps I wasn't clear. The traffic using ra_serf is 2.2 times as much
> as using ra_neon; see the (currently) last comment on the issue Philip
> links to.
I definitely don't see that locally - I only see about a 20-25% gap -
which from looking at the traces appears to generally be due to the
extra PROPFIND requests.
Here's what I see for a checkout of svn trunk -
http(s)://svn.apache.org/repos/asf/subversion/trunk
Neon (HTTP): 17294674 bytes
Neon (HTTPS): 17543641 bytes
Serf (HTTP): 21892977 bytes
Serf (HTTPS): 19399160 bytes
The reason HTTPS for ra_serf is smaller is that Serf and OpenSSL is a
pretty optimized path based on network packets... -- justin
Re: [DISCUSS] delete ra_neon
Posted by Daniel Shahaf <da...@elego.de>.
Justin Erenkrantz wrote on Wed, May 16, 2012 at 11:13:16 -0700:
> On Wed, May 16, 2012 at 10:48 AM, Daniel Shahaf <da...@elego.de> wrote:
> > Philip Martin wrote on Wed, May 16, 2012 at 18:39:37 +0100:
> >> The issue that worries me the most is the traffic issue:
> >>
> >> http://subversion.tigris.org/issues/show_bug.cgi?id=3980
> >
> > Your recent figures on that issue show a +122% increase in traffic. Do
> > we know what the worst case increase is? For example, does ra_serf
> > always require up to x% more traffic than ra_neon (for fixed x)?
>
> Looking at network traces from a current build, I don't think the
> PROPFIND issue Greg identified in the issue has been resolved
> yet...though I know there has been discussion on-list about it
> recently. A ~20% increase in traffic is quite likely due to that -
Perhaps I wasn't clear. The traffic using ra_serf is 2.2 times as much
as using ra_neon; see the (currently) last comment on the issue Philip
links to.
Re: [DISCUSS] delete ra_neon
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Wed, May 16, 2012 at 10:48 AM, Daniel Shahaf <da...@elego.de> wrote:
> Philip Martin wrote on Wed, May 16, 2012 at 18:39:37 +0100:
>> The issue that worries me the most is the traffic issue:
>>
>> http://subversion.tigris.org/issues/show_bug.cgi?id=3980
>
> Your recent figures on that issue show a +122% increase in traffic. Do
> we know what the worst case increase is? For example, does ra_serf
> always require up to x% more traffic than ra_neon (for fixed x)?
Looking at network traces from a current build, I don't think the
PROPFIND issue Greg identified in the issue has been resolved
yet...though I know there has been discussion on-list about it
recently. A ~20% increase in traffic is quite likely due to that -
though there will always be some extra traffic sent over due to the
fact that every file is requested separately - though that will mostly
manifest itself as inbound traffic to the server - not outbound
traffic from the server. The amount of traffic that the server has to
send back will be roughly the same modulo request overhead - but,
ra_serf actually spreads out the traffic amongst 4 requests - which is
typically a tradeoff a server is happy about. And, you don't have to
do the base64/XML-encoding dance either...
Generally speaking, the add'l requests for PROPFIND as-is right now
will be kept constant (one per WC file) - the GET's will vary due to
file size; so, the amount of network traffic due to the extra r/t's
for the PROPFIND won't be proportional to the overall on-disk size of
the WC - but only to the number of files in the WC. -- justin
Re: [DISCUSS] delete ra_neon
Posted by Daniel Shahaf <da...@elego.de>.
Philip Martin wrote on Wed, May 16, 2012 at 18:39:37 +0100:
> The issue that worries me the most is the traffic issue:
>
> http://subversion.tigris.org/issues/show_bug.cgi?id=3980
Your recent figures on that issue show a +122% increase in traffic. Do
we know what the worst case increase is? For example, does ra_serf
always require up to x% more traffic than ra_neon (for fixed x)?
Re: [DISCUSS] delete ra_neon
Posted by Philip Martin <ph...@wandisco.com>.
Greg Stein <gs...@gmail.com> writes:
> We have some issues filed against ra_serf. Which of them should be
> considered as blockers?
I still use neon because it's inconvenient that serf hangs every now and
then over https:
http://subversion.tigris.org/issues/show_bug.cgi?id=3981
The issue that worries me the most is the traffic issue:
http://subversion.tigris.org/issues/show_bug.cgi?id=3980
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Re: [DISCUSS] delete ra_neon
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Sat, May 19, 2012 at 6:39 PM, Justin Erenkrantz
<ju...@erenkrantz.com> wrote:
> On Sat, May 19, 2012 at 7:25 AM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
>>> Ewww. You are evil. Go away, and come back when you are less evil.
>>>
>>> Just kidding. But seriously ... evil.
>>
>> So why won't Ivan's idea work?
>>
>> (I assume it won't work, but now that it's been aired it would be nice
>> to hear from an ra_serf person _why_ it won't work.)
>
> Short answer: neon is a synchronous library while serf is an
> asynchronous library. The internal paradigms for ra_serf and ra_neon
> are completely different due to that.
In reality ra_serf uses synchronous request for everything except
update editor. And it's can be easily avoided by implementing
non-skelta mode in ra_serf (ra_http).
--
Ivan Zhakov
Re: [DISCUSS] delete ra_neon
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Justin Erenkrantz wrote on Sat, May 19, 2012 at 08:05:34 -0700:
> On Sat, May 19, 2012 at 7:55 AM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> > Well, we could have a single ra_dav library with a backend_supports_synchronity
> > bitfield.
> >
> > In theory, at least.
>
> The code would be so completely different there'd be very little code
> reuse - you're best just having it into two separately
> libraries...IMO. -- justin
Yes, I imagined that might be the case.
Re: [DISCUSS] delete ra_neon
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Sat, May 19, 2012 at 7:55 AM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> Well, we could have a single ra_dav library with a backend_supports_synchronity
> bitfield.
>
> In theory, at least.
The code would be so completely different there'd be very little code
reuse - you're best just having it into two separately
libraries...IMO. -- justin
Re: [DISCUSS] delete ra_neon
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Justin Erenkrantz wrote on Sat, May 19, 2012 at 07:39:59 -0700:
> On Sat, May 19, 2012 at 7:25 AM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> >> Ewww. You are evil. Go away, and come back when you are less evil.
> >>
> >> Just kidding. But seriously ... evil.
> >
> > So why won't Ivan's idea work?
> >
> > (I assume it won't work, but now that it's been aired it would be nice
> > to hear from an ra_serf person _why_ it won't work.)
>
> Short answer: neon is a synchronous library while serf is an
> asynchronous library. The internal paradigms for ra_serf and ra_neon
> are completely different due to that. I wouldn't even want to think
> about trying to shoehorn one on top of the other...it's just evil. =)
Well, we could have a single ra_dav library with a backend_supports_synchronity
bitfield.
In theory, at least.
> -- justin
Re: [DISCUSS] delete ra_neon
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Sat, May 19, 2012 at 7:25 AM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
>> Ewww. You are evil. Go away, and come back when you are less evil.
>>
>> Just kidding. But seriously ... evil.
>
> So why won't Ivan's idea work?
>
> (I assume it won't work, but now that it's been aired it would be nice
> to hear from an ra_serf person _why_ it won't work.)
Short answer: neon is a synchronous library while serf is an
asynchronous library. The internal paradigms for ra_serf and ra_neon
are completely different due to that. I wouldn't even want to think
about trying to shoehorn one on top of the other...it's just evil. =)
-- justin
Re: [DISCUSS] delete ra_neon
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
C. Michael Pilato wrote on Thu, May 17, 2012 at 16:34:09 -0400:
> On 05/17/2012 04:19 PM, Ivan Zhakov wrote:
> > On Wed, May 16, 2012 at 2:46 PM, Greg Stein <gs...@gmail.com> wrote:
> >> Short query: is this possible?
> >>
> >> Long query:
> >>
> >> Hyrum has been doing some heavy lifting on moving our code to Ev2 for
> >> commits. That work can be merged soonish, but a strong argument can be made
> >> for "only when all RA providers have an Ev2 commit editor" (because
> >> otherwise poor-perf shims are used). ra_local already has the needed editor,
> >> and I've been doing work on ra_serf to stabilize a base for writing the Ev2
> >> editor. Somebody is going to need to write an editor for ra_svn. My
> >> query/suggestion is to delete ra_neon rather than needing to write an editor
> >> for it.
> >>
> > [..]
> >
> > I believe that best solution would be to merge ra_neon and ra_serf in
> > one ra_http RA provider which can be configured to which http library
> > to use: neon or serf (or maybe WinHTTP in future).
>
> Ewww. You are evil. Go away, and come back when you are less evil.
>
> Just kidding. But seriously ... evil.
So why won't Ivan's idea work?
(I assume it won't work, but now that it's been aired it would be nice
to hear from an ra_serf person _why_ it won't work.)
Re: [DISCUSS] delete ra_neon
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 05/17/2012 04:19 PM, Ivan Zhakov wrote:
> On Wed, May 16, 2012 at 2:46 PM, Greg Stein <gs...@gmail.com> wrote:
>> Short query: is this possible?
>>
>> Long query:
>>
>> Hyrum has been doing some heavy lifting on moving our code to Ev2 for
>> commits. That work can be merged soonish, but a strong argument can be made
>> for "only when all RA providers have an Ev2 commit editor" (because
>> otherwise poor-perf shims are used). ra_local already has the needed editor,
>> and I've been doing work on ra_serf to stabilize a base for writing the Ev2
>> editor. Somebody is going to need to write an editor for ra_svn. My
>> query/suggestion is to delete ra_neon rather than needing to write an editor
>> for it.
>>
> [..]
>
> I believe that best solution would be to merge ra_neon and ra_serf in
> one ra_http RA provider which can be configured to which http library
> to use: neon or serf (or maybe WinHTTP in future).
Ewww. You are evil. Go away, and come back when you are less evil.
Just kidding. But seriously ... evil.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: [DISCUSS] delete ra_neon
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Wed, May 16, 2012 at 2:46 PM, Greg Stein <gs...@gmail.com> wrote:
> Short query: is this possible?
>
> Long query:
>
> Hyrum has been doing some heavy lifting on moving our code to Ev2 for
> commits. That work can be merged soonish, but a strong argument can be made
> for "only when all RA providers have an Ev2 commit editor" (because
> otherwise poor-perf shims are used). ra_local already has the needed editor,
> and I've been doing work on ra_serf to stabilize a base for writing the Ev2
> editor. Somebody is going to need to write an editor for ra_svn. My
> query/suggestion is to delete ra_neon rather than needing to write an editor
> for it.
>
[..]
I believe that best solution would be to merge ra_neon and ra_serf in
one ra_http RA provider which can be configured to which http library
to use: neon or serf (or maybe WinHTTP in future).
--
Ivan Zhakov
Re: [DISCUSS] delete ra_neon
Posted by Peter Samuelson <pe...@p12n.org>.
[Greg Stein]
> A following question asks about API compatibility. I believe we can
> simply sub ra_serf into any loader/query for ra_neon. Hard linkage to
> the ra_neon library may also need a solution (it becomes a shim?).
I was concerned with this exact issue when libsvn_ra_dav was renamed to
libsvn_ra_neon a few releases ago. We did it anyway, I added a
libsvn_ra_dav symlink to libsvn_ra_neon in the Debian package so
existing binaries linked (unnecessarily) to ra_dav would not break, and
apparently the world didn't end.
So I figure the world won't end if we do the same thing again.
(Going forward, it's already impossible to _directly_ link to an ra,
fs, or auth backend using the Debian libsvn packaging. Well, not
impossible, I can't seem to make it impossible while still supporting
static linking ... but it's not supported, and if you do it and try to
create a Debian binary package of your work, the package will be very
visibly broken.)
Re: [DISCUSS] delete ra_neon
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Wed, May 16, 2012 at 3:46 AM, Greg Stein <gs...@gmail.com> wrote:
> Short query: is this possible?
Given that the last major blocker/complaint that I was aware of had to
do with the C-L/chunking on the request side and that's been resolved
now in trunk (serf + svn), I think it's possible without any
user-visible regressions for 1.8.
I still track the mailing lists on a regular basis, but I'm tending to
let others run with the serf code. However, if some tricky bit comes
up, I'm happy to dive in as needed to help out.
Re: smart cards - serf and neon both use OpenSSL - so it shouldn't be
terribly hard (ha ha ha) to teach serf if it's missing something
around authentication that neon can do right now. -- justin