Stefan Fuhrmann <stefan.fuhrm...@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
