headers
Karl Fogel | 1 Jun 2008 01:41
Favicon
Gravatar

Re: Possible data corruption issue?

[Search for "RC9" if you want to skip straight to the question of
whether RC9 is needed.  Material before that is discussion of the bug.]

Karl Fogel <kfogel <at> red-bean.com> writes:
> I've rethreaded this to get the words "data corruption" in the subject,
> as it seems to be that kind of bug.  As glasser pointed out in
>
>   http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=139319
>   Message-ID: <1ea387f60805282312v7f03d297w208cda44ba8ec438 <at> mail.gmail.com>
>
> in this thread, at the end of his repro recipe a file is left thinking
> it's at r1 but has the r2 working content *and* r2 text-base.  Even if
> later we reliably check base-checksum on commit (thus preventing bad
> file content from entering the repository), there may still be the
> possibility of property corruption being committed...  I haven't had
> time to analyze it deeply enough to know one way or the other (hey, it's
> 3am and I just saw this thread).
>
> If we can determine that there is no way for corruption to enter the
> repository with this bug, then I think it's okay to fix this in 1.5.1.
> Not ideal, but okay: a broken working copy is annoying, but it's not the
> same as bad data.  If there *is* a way, though, then we kind of have to
> fix this in 1.5.0.

Okay, I've had a chance to look into this more deeply.

Below is a reproduction script demonstrating that you can take an r2
working file, run 'svn up -r1 --depth=empty', and then commit a text
change to the file resulting in r3.  Your new commit will be against r2,
even though you thought you had r1 of the file.  Ick.
(Continue reading)

Permalink | Reply | 11 comments |
headers
Neels Janosch Hofmeyr | 1 Jun 2008 01:42
Picon

[PATCH] Adjust comment in tests/libsvn_subr/target-test.c

[[[
Add a very clear comment about what the file target-tests.c, which
may look like a C test, really is.

* subversion/tests/libsvn_subr/target-test.c: Add comment describing
  the function of this file

Patch by: Neels Janosch Hofmeyr <neels <at> elego.de>
]]]

--

-- 
Neels Hofmeyr -- elego Software Solutions GmbH
Gustav-Meyer-Allee 25 / Gebäude 12, 13355 Berlin, Germany
phone: +49 30 23458696  mobile: +49 177 2345869  fax: +49 30 23458695
http://www.elegosoft.com | Geschäftsführer: Olaf Wagner | Sitz: Berlin
Handelsreg: Amtsgericht Charlottenburg HRB 77719 | USt-IdNr: DE163214194
Attachment (target_test_c.patch): text/x-diff, 610 bytes
Permalink | Reply | 1 comment |
headers
Mark Phippard | 1 Jun 2008 02:30
Picon
Gravatar

Re: Possible data corruption issue?

On Sat, May 31, 2008 at 7:41 PM, Karl Fogel <kfogel <at> red-bean.com> wrote:

> There's no doubt this is a nasty bug, and obviously, the fix should be
> backported to 1.5.1 at least.  But does it justify an RC9?  I'm not so
> sure.  It's not exactly "corruption"... more like "misleading the user"
> or some lesser misdemeanor.

Thanks for the analysis and explanation.  I could probably be swayed
either way on this one, but based on what you discovered and posted
here I am inclined to agree with you that this might not be worth
doing a new RC for.  BTW, one thing I was not clear on in your
analysis, is whether the fix that glasser and cmpilato proposed for
backport has fixed the problem or not.  It kind of sounds like there
is more to do?

Anyway, clearly there is a bug and clearly it needs to be fixed.  I am
not sure if this qualifies are corruption or not.  If it does, it is
at least on the mild end of the spectrum.  I like to think of it in
other ways.  If this bug existed in our official releases how quickly
would we push out a new .x release.  I kind of do not see us rushing
one out the door over this.  I think we would certainly start alerting
people that we need to create a .x release but I think we would let it
evolve and come out somewhat normally.  The other criteria would be
whether we would backport this to several of our previous releases.
Again, I'd be surprised if we would backport this to 1.3, 1.2 etc.
based on what you have described above.

So based on that, I'd be inclined to say we could put this in a 1.5.1.

--

--
(Continue reading)

Permalink | Reply | 1 comment |
headers
Karl Fogel | 1 Jun 2008 02:33
Favicon
Gravatar

Re: 1.5.0-rc8 up for signing/testing

