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

Re: packing causes 'svnadmin verify' to fail

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Sat, 27 Dec 2008 21:11:14 +0200 (Jerusalem Standard Time)

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

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