Daniel Shahaf wrote on Sat, 27 Dec 2008 at 20:50 +0200:
> I tracked it to get_root_changes_offset(). It returns the offset from the
> start of the revision, but later it's used to seek from the start of the
> pack file.
>
> IOW, it forgets to add the offset of the rev within the pack file.
>
And here's the patch I'm testing:
[[[
Fix 'svnadmin verify' of some packed FSFS repositories by making the
calculation of offsets in the rev file consistent.
* subversion/libsvn_fs_fs/fs_fs.c
(get_root_changes_offset):
Return the offsets from the start of the pack file, not from the start
of the revision data.
(get_fs_id_at_offset):
Don't repeat the logic that's now in get_root_changes_offset().
]]]
[[[
Index: subversion/libsvn_fs_fs/fs_fs.c
===================================================================
--- subversion/libsvn_fs_fs/fs_fs.c (revision 34956)
+++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
@@ -2239,14 +2239,6 @@ get_fs_id_at_offset(svn_fs_id_t **id_p,
apr_hash_t *headers;
const char *node_id_str;
- if (rev < ffd->min_unpacked_rev)
- {
- apr_off_t rev_offset;
-
- SVN_ERR(get_packed_offset(&rev_offset, fs, rev, pool));
- offset += rev_offset;
- }
-
SVN_ERR(svn_io_file_seek(rev_file, APR_SET, &offset, pool));
SVN_ERR(read_header_block(&headers,
@@ -2287,6 +2279,7 @@ get_root_changes_offset(apr_off_t *root_offset,
{
fs_fs_data_t *ffd = fs->fsap_data;
apr_off_t offset;
+ apr_off_t rev_offset;
char buf[64];
int i, num_bytes;
apr_size_t len;
@@ -2309,9 +2302,15 @@ get_root_changes_offset(apr_off_t *root_offset,
else
{
seek_relative = APR_END;
- offset = 0;
+ rev_offset = offset = 0;
}
+ /* Offset of the revision from the start of the pack file, if applicable. */
+ if (rev < ffd->min_unpacked_rev)
+ SVN_ERR(get_packed_offset(&rev_offset, fs, rev, pool));
+ else
+ rev_offset = 0;
+
/* We will assume that the last line containing the two offsets
will never be longer than 64 characters. */
SVN_ERR(svn_io_file_seek(rev_file, seek_relative, &offset, pool));
@@ -2350,7 +2349,7 @@ get_root_changes_offset(apr_off_t *root_offset,
i++;
if (root_offset)
- *root_offset = apr_atoi64(&buf[i]);
+ *root_offset = rev_offset + apr_atoi64(&buf[i]);
/* find the next space */
for ( ; i < (num_bytes - 2) ; i++)
@@ -2365,7 +2364,7 @@ get_root_changes_offset(apr_off_t *root_offset,
/* note that apr_atoi64() will stop reading as soon as it encounters
the final newline. */
if (changes_offset)
- *changes_offset = apr_atoi64(&buf[i]);
+ *changes_offset = rev_offset + apr_atoi64(&buf[i]);
return SVN_NO_ERROR;
}
]]]
I'll appreciate a review by someone more familiar with FSFS than me.
Daniel
P.S. I've also noticed that 'svnadmin recover svn-r9-packed' (where
svn-r9-packed is the repository created by the script I gave in the
original mail) fails (both with and without the patch) with the error:
..\..\..\subversion\libsvn_fs_fs\fs_fs.c:6155: (apr_err=160004)
svnadmin: Expected current rev to be <= 0 but found 9
> Daniel
>
> Daniel Shahaf wrote on Sat, 27 Dec 2008 at 15:39 +0200:
> > Summary: 'svnadmin pack' can bring the repository to a state where
> > 'svnadmin verify' fails.
> >
> >
> > I can reproduce it with the svn repository, I couldn't find how to
> > reproduce it in a Greek tree repository.
> >
> > Reproduction script:
> >
> > @echo off
> >
> > cd \tmp\svn
> >
> > echo @@@ Versions
> > svn --version | head -2
> > svnsync --version | head -2
> > svnadmin --version | head -2
> >
> > echo @@@ Recreate the repository
> > rd/s/q r
> > svnadmin create r
> > touch r\hooks\pre-revprop-change.bat
> >
> > echo @@@ Set small shards (so packing has an effect)
> > ### Couldn't get perl to maintain EOL style.
> > vim -Nneu NONE -i NONE -b -c "set noreadonly | 2s/sharded 1000/sharded 4/ | wq!" r\db\format
> >
> > echo @@@ Hack. Force the sync to abort after syncing r9.
> > mkdir r\db\revprops\2\10
> >
> > echo @@@ Sync.
> > svnsync init file:///tmp/svn/r http://svn.collab.net/repos/svn
> > svnsync sync file:///tmp/svn/r
> > echo @@@ The above error was expected.
> >
> > echo @@@ This will work:
> > svnadmin verify -r2 r
> >
> > echo @@@ Packing...
> > svnadmin pack r
> >
> > echo @@@ This will fail:
> > svnadmin verify -r2 r
> >
> >
> > Output:
> >
> > @@@ Versions
> > svn, version 1.6.0 (dev build-r34949)
> > compiled Dec 27 2008, 00:28:10
> > svnsync, version 1.6.0 (dev build-r34949)
> > compiled Dec 27 2008, 00:28:10
> > svnadmin, version 1.6.0 (dev build-r34949)
> > compiled Dec 27 2008, 00:28:10
> > ### I actually used r34952; the output is wrong.
> > @@@ Recreate the repository
> > @@@ Set small shards (so packing has an effect)
> > @@@ Hack. Force the sync to abort after syncing r9.
> > @@@ Sync.
> > Copied properties for revision 0.
> > Transmitting file data .........................................................
> > ................................................................................
> > ................................................................................
> > ................................................................................
> > ................................................................................
> > ................................................................................
> > .........................................................
> > Committed revision 1.
> > Copied properties for revision 1.
> > Transmitting file data .
> > Committed revision 2.
> > Copied properties for revision 2.
> > Transmitting file data .
> > Committed revision 3.
> > Copied properties for revision 3.
> > Transmitting file data .
> > Committed revision 4.
> > Copied properties for revision 4.
> > Transmitting file data .
> > Committed revision 5.
> > Copied properties for revision 5.
> > Transmitting file data .
> > Committed revision 6.
> > Copied properties for revision 6.
> > Transmitting file data .
> > Committed revision 7.
> > Copied properties for revision 7.
> > Transmitting file data .
> > Committed revision 8.
> > Copied properties for revision 8.
> > Transmitting file data .
> > Committed revision 9.
> > Copied properties for revision 9.
> > Transmitting file data .
> > ..\..\..\subversion\libsvn_subr\io.c:2900: (apr_err=720005)
> > svnsync: Can't move '\tmp\svn\r\db\transactions\9-9.txn\props' to '\tmp\svn\r\db\revprops\2\10': Access is denied.
> > @@@ The above error was expected.
> > @@@ This will work:
> > * Verified revision 2.
> > @@@ Packing...
> > @@@ This will fail:
> > ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:3695: (apr_err=160004)
> > svnadmin: Invalid changes line in rev-file
> >
> > Daniel
> >
> > ------------------------------------------------------
> > http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=993717
> >
>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=993838
Received on 2008-12-27 20:31:58 CET