[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Testers wanted for FSFS svnadmin recover feature (r23595)

From: Daniel Rall <dlr_at_collab.net>
Date: 2007-03-08 02:06:32 CET

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

This is an archived mail posted to the Subversion Dev mailing list.