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.)
> > 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?
--dave
--
David Glasser | glasser_at_mit.edu | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Mar 8 05:31:38 2007