You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "C. Michael Pilato" <cm...@collab.net> on 2009/05/29 13:27:51 UTC
Issue #3242 is heating up.
Folks, we made a pretty serious regression in Subversion 1.5 regarding the
way our copy and move operations drive the commit editor. The result is
that operations previously permitted via a particular authz policy no longer
do. As a result, there are people still running 1.4 simply because they
can't use 1.5. That's really bad, because 1.4 has dropped from official
non-emergency support already. This is tracked in issue #3242 [1], and that
issue is started to heat up.
I see three courses of action:
* Continue to ignore the issue. This comes with all the disadvantages
you might assume from such a user-unfriendly approach.
* Rewrite Subversion's authz framework, perhaps in the way I've
recommended in issue #3380 [2]. This is very non-trivial amount
of work though, and not backportable to existing release streams.
* Fix the copy and move code to be more conservative in their
approach. I took a stab at this a while ago, and got completely
lost in the logic. My guess is that Hyrum is the only person who
really has the big picture in his head there. But as a stopgap,
I wonder if we couldn't just resurrect the 1.4 logic (as a sibling
to the 1.5 stuff), and make the APIs fork immediately: doing a
single-source copy or move? Use the old logic (plus some merge
tracking handling). Else, use the new, multi-source-supporting
stuff.
I'm a bit concerned about our relationship with our user base, and not just
saying that to be dramatic. What do you guys think? Is there anyone with
the time and a degree of focus who can get us out of this?
[1] http://subversion.tigris.org/issues/show_bug.cgi?id=3242
[2] http://subversion.tigris.org/issues/show_bug.cgi?id=3380
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2356771
Re: Issue #3242 is heating up.
Posted by Jeremy Whitlock <jc...@gmail.com>.
> This bug *is* on my list of stuff to get to eventually, but I'd be
> thrilled if somebody else wanted to take a look at it. I think the
> first step to fix the bug is to create a test for it, so the bug is
> well-scoped. Creating a test may also help in understanding potential
> solutions. Any volunteers?
I've created a test for this and committed it. (It's being XFailed
right now.) I also updated the issue with my findings while writing
the test.
--
Take care,
Jeremy Whitlock
http://www.thoughtspark.org
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2358604
RE: Issue #3242 is heating up.
Posted by Bert Huijben <rh...@sharpsvn.net>.
> -----Original Message-----
> From: C. Michael Pilato [mailto:cmpilato@collab.net]
> Sent: vrijdag 29 mei 2009 16:13
> To: Hyrum K. Wright
> Cc: dev@subversion.tigris.org
> Subject: Re: Issue #3242 is heating up.
>
> Hyrum K. Wright wrote:
> > In order to minimize ra accesses, the code looks for the common parent
> > of the source and the destination, reparents the ra_session to that
> > location, and then proceeds to act using at that the operational root.
> > The problem arises when that common root isn't accessible (due to authz
> > restrictions), so the copy or move errors out. I *think* the solution
> > is something along the lines of "be more intelligent about reparenting
> > the ra_session" up to and including "don't reparent". That's a
> > first-order approximation of what the solution could be.
>
> Your analysis is correct. The solution involves giving precedence to
> correctness (don't access paths you don't absolutely need to access) over
> performance.
>
> "svn move" is hampered at the moment by the design-level requirement of
> anchoring the operation at the deepest common parent of the source(s) and
> destination(s), and possibly one level higher if one of the sources of the
> move *is* that deepest common parent. But that's always been true of "svn
> move".
>
> "svn copy" request only access enough to check the existing of the copy
> source(s), and access to the lowest common parent of the copy
destination(s)
> (though... I seem to recall that our API pretty much demands that in a
> multi-source copy, all the destinations are siblings of the same single
> directory).
>
> Of course, --parents throws a wrench into this, as we now might have to
move
> our anchor point (in both the copy and move cases) up to deepest
> already-existing parent of the anchor point we would otherwise have used
if
> --parents wasn't specified.
I did an attempt on fixing this a few months ago. (I should have a dirty
patch somewhere).
Most operations are rooted on the common parent, but this was the same
before 1.5. But in 1.5 opening a RA session over http starts with an OPTION
request to the root where the ra session is rooted on. (I found a way to
work around this by retrying on a lower level and then reparenting).
But looking further I found a mergeinfo lookup issue. This merge info
request (after most other work is complete) explicitly roots the ra session
at the repository root. It could be that just resolving this specific
session reparenting resolves a big part of this issue.
> So... yeah. Nasty stuff.
Bert
>
> --
> C. Michael Pilato <cm...@collab.net>
> CollabNet <> www.collab.net <> Distributed Development On Demand
>
> ------------------------------------------------------
>
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=235
67
> 88
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2356791
Re: Issue #3242 is heating up.
Posted by "C. Michael Pilato" <cm...@collab.net>.
Hyrum K. Wright wrote:
> In order to minimize ra accesses, the code looks for the common parent
> of the source and the destination, reparents the ra_session to that
> location, and then proceeds to act using at that the operational root.
> The problem arises when that common root isn't accessible (due to authz
> restrictions), so the copy or move errors out. I *think* the solution
> is something along the lines of "be more intelligent about reparenting
> the ra_session" up to and including "don't reparent". That's a
> first-order approximation of what the solution could be.
Your analysis is correct. The solution involves giving precedence to
correctness (don't access paths you don't absolutely need to access) over
performance.
"svn move" is hampered at the moment by the design-level requirement of
anchoring the operation at the deepest common parent of the source(s) and
destination(s), and possibly one level higher if one of the sources of the
move *is* that deepest common parent. But that's always been true of "svn
move".
"svn copy" request only access enough to check the existing of the copy
source(s), and access to the lowest common parent of the copy destination(s)
(though... I seem to recall that our API pretty much demands that in a
multi-source copy, all the destinations are siblings of the same single
directory).
Of course, --parents throws a wrench into this, as we now might have to move
our anchor point (in both the copy and move cases) up to deepest
already-existing parent of the anchor point we would otherwise have used if
--parents wasn't specified.
So... yeah. Nasty stuff.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2356788
Re: Issue #3242 is heating up.
Posted by Jeremy Whitlock <jc...@gmail.com>.
> I haven't reviewed the patch, but I think the idea of having looser
> restrictions on when we return OPTIONS is valid. IIRC, the response
> to OPTIONS is per-repository, meaning that it won't change depending
> on which path the user is accessing. It sounds like the cause of
> #3242 is an unfortunate interaction between common parent reparenting,
> and the OPTIONS stuff introduced in 1.5. Perhaps this is a creative
> way to solve it?
Well, PROPFIND is also impacted by this. If you solve the OPTIONS
thing, you end up getting caught up at PROPFIND. As for being
creative, I think Mike's suggestion to improve our authz framework
would be that creative way to solve. We can accomplish two things at
once and still not change the performance improvements we get by the
new reparenting approach.
--
Take care,
Jeremy Whitlock
http://www.thoughtspark.org
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2358845
Re: Issue #3242 is heating up.
Posted by "Hyrum K. Wright" <hy...@hyrumwright.org>.
On May 30, 2009, at 4:46 PM, Rupert Wood wrote:
> Hyrum K. Wright wrote:
>
>> which results in
>> svn: Server sent unexpected return value (403 Forbidden) in
>> response to PROPFIND request for '/svn'
>
> Here's the hack we use locally to mitigate this: allow OPTIONS and
> PROPFIND at the repository root for all authenticated users. It
> broadly seems to do the trick and for most operations just OPTIONS
> will do.
I haven't reviewed the patch, but I think the idea of having looser
restrictions on when we return OPTIONS is valid. IIRC, the response
to OPTIONS is per-repository, meaning that it won't change depending
on which path the user is accessing. It sounds like the cause of
#3242 is an unfortunate interaction between common parent reparenting,
and the OPTIONS stuff introduced in 1.5. Perhaps this is a creative
way to solve it?
(The only information I could see leaking is the fact that the path
exists. But the path is a parent of something the user already has
access to, so the user should already know the path exists, so it's
not really leakage at all.)
In other news, while looking through the lock code earlier today, I
noticed we open an ra session to the common parent of all the lock
targets, so it would probably have the same problem as #3242. We
probably haven't heard much about it because people probably don't try
to lock paths in different parts of a repository using the same
command very often.
-Hyrum
> I meant to work out if this was sufficient and not too dangerous
> sometime before passing the patch on - I haven't done this, and
> sounds like it this isn't even remotely correct, so I'm not really
> proposing this as a patch. But here goes anyway in case it's useful.
>
> Rup.
>
> [[[
> Hack work-around for issue 3242.
>
> * subversion/mod_authz_svn/mod_authz_svn.c:
> Permit OPTIONS and PROPFIND at repository root for all authenticated
> users.
> ]]]
>
> <subversion-1.6.0-optionsroot.patch>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2358835
RE: Issue #3242 is heating up.
Posted by Rupert Wood <me...@rupey.net>.
Hyrum K. Wright wrote:
> which results in
> svn: Server sent unexpected return value (403 Forbidden) in
> response to PROPFIND request for '/svn'
Here's the hack we use locally to mitigate this: allow OPTIONS and PROPFIND at the repository root for all authenticated users. It broadly seems to do the trick and for most operations just OPTIONS will do.
I meant to work out if this was sufficient and not too dangerous sometime before passing the patch on - I haven't done this, and sounds like it this isn't even remotely correct, so I'm not really proposing this as a patch. But here goes anyway in case it's useful.
Rup.
[[[
Hack work-around for issue 3242.
* subversion/mod_authz_svn/mod_authz_svn.c:
Permit OPTIONS and PROPFIND at repository root for all authenticated
users.
]]]
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2357123
Re: Issue #3242 is heating up.
Posted by "Hyrum K. Wright" <hy...@hyrumwright.org>.
On May 29, 2009, at 8:27 AM, C. Michael Pilato wrote:
> Folks, we made a pretty serious regression in Subversion 1.5
> regarding the
> way our copy and move operations drive the commit editor. The
> result is
> that operations previously permitted via a particular authz policy
> no longer
> do. As a result, there are people still running 1.4 simply because
> they
> can't use 1.5. That's really bad, because 1.4 has dropped from
> official
> non-emergency support already. This is tracked in issue #3242 [1],
> and that
> issue is started to heat up.
>
> I see three courses of action:
>
> * Continue to ignore the issue. This comes with all the
> disadvantages
> you might assume from such a user-unfriendly approach.
>
> * Rewrite Subversion's authz framework, perhaps in the way I've
> recommended in issue #3380 [2]. This is very non-trivial amount
> of work though, and not backportable to existing release streams.
>
> * Fix the copy and move code to be more conservative in their
> approach. I took a stab at this a while ago, and got completely
> lost in the logic. My guess is that Hyrum is the only person who
> really has the big picture in his head there. But as a stopgap,
> I wonder if we couldn't just resurrect the 1.4 logic (as a sibling
> to the 1.5 stuff), and make the APIs fork immediately: doing a
> single-source copy or move? Use the old logic (plus some merge
> tracking handling). Else, use the new, multi-source-supporting
> stuff.
>
> I'm a bit concerned about our relationship with our user base, and
> not just
> saying that to be dramatic. What do you guys think? Is there
> anyone with
> the time and a degree of focus who can get us out of this?
>
> [1] http://subversion.tigris.org/issues/show_bug.cgi?id=3242
> [2] http://subversion.tigris.org/issues/show_bug.cgi?id=3380
I've said it before, but #3242 is mea maxima culpa from my early days
of Subversion development. It does seem to be heating up, and I think
that we should fix the bug, and then cut a (final?) 1.5.x release with
that fix.
It's been a while since I've looked at the code, but this is my
(possibly outdated) analysis of #3242:
Users are encountering the bug when trying to do something like
$ svn -m version cp https://example.com/svn/ProjectName/2008/trunk \
-r56318 https://example.com/svn/ProjectName/2008/tags/8.11.07
which results in
svn: Server sent unexpected return value (403 Forbidden) in
response to PROPFIND
request for '/svn'
In order to minimize ra accesses, the code looks for the common parent
of the source and the destination, reparents the ra_session to that
location, and then proceeds to act using at that the operational
root. The problem arises when that common root isn't accessible (due
to authz restrictions), so the copy or move errors out. I *think* the
solution is something along the lines of "be more intelligent about
reparenting the ra_session" up to and including "don't reparent".
That's a first-order approximation of what the solution could be.
This bug *is* on my list of stuff to get to eventually, but I'd be
thrilled if somebody else wanted to take a look at it. I think the
first step to fix the bug is to create a test for it, so the bug is
well-scoped. Creating a test may also help in understanding potential
solutions. Any volunteers?
-Hyrum
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2356783
Re: Issue #3242 is heating up.
Posted by Edmund Wong <ed...@kdtc.net>.
Stefan Sperling wrote:
>
> I think the problem is bigger than "just" the relationship to our
> user base. Even though that problem alone is a huge one.
> And I'm not just saying that to be dramatic ;)
Especially when most of the questions users place on IRC are
left unanswered. As much as I would want to help them, I
am still new 'round these parts and really don't know how
to begin.
> My impression is that last year, we had more active people working
> on Subversion than we do have now. Though I haven't really counted the
> number of commits per contributor or some such figure. It's just my
> personal impression based on who I get to communicate with on the
> dev list and on IRC. Some people are simply less active, and the most
> likely reason is that they stopped having fun.
>
> Work lies around because people do not have fun tackling really
> difficult problems under pressure from users demanding a fix under
> a right-now-or-else mantra, like $most_verbose_user_posting_to_#3242.
Heh. I read that post. I'd say I'm having fun reading the code,
but the frustrating part is there is so much to read and being
'slow' really doesn't help me at all. I did manage to grab a
hold of a C book (aside for K&R, I got the "C in a nutshell" one)
and while I'm not say I'm fluent, I do understand for the
most part in C. Now reading the headers isn't as hard. What
is now being the hurdle is how everything fits together. This
brings me to the topic of understanding how Subversion works.
To my dismay, I think I've been spoilt with TortoiseSVN. Sure,
I can checkout and update using the command line. I realized
I still have lots to learn. Which is a good thing. Me digressing
isn't, though. :)
(Side note re: 3342, I realized I have yet to see the 'messages'
at the end of the sw/up/merge command. Mostly due to me using
TortoiseSVN. (talking to myself mode) I'll need to learn the command
line, especially creating repositories locally and doing what those
automatic tests do. (end of talking to myself mode).
>
>> What do you guys think? Is there anyone with
>> the time and a degree of focus who can get us out of this?
>
> If Hyrum can invest the time, then great, he should do it. But he's
> already busy with wc-ng, which fixes a whole lot of other serious
> problems (doh!). So our resources are scarce, we have too many problems
> to fix.
>
> Nevertheless, we should try to do something as soon as possible.
>
> I can also look into finding time for this in June, though I was
> thinking about trying to tackle #1964, actually.
> I don't think I can do both. I have no prior experience with much
> of anything related to #1964 nor #3242, so either of these would
> already mean a huge investment of time for me.
>
It's ironic that I do have the time (mostly during the night) to do
a lot of the issues; but the knowledge is lacking (one of my
major frustrations).
Anyway, just my $0.02 even though I haven't offered much. I'll
work harder.
Edmund
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2359030
Re: Issue #3242 is heating up.
Posted by Stefan Sperling <st...@elego.de>.
On Fri, May 29, 2009 at 09:27:51AM -0400, C. Michael Pilato wrote:
> Folks, we made a pretty serious regression in Subversion 1.5 regarding the
> way our copy and move operations drive the commit editor. The result is
> that operations previously permitted via a particular authz policy no longer
> do. As a result, there are people still running 1.4 simply because they
> can't use 1.5. That's really bad, because 1.4 has dropped from official
> non-emergency support already. This is tracked in issue #3242 [1], and that
> issue is started to heat up.
Yes. For many people using authz, things are really broken.
> I see three courses of action:
>
> * Continue to ignore the issue. This comes with all the disadvantages
> you might assume from such a user-unfriendly approach.
>
> * Rewrite Subversion's authz framework, perhaps in the way I've
> recommended in issue #3380 [2]. This is very non-trivial amount
> of work though, and not backportable to existing release streams.
>
> * Fix the copy and move code to be more conservative in their
> approach. I took a stab at this a while ago, and got completely
> lost in the logic. My guess is that Hyrum is the only person who
> really has the big picture in his head there. But as a stopgap,
> I wonder if we couldn't just resurrect the 1.4 logic (as a sibling
> to the 1.5 stuff), and make the APIs fork immediately: doing a
> single-source copy or move? Use the old logic (plus some merge
> tracking handling). Else, use the new, multi-source-supporting
> stuff.
>
> I'm a bit concerned about our relationship with our user base, and not just
> saying that to be dramatic.
I think the problem is bigger than "just" the relationship to our
user base. Even though that problem alone is a huge one.
And I'm not just saying that to be dramatic ;)
We have a few serious issues that have been lying around unfixed for
too long, and which we ended up releasing with. The effect of this is
both user _and_ developer frustration.
My impression is that last year, we had more active people working
on Subversion than we do have now. Though I haven't really counted the
number of commits per contributor or some such figure. It's just my
personal impression based on who I get to communicate with on the
dev list and on IRC. Some people are simply less active, and the most
likely reason is that they stopped having fun.
Work lies around because people do not have fun tackling really
difficult problems under pressure from users demanding a fix under
a right-now-or-else mantra, like $most_verbose_user_posting_to_#3242.
I think we should really reconsider the way we are doing testing for
releases to prevent such problems. We cannot afford to release with
regressions like this. The consequences are too dire.
We have to catch stuff like this before we release, not after.
The kinds of bugs that get fixed in point-releases should be small,
simple problems. Not design problems like #3242. A broken design
should not be released, it should not pass our testing.
But our current release and testing process cannot possibly capture
all potential problems, and it has little chance of catching the ones
which are really only occurring in huge setups like some of our advanced
users are running.
I mean the likes of users having good reasons for using authz, with
hundreds of developers in their teams relying on Subversion and doing
insanely huge merges every day.
I have some plans to try to fix this in the long term.
More on that at some other time, in a different thread, because
explaining this now would go too much off-topic for #3242.
For those who are interested to know more about this now, take an
hour to watch this talk: http://www.youtube.com/watch?v=i7pkyDUX5uM
You should pretty much be able to infer what my ideas are and
were I got them from :) And I hope we'll be able to bring the fun
back for people who have lost it.
> What do you guys think? Is there anyone with
> the time and a degree of focus who can get us out of this?
If Hyrum can invest the time, then great, he should do it. But he's
already busy with wc-ng, which fixes a whole lot of other serious
problems (doh!). So our resources are scarce, we have too many problems
to fix.
Nevertheless, we should try to do something as soon as possible.
I can also look into finding time for this in June, though I was
thinking about trying to tackle #1964, actually.
I don't think I can do both. I have no prior experience with much
of anything related to #1964 nor #3242, so either of these would
already mean a huge investment of time for me.
Stefan