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