On Wed, 07 Mar 2007, David Glasser wrote:
> On 3/7/07, Malcolm Rowe <malcolm-svn-dev@farside.org.uk> wrote:
> >On Wed, Mar 07, 2007 at 01:43:46PM -0500, David Glasser wrote:
> >> I started to run this on my 114285-revision svk depot. The first
> >> thing I noticed was that it seems to not want to be canceled; it seems
> >> to ignore SIGINTS, and I had to kill -9 it to cancel it. I assume
> >> this is intentional?
> >
> >No, it's not intentional as such, but it's the same behaviour as BDB
> >recovery, I believe. Unfortunately the current recovery API doesn't
> >provide for cancellation or feedback, though it should be pretty trivial
> >to extend the new API to include at least cancellation (for FSFS, anyway)
> >- does that sound like a good idea?
>
> I think that it would be useful, especially since there's nothing that
> gets broken by an early cancellation.
+1. Since we're already changed the FS-layer API, might as well add
the parameters necessary for cancellation while we're at it.
> >> More disturbingly, despite the message "Repository lock acquired.", I
> >> was able to run multiple copies of svnadmin recover on the repository
> >> at the same time. When I noticed that, I freaked out and killed them
> >> both; I'll run it for real once I do an actual backup of my computer
> >> at home sometime soon.
> >>
> >
> >Heh. FSFS recovery doesn't require an exclusive lock, so this is actually
> >okay. That said, the user experience is certainly sub-optimal, but it's
> >not entirely clear how we can improve it (the locking is semi-hardcoded
> >in svnadmin and libsvn_repos). Suggestions welcome.
>
> Hmm. But svn_fs_fs__recover does take out a write lock on the
> repository (not an exclusive lock, of course)...
>
> Oh, I see. svn_repos_recover2 opens the repository, and in BDB only
> it takes the lock at this time. It then calls back to svnadmin, which
> prints out "Repository lock acquired." even if the lock was not asked
> for (ie, FSFS). It then gets to svn_fs_fs__recover... at which point
> the second svnadmin blocks. Heh.
You're saying the "lock acquired" message is appropriate even for FSFS?
> Does anyone know what the reason for special-casing BDB locking in
> libsvn_repos/repos.c was (instead of moving that behavior into
> libsvn_fs_base)? Is it possible for us to fix this incongruity?
Ah, in repos.c:lock_repos() (called by get_repos(), called by
svn_repos_recover2()). The doc string added back in r16241 says:
/* There is, at present, nothing within the direct responsibility
of libsvn_repos which requires locking. For historical compatibility
reasons, the BDB libsvn_fs backend does not do its own locking, expecting
libsvn_repos to do the locking for it. Here we take care of that
backend-specific requirement.
The kind of lock is controlled by EXCLUSIVE and NONBLOCKING.
The lock is scoped to POOL. */
The BDB check was added in r16297, with the log message:
------------------------------------------------------------------------
r16297 | maxb | 2005-09-27 03:44:44 -0700 (Tue, 27 Sep 2005) | 12 lines
Teach libsvn_repos to use svn_fs_type() to know the kind of filesystem it
contains, and only apply BDB-specific locking requirements to BDB FSes.
This change *might* (untested) allow the hosting of FSFS repositories on Win9x.
* subversion/libsvn_repos/repos.h (svn_repos_t): New field fs_type.
* subversion/libsvn_repos/repos.c (create_svn_repos_t): Adjust documentation
to reference new fs_type struct member.
(lock_repos): Do locking only for BDB filesystems.
(svn_repos_create): Initialize new svn_repos_t fs_type field.
(get_repos): Initialize new svn_repos_t fs_type field.
(svn_repos_hotcopy): Initialize new svn_repos_t fs_type field.
------------------------------------------------------------------------
- application/pgp-signature attachment: stored
Received on Thu Mar 8 02:07:07 2007