You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Hyrum K Wright <hy...@wandisco.com> on 2012/02/08 18:58:31 UTC
Status of 1.7.3
[ starting a new thread, as the previous has been seriously hijacked ]
Is there any sense of closure on the serf+windows test failure on the
1.7.x branch? My sense is that the failure does *not* expose a new
bug on the branch, but rather smokes out an existing one. If that is
the case, I am not opposed with moving forward on a 1.7.3 release on
the branch as-is, though we'd need to make some kind of note about the
test failure (or quash it for the time being).
Given the above, I'm inclined to move forward and roll tarballs on
Friday, if nobody objects too loudly.
-Hyrum
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/
Re: Status of 1.7.3
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 02/09/2012 12:35 PM, Greg Stein wrote:
> If it is determined that ra_serf's parallelism is at fault here, then we can
> force it to use a single connection for svnrdump. That should make it follow
> the right order.
>
> (and yeah, it sucks when you try to advance the capabilities and get yanked
> backwards into old-school mediocrity)
Indeed. And I didn't want to come across as banging my old "serf dun it
rong" drum -- I'm past that. I merely want to inform so that a solution can
hopefully be found more quickly.
Let's get Ev2 in place ASAP, m'kay? :-)
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: Status of 1.7.3
Posted by Greg Stein <gs...@gmail.com>.
On Feb 9, 2012 12:26 PM, "C. Michael Pilato" <cm...@collab.net> wrote:
>
> On 02/09/2012 11:41 AM, Philip Martin wrote:
> > "C. Michael Pilato" <cm...@collab.net> writes:
> >
> >> On 02/09/2012 05:22 AM, Philip Martin wrote:
> >>> Hyrum K Wright <hy...@wandisco.com> writes:
> >>>
> >>>> Is there any sense of closure on the serf+windows test failure on the
> >>>> 1.7.x branch? My sense is that the failure does *not* expose a new
> >>>> bug on the branch, but rather smokes out an existing one.
> >>>
> >>> That's my view as well. svnrdump has a bug that causes it to rely on
> >>> the server responding to serf's series HTTP requests in a particular
> >>> order. It's not a new bug.
> >>
> >> Has that actually been established? I mean, if all svnrdump is doing
is
> >> expecting ra_serf to honor the well-established, well-documented, but
> >> ra_serf-flaunting Ev1 editor drive ordering rules ... is that really
> >> svnrdump's bug?
> >
> > svnrdump doesn't handle postfix text deltas. It doesn't handle the
> > sequence open_file, open_file, ... close_file, close_file. I don't
> > know whether it's the cause of this particular problem but the fact that
> > we are seeing an ordering bug is not surprising.
>
> Bah. I'd be surprised if most of our editors supported postfix text
deltas.
> To my knowledge, there is exactly one driver which uses that approach in
> our own codebase (the commit driver). So, I can easily forgive svnrdump's
> dump editor (and any other non-commit editor) for not supporting postfix
> text deltas. That's one of two valid "modes" of an editor drive -- one
> rarely used in practice.
>
> ra_serf has a well-established history (and intentional design) which
> violates a far more fundamental editor restriction which applies to both
> "modes" -- depth-first-ness. (See
> http://subversion.tigris.org/issues/show_bug.cgi?id=2932 for details.)
This
> is why I'm more apt to think that the problem here is with ra_serf than
with
> svnrdump.
If it is determined that ra_serf's parallelism is at fault here, then we
can force it to use a single connection for svnrdump. That should make it
follow the right order.
(and yeah, it sucks when you try to advance the capabilities and get yanked
backwards into old-school mediocrity)
Cheers,
-g
Re: Status of 1.7.3
Posted by Hyrum K Wright <hy...@wandisco.com>.
On Thu, Feb 9, 2012 at 11:32 AM, Mark Phippard <ma...@gmail.com> wrote:
> On Thu, Feb 9, 2012 at 12:26 PM, C. Michael Pilato <cm...@collab.net> wrote:
>
>> Bah. I'd be surprised if most of our editors supported postfix text deltas.
>> To my knowledge, there is exactly one driver which uses that approach in
>> our own codebase (the commit driver). So, I can easily forgive svnrdump's
>> dump editor (and any other non-commit editor) for not supporting postfix
>> text deltas. That's one of two valid "modes" of an editor drive -- one
>> rarely used in practice.
>>
>> ra_serf has a well-established history (and intentional design) which
>> violates a far more fundamental editor restriction which applies to both
>> "modes" -- depth-first-ness. (See
>> http://subversion.tigris.org/issues/show_bug.cgi?id=2932 for details.) This
>> is why I'm more apt to think that the problem here is with ra_serf than with
>> svnrdump.
>
> In terms of 1.7.3, which is the scope of this discussion, the only
> thing that should be relevant is whether this is a significant
> regression from previous 1.7 releases. If we tolerated the bug for
> 1.7.0 there is no point in holding up 1.7.3 while we discuss it.
I think the concern is that 1.7.3 servers may cause more problems than
1.7.0-1.7.2 servers. I also get the feeling that it's all academic at
this point, though.
That being said, I'll wait for the nightly merges to go through
tonight, and roll the 1.7.3 tarball tomorrow as promised.
-Hyrum
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/
Re: Status of 1.7.3
Posted by Philip Martin <ph...@wandisco.com>.
Greg Stein <gs...@gmail.com> writes:
> What happens when you set MAX_NR_CONNS (that's not the right symbol, but
> close) in linsvn_ra_serf/update.c to 2 ? That should eliminate the
> parallelism. If that succeeds, then maybe we can patch 1.7.x in some way to
> allow svnrdump to set that max, yet keep multiple for regular
> checkout/update/sync.
Still fails with:
-#define MAX_NR_OF_CONNS 4
+#define MAX_NR_OF_CONNS 2
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Re: Status of 1.7.3
Posted by Greg Stein <gs...@gmail.com>.
On Feb 9, 2012 1:29 PM, "Philip Martin" <ph...@wandisco.com> wrote:
>
> Johan Corveleyn <jc...@gmail.com> writes:
>
> > The "dangerous behavior" may have been present in 1.7.0 already, but
> > 1.7.3 will be the first release where this appeared to cause a real
> > issue. I mean, for some reason the bug (or manifestation of the
> > "lingering bug") first appeared on 1.7.x after r1239697 (backport of
> > r1237720 and r1239596 (stuff in mod_dav_svn/liveprops.c) from trunk).
>
> I've managed to reproduce the problem on my Linux box using a different
> dataset. Using r1239696, i.e. before the revision that triggers the
> regression test FAIL on your machine, I see:
>
> $ rm -rf repo && svnadmin create repo && \
> svnrdump dump -r840000:head http://localhost:8080/repos/asf \
> --config-option servers:global:http-library=serf | svnadmin load repo
> * adding path : subversion/trunk/ac-helpers/gnu-diff.sh ... done.
> svnadmin: E140001: Unrecognized record type in stream
> svnrdump: E135007: Write error in pipe
>
> A second run gives:
>
> svnadmin: E185003: Delta does not fill the target window
> * adding path : subversion/trunk/www/project_nav.html ...svnrdump:
E135007: Write error in pipe
>
> I get similar errors when I switch my server to mod_dav_svn 1.6.
>
> So the problem is simply that svnrdump doesn't work with serf.
Yeah... after our conversation on IRC, that seemed almost inevitable.
What happens when you set MAX_NR_CONNS (that's not the right symbol, but
close) in linsvn_ra_serf/update.c to 2 ? That should eliminate the
parallelism. If that succeeds, then maybe we can patch 1.7.x in some way to
allow svnrdump to set that max, yet keep multiple for regular
checkout/update/sync.
Cheers,
-g
Re: Status of 1.7.3
Posted by Philip Martin <ph...@wandisco.com>.
Johan Corveleyn <jc...@gmail.com> writes:
> The "dangerous behavior" may have been present in 1.7.0 already, but
> 1.7.3 will be the first release where this appeared to cause a real
> issue. I mean, for some reason the bug (or manifestation of the
> "lingering bug") first appeared on 1.7.x after r1239697 (backport of
> r1237720 and r1239596 (stuff in mod_dav_svn/liveprops.c) from trunk).
I've managed to reproduce the problem on my Linux box using a different
dataset. Using r1239696, i.e. before the revision that triggers the
regression test FAIL on your machine, I see:
$ rm -rf repo && svnadmin create repo && \
svnrdump dump -r840000:head http://localhost:8080/repos/asf \
--config-option servers:global:http-library=serf | svnadmin load repo
* adding path : subversion/trunk/ac-helpers/gnu-diff.sh ... done.
svnadmin: E140001: Unrecognized record type in stream
svnrdump: E135007: Write error in pipe
A second run gives:
svnadmin: E185003: Delta does not fill the target window
* adding path : subversion/trunk/www/project_nav.html ...svnrdump: E135007: Write error in pipe
I get similar errors when I switch my server to mod_dav_svn 1.6.
So the problem is simply that svnrdump doesn't work with serf.
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Re: Status of 1.7.3
Posted by Johan Corveleyn <jc...@gmail.com>.
On Thu, Feb 9, 2012 at 6:32 PM, Mark Phippard <ma...@gmail.com> wrote:
> On Thu, Feb 9, 2012 at 12:26 PM, C. Michael Pilato <cm...@collab.net> wrote:
>
>> Bah. I'd be surprised if most of our editors supported postfix text deltas.
>> To my knowledge, there is exactly one driver which uses that approach in
>> our own codebase (the commit driver). So, I can easily forgive svnrdump's
>> dump editor (and any other non-commit editor) for not supporting postfix
>> text deltas. That's one of two valid "modes" of an editor drive -- one
>> rarely used in practice.
>>
>> ra_serf has a well-established history (and intentional design) which
>> violates a far more fundamental editor restriction which applies to both
>> "modes" -- depth-first-ness. (See
>> http://subversion.tigris.org/issues/show_bug.cgi?id=2932 for details.) This
>> is why I'm more apt to think that the problem here is with ra_serf than with
>> svnrdump.
>
> In terms of 1.7.3, which is the scope of this discussion, the only
> thing that should be relevant is whether this is a significant
> regression from previous 1.7 releases. If we tolerated the bug for
> 1.7.0 there is no point in holding up 1.7.3 while we discuss it.
The "dangerous behavior" may have been present in 1.7.0 already, but
1.7.3 will be the first release where this appeared to cause a real
issue. I mean, for some reason the bug (or manifestation of the
"lingering bug") first appeared on 1.7.x after r1239697 (backport of
r1237720 and r1239596 (stuff in mod_dav_svn/liveprops.c) from trunk).
Also: the problem is related to the server-side (mod_dav_svn), which
means svnrdump over serf will be problematic with 1.7.3 servers.
Disclaimer: I don't feel strongly at all about this problem, either
way. In fact, I don't really care. I just happen to be one of the few
people who can reproduce the problem, so I'm trying my best to
investigate it in any way I can and present the information.
--
Johan
Re: Status of 1.7.3
Posted by Mark Phippard <ma...@gmail.com>.
On Thu, Feb 9, 2012 at 12:26 PM, C. Michael Pilato <cm...@collab.net> wrote:
> Bah. I'd be surprised if most of our editors supported postfix text deltas.
> To my knowledge, there is exactly one driver which uses that approach in
> our own codebase (the commit driver). So, I can easily forgive svnrdump's
> dump editor (and any other non-commit editor) for not supporting postfix
> text deltas. That's one of two valid "modes" of an editor drive -- one
> rarely used in practice.
>
> ra_serf has a well-established history (and intentional design) which
> violates a far more fundamental editor restriction which applies to both
> "modes" -- depth-first-ness. (See
> http://subversion.tigris.org/issues/show_bug.cgi?id=2932 for details.) This
> is why I'm more apt to think that the problem here is with ra_serf than with
> svnrdump.
In terms of 1.7.3, which is the scope of this discussion, the only
thing that should be relevant is whether this is a significant
regression from previous 1.7 releases. If we tolerated the bug for
1.7.0 there is no point in holding up 1.7.3 while we discuss it.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: Status of 1.7.3
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 02/09/2012 11:41 AM, Philip Martin wrote:
> "C. Michael Pilato" <cm...@collab.net> writes:
>
>> On 02/09/2012 05:22 AM, Philip Martin wrote:
>>> Hyrum K Wright <hy...@wandisco.com> writes:
>>>
>>>> Is there any sense of closure on the serf+windows test failure on the
>>>> 1.7.x branch? My sense is that the failure does *not* expose a new
>>>> bug on the branch, but rather smokes out an existing one.
>>>
>>> That's my view as well. svnrdump has a bug that causes it to rely on
>>> the server responding to serf's series HTTP requests in a particular
>>> order. It's not a new bug.
>>
>> Has that actually been established? I mean, if all svnrdump is doing is
>> expecting ra_serf to honor the well-established, well-documented, but
>> ra_serf-flaunting Ev1 editor drive ordering rules ... is that really
>> svnrdump's bug?
>
> svnrdump doesn't handle postfix text deltas. It doesn't handle the
> sequence open_file, open_file, ... close_file, close_file. I don't
> know whether it's the cause of this particular problem but the fact that
> we are seeing an ordering bug is not surprising.
Bah. I'd be surprised if most of our editors supported postfix text deltas.
To my knowledge, there is exactly one driver which uses that approach in
our own codebase (the commit driver). So, I can easily forgive svnrdump's
dump editor (and any other non-commit editor) for not supporting postfix
text deltas. That's one of two valid "modes" of an editor drive -- one
rarely used in practice.
ra_serf has a well-established history (and intentional design) which
violates a far more fundamental editor restriction which applies to both
"modes" -- depth-first-ness. (See
http://subversion.tigris.org/issues/show_bug.cgi?id=2932 for details.) This
is why I'm more apt to think that the problem here is with ra_serf than with
svnrdump.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: Status of 1.7.3
Posted by Philip Martin <ph...@wandisco.com>.
"C. Michael Pilato" <cm...@collab.net> writes:
> On 02/09/2012 05:22 AM, Philip Martin wrote:
>> Hyrum K Wright <hy...@wandisco.com> writes:
>>
>>> Is there any sense of closure on the serf+windows test failure on the
>>> 1.7.x branch? My sense is that the failure does *not* expose a new
>>> bug on the branch, but rather smokes out an existing one.
>>
>> That's my view as well. svnrdump has a bug that causes it to rely on
>> the server responding to serf's series HTTP requests in a particular
>> order. It's not a new bug.
>
> Has that actually been established? I mean, if all svnrdump is doing is
> expecting ra_serf to honor the well-established, well-documented, but
> ra_serf-flaunting Ev1 editor drive ordering rules ... is that really
> svnrdump's bug?
svnrdump doesn't handle postfix text deltas. It doesn't handle the
sequence open_file, open_file, ... close_file, close_file. I don't
know whether it's the cause of this particular problem but the fact that
we are seeing an ordering bug is not surprising. I don't think we have
made any explicit changes to server ordering so the bug is probably not
new in 1.7.3 or even 1.7. It's just the usual things that affect server
ordering, the exact build, setup, machine load, etc.
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Re: Status of 1.7.3
Posted by Hyrum K Wright <hy...@wandisco.com>.
On Thu, Feb 9, 2012 at 10:13 AM, C. Michael Pilato <cm...@collab.net> wrote:
> On 02/09/2012 05:22 AM, Philip Martin wrote:
>> Hyrum K Wright <hy...@wandisco.com> writes:
>>
>>> Is there any sense of closure on the serf+windows test failure on the
>>> 1.7.x branch? My sense is that the failure does *not* expose a new
>>> bug on the branch, but rather smokes out an existing one.
>>
>> That's my view as well. svnrdump has a bug that causes it to rely on
>> the server responding to serf's series HTTP requests in a particular
>> order. It's not a new bug.
>
> Has that actually been established? I mean, if all svnrdump is doing is
> expecting ra_serf to honor the well-established, well-documented, but
> ra_serf-flaunting Ev1 editor drive ordering rules ... is that really
> svnrdump's bug?
svnrdump may do many things, but honoring the Ev1 ordering rules is
not one of them. The violations are legion, but suffice it to say
that if the svnrdump editor is not driven in a precise manner--which
is some subset of that allowed by Ev1--all bets are off.
Some of these problems, such as re-using an editor for multiple
revisions, have been fixed on trunk, and may be candidates for
backport. Others, such as the reliance on the editor baton to store
everything, are a bit more fundamental, and may require more work to
get right.
-Hyrum
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/
Re: Status of 1.7.3
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 02/09/2012 05:22 AM, Philip Martin wrote:
> Hyrum K Wright <hy...@wandisco.com> writes:
>
>> Is there any sense of closure on the serf+windows test failure on the
>> 1.7.x branch? My sense is that the failure does *not* expose a new
>> bug on the branch, but rather smokes out an existing one.
>
> That's my view as well. svnrdump has a bug that causes it to rely on
> the server responding to serf's series HTTP requests in a particular
> order. It's not a new bug.
Has that actually been established? I mean, if all svnrdump is doing is
expecting ra_serf to honor the well-established, well-documented, but
ra_serf-flaunting Ev1 editor drive ordering rules ... is that really
svnrdump's bug?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: Status of 1.7.3
Posted by Philip Martin <ph...@wandisco.com>.
Hyrum K Wright <hy...@wandisco.com> writes:
> Is there any sense of closure on the serf+windows test failure on the
> 1.7.x branch? My sense is that the failure does *not* expose a new
> bug on the branch, but rather smokes out an existing one.
That's my view as well. svnrdump has a bug that causes it to rely on
the server responding to serf's series HTTP requests in a particular
order. It's not a new bug.
> If that is
> the case, I am not opposed with moving forward on a 1.7.3 release on
> the branch as-is, though we'd need to make some kind of note about the
> test failure (or quash it for the time being).
>
> Given the above, I'm inclined to move forward and roll tarballs on
> Friday, if nobody objects too loudly.
+1
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com