Karl Fogel <kfogel <at> red-bean.com> writes:
> "Hyrum K. Wright" <hyrum_wright <at> mail.utexas.edu> writes:
>> http://test.hyrumwright.org/svn/1.5.0-rc8/
>
> Summary:
>
>   All good (but see "Caveat" below).
>
> Caveat:
>
>   See http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=139336
>   (Message-ID: <874p8hleq0.fsf_-_ <at> red-bean.com>).  That thread discusses
>   a bug that may or may not be serious enough to affect this release.
>   (I still haven't set things up to test new client against old servers
>   and vice versa; that's clearly needed, as the above thread proves.)

Note that that thread continues...

   http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=139512
   From: Karl Fogel <kfogel <at> red-bean.com>
   To: dev <at> subversion.tigris.org
   Subject: Re: Possible data corruption issue?
   Date: Sat, 31 May 2008 19:41:17 -0400
   Message-ID: <87k5ha3uua.fsf <at> red-bean.com>

...and touches on whether or not an RC9 is needed.  The above message
expresses the opinion that RC9 may *not* be needed.  However, others may
follow up with different thoughts.  To avoid thread duplication, let's
keep the discussion over there; and that's what I'm trying to do by
pointing to it from this email :-).
(Continue reading)

Permalink | Reply | 2 comments |
headers
Karl Fogel | 1 Jun 2008 05:11
Favicon
Gravatar

Re: Possible data corruption issue?

"Mark Phippard" <markphip <at> gmail.com> writes:
> On Sat, May 31, 2008 at 7:41 PM, Karl Fogel <kfogel <at> red-bean.com> wrote:
>> There's no doubt this is a nasty bug, and obviously, the fix should be
>> backported to 1.5.1 at least.  But does it justify an RC9?  I'm not so
>> sure.  It's not exactly "corruption"... more like "misleading the user"
>> or some lesser misdemeanor.
>
> Thanks for the analysis and explanation.  I could probably be swayed
> either way on this one, but based on what you discovered and posted
> here I am inclined to agree with you that this might not be worth
> doing a new RC for.  BTW, one thing I was not clear on in your
> analysis, is whether the fix that glasser and cmpilato proposed for
> backport has fixed the problem or not.  It kind of sounds like there
> is more to do?

Sorry, I didn't explain that part very clearly.

The fix committed on trunk (r31516) solves the problem, in that it
leaves no known way for a user to get into this situation.  However,
r31516 may mask a latent problem that could bite us later: the 1.4.x
repository ought to reject the commit in the first place, given that the
client [should have] reported r1 yet the file had changed since r1.

The potential for latent problems is on both the client side and the
server side: during commit, the client may be reporting base revisions
in a wrong way, and/or the server may be interpreting them in a wrong
way.  None of that code was affected by r31516, so if those (potential)
bugs were there before, they're still there.

That's the question I'm going to look into tomorrow.
(Continue reading)

Permalink | Reply | 0 comments |
headers
Karl Fogel | 1 Jun 2008 05:43
Favicon
Gravatar

Re: [PATCH] ISSUE #3193 Fix peg revision parsing for CLI repository root relative urls.

Folker Schamel <schamel23 <at> spinor.com> writes:
> Karl Fogel wrote:
>> We've been returning constant strings for about eight years now, and
>> we've never heard a bug report about it.  Given that, I don't think we
>> should clutter up our API documentation by mentioning this concern.
>>
>> -Karl
>
> Wasn't meant offensive and I don't want to demand anything.
> I just noticed that aspect and wanted to mentioned it briefly
> in case you are not aware.
> Obviously you are aware of it and deliberately do it that way,
> so everything is fine.
> Sorry for the noise.

Huh -- oh, gosh, no need to apologize, Folker!  You weren't offensive at
all.  I just write replies very tersely because I write a lot of them;
please don't take the terseness as indicating anything else.  Your post
was fine.

   http://producingoss.com/en/communications.html#writing-tone
   http://producingoss.com/en/communications.html#rudeness

:-)

-Karl
Permalink | Reply | 0 comments |
headers
Karl Fogel | 1 Jun 2008 05:53
Favicon
Gravatar

Re: Jens, please try to get more peer review

"Hyrum K. Wright" <hyrum_wright <at> mail.utexas.edu> writes:
>> From: Daniel Shahaf
>>   Maybe "Approved by: obvious fix".
>
> That may confuse the contribulyzer.  I think just prefacing the log
> message with "Obvious fix", much the same way people sometimes do with
> work on branches, would be fine.  I really don't care how it's done,
> but being explicit about the obvious fix rule being invoked would be a
> good thing.

The Contribulyzer won't get confused -- it doesn't search for ".* by:",
but rather for certain specific words followed by " by:".  So we'll be
fine on that front either way.

I think just including the string "obvious fix" or "obvious fix rule"
somewhere in the log message will be enough to indicate when the rule is
being invoked.  We don't need a parseable convention for it.

