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

Re: 'svnadmin recover' with FSFS revprop packing: edge case bug

From: Julian Foad <julian.foad_at_wandisco.com>
Date: Mon, 01 Feb 2010 11:54:56 +0000

On Sun, 2010-01-31, Daniel Shahaf wrote:
> fs_fs.c:recover_body() needs to be taught that the youngest revision
> does not necessarily have a revprop file. The following repository
> demonstrates the bug:
>
> % cat db/format
> 5
> layout sharded 4
> % cat db/current
> 3
> % find db/rev*
> db/revprops
> db/revprops/revprops.db
> db/revs
> db/revs/0.pack
> db/revs/0.pack/manifest
> db/revs/0.pack/pack
> % svnadmin recover ./
> Repository lock acquired.
> Please wait; recovering the repository may take some time...
> subversion/libsvn_repos/repos.c:1740: (apr_err=160004)
> subversion/libsvn_fs_fs/fs_fs.c:606: (apr_err=160004)
> subversion/libsvn_fs_fs/fs_fs.c:6780: (apr_err=160004)
> svnadmin: Revision 3 has a revs file but no revprops file
> %
>
> I created the repository as follows:
>
> % svnadmin create r
> % sed -i 2s/1000/4/ r/db/format
> % for i in 1 2 3; do svn mkdir file://`pwd`/r/$i -mr$i; done
>
> Found by: svnadmin_tests 13, when built with
> -DSVN_FS_FS_DEFAULT_MAX_FILES_PER_DIR=4 and -DPACK_AFTER_EVERY_COMMIT.

Well found!

To help catch these kinds of bugs, it would be a good idea to make the
test suite use a low fsfs-sharding number all the time, would it not?

[[[
Index: subversion/tests/cmdline/svntest/main.py
===================================================================
--- subversion/tests/cmdline/svntest/main.py (revision 905239)
+++ subversion/tests/cmdline/svntest/main.py (working copy)
@@ -190,7 +190,7 @@
 
 # Global variable: Number of shards to use in FSFS
 # 'None' means "use FSFS's default"
-fsfs_sharding = None
+fsfs_sharding = 2
 
 # Global variable: automatically pack FSFS repositories after every commit
 fsfs_packing = None
]]]

(Golden rule: if an edge case happens seldom, make it happen often.)

This wouldn't have caught the above bug by itself, of course: that
requires packing after every commit. It would not be right to make the
test suite default to packing after ever commit, because then the normal
(partly packed, partly unpacked) case would not get tested.

- Julian
Received on 2010-02-01 12:55:33 CET

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