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

Re: FSFS Issue...

From: Malcolm Rowe <malcolm-svn-dev_at_farside.org.uk>
Date: 2005-12-02 18:39:31 CET

Hi John,

I've been looking at this issue for the past few days.

I really can't see what's going wrong. I've audited the FSFS write
code, and it looks fine. Critically, I can't see how to drive
libsvn_fs_fs/fs_fs.c in such a way as to get the rev files we're seeing.

The rev file is only written once (when the proto-rev file is moved into
place), and the proto-rev file is only ever appended to (and as a stream:
so there's no possibility of parts of it being overwritten).

It's possible that there's a problem re-reading and writing the noderev
hash, but if there is, I can't see it (and it would have to affect
transaction noderev's only). Similarly, there could possibly be a
problem in the code that rewrites a mutable (rev -1) noderev hash to
point it to the current revision number - I've not checked that, but it
doesn't seem particularly likely.

However, there _must_ be something wrong in the FSFS code, because I can't
explain how the 'text' line is written out with the wrong rep-offset.
That offset is only set in rep_write_get_baton(), just before we write
out the DELTA line (and we're not getting a repeated DELTA).

I've looked at pool management, and I can't see anything wrong in
that area. I've not yet run valgrind over it though, so I could have
easily missed something. (Could someone point me towards instructions
for enabling pool debugging please?)

However, the numbers don't really look random enough for this to be
simple memory corruption: as you point out, the 'text' offset always
points inside the delta rep (whereas I'd expect it to point into infinity
if it was just a random overwrite).

I think there must (also?) be something wrong in the svndiff code, since,
as you pointed out, the windows are repeating. This is a dump from your
r94 revision:

0001@00000017: src 00000000 len 19000 tgtlen 19000 isize 0119 dsize 000055e4
0002@00005720: src 00019000 len 19000 tgtlen 19000 isize 0125 dsize 000002c3
0003@00005b15: src 00032000 len 19000 tgtlen 19000 isize 0150 dsize 00000127
...
0056@0007bc43: src 0084d000 len 19000 tgtlen 19000 isize 0083 dsize 00000221
0057@0007bef5: src 00866000 len 19000 tgtlen 19000 isize 007a dsize 00006380
0058@000822fd: src 0087f000 len 19000 tgtlen 19000 isize 0023 dsize 00018c4c
0059@0009af7a: src 00898000 len 19000 tgtlen 19000 isize 0023 dsize 00018c4a
005a@000b3bf5: src 008b1000 len 19000 tgtlen 19000 isize 0015 dsize 00018e2c
005b@000cca44: src 008ca000 len 19000 tgtlen 19000 isize 0015 dsize 00018e2c
005c@000e5893: src 008e3000 len 19000 tgtlen 19000 isize 0025 dsize 00018c4c
005d@000fe512: src 008fc000 len 19000 tgtlen 19000 isize 0015 dsize 00018f09
005e@0011743e: src 00915000 len 17200 tgtlen 19000 isize 000e dsize 00018ef8
005f@00130352: src 0092c200 len 00000 tgtlen 19000 isize 057d dsize 0000023c
0060@00130b17: src 0092c200 len 00000 tgtlen 19000 isize 074e dsize 0000043d
0061@001316ae: src 00000030 len 00007 tgtlen 00000 isize 0000 dsize 001f0990
window ends past end of buffer

(For this file, if the expanded size is correct, I'd expect 0x62 full size
(target size 0x19000) windows, and one smaller window).

The first 0x60 windows look completely regular (I assume the source window
dropping to zero is because the source stream emptied; and I assume the
large data sizes in the final windows are because the source window was
not useful for a diff in those cases).

Window 0x61 appears completely broken: it's moved backward in the source
stream, it's got a zero target length, a zero instruction size, and an
enormous data size. My hacky program doesn't think there's enough data
left in the rep to get to the next window. (Could you double-check
my decoding?). Possibly worth noting that the 'text' offset points
inside window 0x60, which is the last one that looks good, so window
0x61 might be just junk.

It's also interesting that if we subtract the 'text' size (1347483) from
the ENDREP offset (2596801), we get 1249318, which is 19 bytes after the
'text' offset (1249299), which is exactly the correct number of bytes
for the DELTA line, implying that b->rep_offset and b->delta_start _both_
had to be wrong, but wrong in the correct way, if you see what I mean.

Of course, the only DELTA line (and svndiff0 header) appears at offset
zero.

Other stuff:
I've taken a look at the Red Hat packages of Subversion 1.2.1, and they
look fine - there are build patches, but nothing that looks relevant.

APR 0.9.x looks fine as well (there are some recent bugfixes, but nothing
that looks significant, whatever version people were using). I haven't
checked Red Hat's APR package though, so they might potentially have
local patches in it.

The APR file-handling code looks solid: the buffering is a bit complex,
but it doesn't look like it should be causing a problem (it's mostly
thin wrappers around open/read/etc). The only bugfix in recent memory
won't affect us.

I wondered whether this might be caused by a disk-write error somehow
(since you mentioned failing hardware in some cases): if a transient
error caused a failure at transaction commit time, I wondered if a
higher-level function could get an error and try to re-run the commit.
However, even if that was the case, I can't explain the behaviour we're
seeing now (and I think that disk-write errors probably would just kill
the commit completely).

In short, I'm a bit stumped. I think we need more real data (the output
from your tool is useful, but it only goes so far). What I'd really
like is a complete un-recovered repository, and possibly an uncorrupted
copy of the corrupt revision's file if possible. And a pony.

Regards,
Malcolm

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Dec 2 18:42:53 2005

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