Daniel Shahaf wrote on Sat, Oct 30, 2010 at 17:01:43 +0200:
> Looks like at least one test fails:
>
> [[[
> lp-shahaf,14:svn/performance% ../runctest fs fs 22
> subversion/tests/libsvn_fs/fs-test.c:3411: (apr_err=160000)
> subversion/tests/libsvn_fs/fs-test.c:3229: (apr_err=160000)
> svn_tests: Error validating revision 1 (youngest is 3)
> subversion/tests/svn_test_fs.c:492: (apr_err=160000)
> svn_tests: Repository tree does not look as expected.
> Corrupt entries:
> A/D/G/rho
> Missing entries:
> Extra entries:
>
> FAIL: lt-fs-test 22: after each commit, check all revisions
> zsh: exit 1 ../runctest fs fs 22
> ]]]
I know what causes it: somewhere there is an offset relative to the
start of the rev file r1, which --- once that file is packed --- is used
as relative to the start of the pack file. This explains why the error
message occurs at r3 (since that's when packing occurs) and why it
affects r1 (since r0's offsets relative to the pack file or to the r0
file are the same).
How I reached this conclusion: I've added a bit of debugging prints (see
attached more-information-in-errors.diff), and got this output:
% make -sj fs-test && CACHE_NODE_REVS=1 ../runctest fs fs 22
subversion/tests/libsvn_fs/fs-test.c:3229: (apr_err=160000)
svn_tests: Error validating revision 1 (youngest is 3)
subversion/tests/svn_test_fs.c:369: (apr_err=160004)
subversion/libsvn_fs_fs/tree.c:2310: (apr_err=160004)
subversion/libsvn_fs_fs/tree.c:2310: (apr_err=160004)
subversion/libsvn_fs_fs/dag.c:922: (apr_err=160004)
subversion/libsvn_fs_fs/fs_fs.c:3851: (apr_err=160004)
subversion/libsvn_fs_fs/fs_fs.c:3360: (apr_err=160004)
subversion/libsvn_fs_fs/fs_fs.c:3252: (apr_err=160004)
subversion/libsvn_fs_fs/fs_fs.c:3222: (apr_err=160004)
svn_tests: Corrupt representation '1 350 36 24 82f2211cf4ab22e3555fc7b835fbc604 56388a031dffbf9df7c32e1f299b1d5d7ef60881 0-0/_s'
subversion/libsvn_fs_fs/fs_fs.c:3175: (apr_err=160004)
subversion/libsvn_fs_fs/fs_fs.c:2707: (apr_err=160004)
svn_tests: Malformed representation header ' 'beta'.'
FAIL: lt-fs-test 22: after each commit, check all revisions
zsh: exit 1 CACHE_NODE_REVS=1 ../runctest fs fs 22
The representation reads (r1, offset 350). At offset 350 in the pack
file, we find 'beta' (just like the error does):
% xxd -s 350 -l 32 subversion/tests/libsvn_fs/test-repo-check-all-revisions/revs/0.pack/pack
000015e: 2027 6265 7461 272e 0a45 4e44 5245 500a 'beta'..ENDREP.
000016e: 4445 4c54 410a 5356 4e01 0000 1a02 1b01 DELTA.SVN.......
However, the correct location is offset 350 from the start of r1 within
the pack file. The manifest file says r1 starts at offset 115, and
115+350=465, and that is indeed where 'rho' is found:
% xxd -s 465 -l 64 subversion/tests/libsvn_fs/test-repo-check-all-revisions/revs/0.pack/pack
00001d1: 4445 4c54 410a 5356 4e01 0000 1802 1901 DELTA.SVN.......
00001e1: 9818 5468 6973 2069 7320 7468 6520 6669 ..This is the fi
00001f1: 6c65 2027 7268 6f27 2e0a 454e 4452 4550 le 'rho'..ENDREP
0000201: 0a44 454c 5441 0a53 564e 0100 0018 0219 .DELTA.SVN......
What to do:
Need to find the location that uses rev1->offset --- I see you've
touched open_pack_or_rev_file() et al on the branch --- and ensure that
it consults the manifest when it should. It is important to note that
a revision may become packed at any moment (even while an FS object is
open); the code compensates for this in various ways/places.
Practical effect of this bug:
Repositories can be packed while live. If this were done with this bug
in place, cached noderevs would have been falsely reported as corrupted.
Daniel
Received on 2010-10-31 04:02:05 CET