On Wed, 07 Mar 2007, David Glasser wrote:
> On 3/7/07, Daniel Rall <dlr@collab.net> wrote:
> >> 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?
>
> My impression is that printing a "lock acquired" message is
> appropriate for FSFS, but the time it is printed is too early. (The
> current svn_fs_fs__recover implementation wraps most of the
> calculation in a write lock on the repository, but this isn't called
> until *after* svn_svn_recover2 calls the callback which prints the
> message.)
Gotcha. So moving the locking down into libsvn_fs_base would not fix
this problem -- we'd have to change the message printed by the
svnadmin callback to "Acquiring repository lock..." or some such, or
punch the invocation of the callback down into the FS layer.
> >> 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.
>
> OK, so it appears that some repos things require locking on BDB but
> not FSFS, and this is special-cased inside libsvn_repos instead of
> just letting libsvn_fs_base do the locking. Is there a good reason to
> not move the locking into libsvn_fs_base?
Just guessing here, but perhaps there are some BDB operations which
don't require a repository-global lock?
It still seems like the FS layer should be managing the lock...
- application/pgp-signature attachment: stored
Received on Thu Mar 8 23:42:08 2007