On Tue, Jun 16, 2015 at 8:12 PM, Evgeny Kotkov <evgeny.kotkov_at_visualsvn.com>
wrote:
> FSFS7 introduced a new on-disk lockfile, db/pack-lock, that allows packing
> a repository without completely blocking the commits. We also extended the
> svn_fs_fs__hotcopy() logic to take this lock for the source repository, see
> r1589284 and the related discussion [1, 2].
>
> I think that this behavior — taking a lock in the filesystem of the source
> repository — is a regression, because now 'svnadmin hotcopy' requires write
> access to the source repository.
Well, not being able to hotcopy a r/o repo is a regression,
preventing packs is not. So, the fix would be not to fail
for r/o repos because we know that those can't be packed.
We could simply attempt to lock the file and if we get an
error, we simply continue with the default / old-format code
path. We might even show a warning in that case.
Of curse, that assumes that the repo is r/o of everyone.
Otherwise, our consistency guarantees for hotcopy fly
out of the window.
> One example of how this prerequisite could
> break existing installations would be a system where an account performing
> the backup only has read access to the source (that sounds logical to me).
> Another example would be using 'svnadmin hotcopy' to import an existing
> repository from something like a read-only SMB share, and an attempt to do
> so would trigger an error for FSFS7 repositories.
>
> Here is a quick reproduction script:
>
> $ svnadmin create source
> $ svnadmin create source-old --compatible-version=1.8
> $ chmod -w -R source
> $ chmod -w -R source-old
> $ svnadmin hotcopy source target
> svnadmin: E000013: Can't open file 'source/db/pack-lock': Permission denied
> $ svnadmin hotcopy source-old target-old
> * Copied revision 0.
>
That makes we wonder how we prevent packing in old
format repositories. We know that hotcopying while
packing can corrupt the destination repo. Also, taking
out any locks on the source should come with the same
problems as we see with the pack lock. I wonder what
we do or should do there.
> Please note that the existence of db/pack-lock in the source is orthogonal
> to
> this reproduction script. Calling 'echo "" > source/db/pack-lock' after
> the
> first step would still trigger the same error for the FSFS7 repository.
>
Good point. The lock file might not exist and the r/o
check must take care of that, too.
> Perhaps we should add a corresponding regression test and revert r1589284?
>
I'm -1 on reverting: Not being able to use hotcopy on
an r/o repo is less problematic than ending up with a
corrupted copy.
Furthermore, checking for r/o and handling that should
be easy enough. So, let's do that.
Once we are at it, let's detect pack operations during
hotcopy (compare pack status before & after the hotcopy).
Then we can at least warn the user of older format repos
that their hotcopy is possibly corrupt now.
How does that sound?
-- Stefan^2.
Received on 2015-06-16 20:49:38 CEST