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

[PATCH] lock transaction-current separately from current

From: David Glasser <glasser_at_davidglasser.net>
Date: 2007-10-04 18:22:49 CEST

Wow, that was a terrible subject line for a patch.

--dave

---------- Forwarded message ----------
From: David Glasser <glasser@davidglasser.net>
Date: Oct 3, 2007 1:16 PM
Subject: Re: svn commit: r25869 - in trunk/subversion: include/private
libsvn_fs_fs tests/libsvn_fs
To: David Glasser <glasser@davidglasser.net>, Blair Zajac
<blair@orcaware.com>, dev@subversion.tigris.org

On 9/19/07, Malcolm Rowe <malcolm-svn-dev@farside.org.uk> wrote:
> On Wed, Sep 19, 2007 at 09:26:02AM -0700, David Glasser wrote:
> > On 9/19/07, Blair Zajac <blair@orcaware.com> wrote:
> > > Didn't think of locking transaction-current. So we could definitely
> > > do that.
> >
> > Yes, if it is feasible to make the transaction-current and commit
> > locks separate (and I don't see why it wouldn't be...) I would
> > definitely be +1.
> >
>
> We can (and probably should: if we don't fix it for 1.5, we won't be
> able to fix it easily thereafter).
>
> Unfortunately, I forgot that we also need to create and manage a mutex
> for cross-thread synchronisation, so we'd need to duplicate a fair bit
> of the current write-lock code. Anyone see an easy way to abstract that
> away so that we don't need to copy and paste everything? (bonus
> question: anyone see a way to abstract it into a file that's _not_
> fs_fs.c, which is ~180KB and growing?)

The following patch locks transaction-current itself instead of using
the write-lock. (I assume it's OK to lock the file even if we're
changing it by overwriting, right? that is, there's no need to
introduce transaction-current.lock?) epg also suggested locking
revprop files individually instead of using write-lock there; maybe
I'll do that later. It doesn't abstract out of fs_fs.c, but it
doesn't add too many lines of code either.

What do you think?

[[[
To get and increment the FSFS transaction-current file, just lock
'transaction-current' itself instead of 'write-lock', so that new
transactions and commits don't block each other. In order to do so,
abstract out the locking code a little more.

* subversion/libsvn_fs_fs/fs.h
  (fs_fs_shared_data_t): Add a txn_current_lock mutex field to protect
   the transaction-current file from concurrent access by multiple
   threads of one process.

* subversion/libsvn_fs_fs/fs.c
  (fs_serialized_init): Create the txn_current_lock mutex.

* subversion/libsvn_fs_fs/fs_fs.c
  (path_txn_current): New helper function for getting the path of the
   transaction-current file.
  (get_lock_on_filesystem): Rename from get_write_lock, and generalize
   to lock a given file instead of 'write-lock'. (Also move earlier
   in the file.)
  (with_some_lock): Extract most of svn_fs_fs__with_write_lock into
   this function, which takes as argument a lock filename and (if
   threading) a mutex, instead of a svn_fs_t.
  (svn_fs_fs__with_write_lock): Now just a wrapper around
   with_some_lock (and moved earlier in the file).
  (with_txn_current_lock): New function which runs its body with the
   transaction-current file (and mutex) locked.
  (get_and_increment_txn_key_body): Use new path_txn_current helper
   function.
  (create_txn_dir): Call get_and_increment_txn_key_body using
   with_txn_current_lock instead of svn_fs_fs__with_write_lock (this
   change is the whole point of the revision).

* subversion/libsvn_fs_fs/structure
  (Layout of the FS directory): Document that transaction-current is
   modified under its own lock.
]]]

--
David Glasser | glasser_at_davidglasser.net | http://www.davidglasser.net/
-- 
David Glasser | glasser_at_davidglasser.net | 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 Oct 4 18:23:01 2007

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