Stefan Fuhrmann <stefan.fuhrmann_at_wandisco.com> writes:
> Sorry, my bad, I was wrong there. I only remembered that we had and still
> have the pack/hotcopy race condition in 1.8 and then I saw the recursive
> directory copy calls in the /trunk hotcopy code and had my false positive.
>
> You are absolutely right. We already copy revisions explicitly and will
> simply stop upon missing files w/o bumping 'current' (as I had suggested
> yesterday for a future improvement). And there is plenty of commentary
> on that issue in the code as well. A user might later retry the hotcopy
> and they will likely succeed.
Thanks for confirming my thoughts.
> So, at the moment, the pack lock prevents those (in some setups rare)
> ENOENT errors. I think that apparent reliability (users don't see failures
> once on a blue moon) is worth keeping. I'll try to come up with a patch
> for the r/o issue tomorrow.
So, we have a regression in how 'svnadmin hotcopy' behaves in 1.9.0-rc2 in
terms that it's no longer a read-only operation — as it used to be. I'd say
that this is much more important than providing a better usability in an edge
case when a repository is being concurrently packed and hotcopied (as we now
agree that the integrity of the hotcopy target is not a suspect).
Furthermore, with such a fix we would be providing better usability only when
a lot of preconditions are met, i.e., the filesystem should have format 7, it
should be concurrently packed during hotcopy, the hotcopy should run into a
certain race condition (attempt copying a file that just got moved to a pack)
and — finally, this is *still going to fail* in a situation where the account
performing the backup has read-only access to the repository, and the
repository is being packed using an account with write access.
Maybe this is a nice improvement after all, but not if we find ourselves in a
situation where we need to come up with a non-trivial fix just to get rid of
a regression in the stable branch. As I said in my first e-mail, I think that
the proper solution would be reverting r1589284, pushing this change — with
a test, perhaps — to /branches/1.9.x. Then we could think on doing something
with the aforementioned usability problem, but only after we're done with the
real problem.
I am ready to do this, if no one objects.
Regards,
Evgeny Kotkov
Received on 2015-06-18 15:25:05 CEST