While I know Stefan's original mail was well-meant, remember that part
of the point of the "obvious fix rule" is to remove barries to getting
obvious problems fixed.  Waiting for approval from someone else is a
barrier -- a small one, often a reasonable one, but a barrier
nonetheless.  So while it's always fine to seek review on a change, a
commit under the "obvious fix rule" shouldn't also need pre-approval.
Otherwise, why have an "obvious fix rule"?

If on rare occasions we get a failing test because the test output
wasn't updated, that's an annoyance, but IMHO it's an acceptable price
to pay for lots of obvious fixes going in with a minimum of overhead.
(Continue reading)

Permalink | Reply | 835 comments |
headers
Karl Fogel | 1 Jun 2008 05:54
Favicon
Gravatar

Re: FEATURE: Something like a 'svn rebuild'

"Sam Washburn" <swashburn <at> inovationeering.com> writes:
> It doesn't even need to schedule it for deletion, just mark it missing.  But
> it should not download a new copy.  The key is fixing the pristine copy and
> svn can then report the differences as it usually does
>
> I don't think it should be that difficult to implement.  Even
> the --metadata-only idea Alan had would be fine.  Which, I guess would be
> excluding a piece of code instead of writing something.
>
> Yes, it can happen in many other circumstances than just a fluke networking
> issue.  "Listman" mentioned broken tools.  That happens frequently for us
> too.
>
> And, imho, it seems a lot more professional for the project to say "run svn
> rebuild" rather than "first delete all your .svn files..."
>
> Well, thanks for considering it.  I really like svn.  I know others have bad
> things to say about cvs and svn due to frustrating experiences like mine,
> and maybe this would be a step closer to winning them over.

I think we're all in favor of a script to do this.  Adding a new
subcommand to svn itself, however, is a large user interface burden, so
we're usually pretty conservative about adding new subcommands.

-Karl
Permalink | Reply | 0 comments |
headers
Ivan Zhakov | 1 Jun 2008 08:16
Picon

Re: Possible memory leak in mod_dav_svn

On Thu, May 29, 2008 at 4:11 PM, Alexander Björnhagen
<Alexander.Bjornhagen <at> nasdaqomx.com> wrote:
> It seems that mod_dav_svn has issues handling large operations with
> authorization enabled.
>
> Our setup is a RHEL5 server with stock rpms :
> httpd.x86_64->2.2.3-6.el5
> mod_ssl.x86_64 -> 2.2.3-6.el5
> mod_dav_svn.x86_64 -> 1.4.2-2.el5
> subversion.x86_64 -> 1.4.2-2.el5
>
> The relevant apache config is this:
Hi Alexadner,

Do you aware about memory leak in mod_ssl when SSLSessionCache using
dbm cache [1]? It was source of huge memory leak in our product. I
recommend you to disable SSL and test again.

[1] http://www.mail-archive.com/modssl-users <at> modssl.org/msg16552.html

--

-- 
Ivan Zhakov
Permalink | Reply | 0 comments |
headers
David Glasser | 1 Jun 2008 08:40

Re: Possible data corruption issue?

I don't have time to think in detail right now, but couldn't the
opposite situation happen: where the user has the text of r1 but the
fake metadata of r2, and commits happily not ever seeing r2?  (ie, if
the update is not a back-update.)

--dave

On Sat, May 31, 2008 at 7:41 PM, Karl Fogel <kfogel <at> red-bean.com> wrote:
> [Search for "RC9" if you want to skip straight to the question of
> whether RC9 is needed.  Material before that is discussion of the bug.]
>
> Karl Fogel <kfogel <at> red-bean.com> writes:
>> I've rethreaded this to get the words "data corruption" in the subject,
>> as it seems to be that kind of bug.  As glasser pointed out in
>>
>>   http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=139319
>>   Message-ID: <1ea387f60805282312v7f03d297w208cda44ba8ec438 <at> mail.gmail.com>
>>
>> in this thread, at the end of his repro recipe a file is left thinking
>> it's at r1 but has the r2 working content *and* r2 text-base.  Even if
>> later we reliably check base-checksum on commit (thus preventing bad
>> file content from entering the repository), there may still be the
>> possibility of property corruption being committed...  I haven't had
>> time to analyze it deeply enough to know one way or the other (hey, it's
>> 3am and I just saw this thread).
>>
>> If we can determine that there is no way for corruption to enter the
>> repository with this bug, then I think it's okay to fix this in 1.5.1.
>> Not ideal, but okay: a broken working copy is annoying, but it's not the
>> same as bad data.  If there *is* a way, though, then we kind of have to
(Continue reading)

Permalink | Reply | 8 comments |

Gmane