You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Bert Huijben <be...@qqmail.nl> on 2014/03/17 09:47:56 UTC
RE: svn commit: r1578176 - in /subversion/trunk/subversion/libsvn_fs_fs: fs.c fs.h fs_fs.c fs_fs.h pack.c
> -----Original Message-----
> From: stefan2@apache.org [mailto:stefan2@apache.org]
> Sent: maandag 17 maart 2014 00:10
> To: commits@subversion.apache.org
> Subject: svn commit: r1578176 - in
> /subversion/trunk/subversion/libsvn_fs_fs: fs.c fs.h fs_fs.c fs_fs.h pack.c
>
> Author: stefan2
> Date: Sun Mar 16 23:09:45 2014
> New Revision: 1578176
>
> URL: http://svn.apache.org/r1578176
> Log:
> Model the FSFS pack lock similarly to our other file based locks in FSFS.
> This gives us proper mutex functionality should multiple threads try to
> pack the same repo at the same time.
>
> * subversion/libsvn_fs_fs/fs.h
> (fs_fs_shared_data_t): Add a thread mutex alongside the file lock.
>
> * subversion/libsvn_fs_fs/fs.c
> (fs_serialized_init): Initialize the new mutex.
>
> * subversion/libsvn_fs_fs/fs_fs.h
> (svn_fs_fs__get_lock_on_filesystem): Privatize again.
> (svn_fs_fs__with_pack_lock): New lock handling function similar
> to what we do for the other locks.
>
> * subversion/libsvn_fs_fs/fs_fs.c
> (path_pack_lock): New utility function.
> (svn_fs_fs__get_lock_on_filesystem): Rename back to ...
> (get_lock_on_filesystem): ... this again.
> (with_some_lock_file): Update caller.
> (svn_fs_fs__with_pack_lock): Implement the new lock function.
>
> * subversion/libsvn_fs_fs/pack.c
> (svn_fs_fs__pack): Use the new pack lock handling function.
It looks like this patch also removed a subpool around the packing in pack.c.
Was that an intended behavior change?
Bert
Modified: subversion/trunk/subversion/libsvn_fs_fs/pack.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/pa
> ck.c?rev=1578176&r1=1578175&r2=1578176&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_fs_fs/pack.c (original)
> +++ subversion/trunk/subversion/libsvn_fs_fs/pack.c Sun Mar 16 23:09:45
> 2014
> @@ -2023,14 +2023,7 @@ svn_fs_fs__pack(svn_fs_t *fs,
> separate subpool here to release the lock immediately after the
> operation finished.
> */
> - apr_pool_t *subpool = svn_pool_create(pool);
> - const char *lock_path = svn_dirent_join(fs->path,
> PATH_PACK_LOCK_FILE,
> - subpool);
> - err = svn_fs_fs__get_lock_on_filesystem(lock_path, subpool);
> - if (!err)
> - err = pack_body(&pb, subpool);
> -
> - svn_pool_destroy(subpool);
> + err = svn_fs_fs__with_pack_lock(fs, pack_body, &pb, pool);
> }
> else
> {
>
Re: svn commit: r1578176 - in /subversion/trunk/subversion/libsvn_fs_fs:
fs.c fs.h fs_fs.c fs_fs.h pack.c
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Mon, Mar 17, 2014 at 9:47 AM, Bert Huijben <be...@qqmail.nl> wrote:
>
>
> > -----Original Message-----
> > From: stefan2@apache.org [mailto:stefan2@apache.org]
> > Sent: maandag 17 maart 2014 00:10
> > To: commits@subversion.apache.org
> > Subject: svn commit: r1578176 - in
> > /subversion/trunk/subversion/libsvn_fs_fs: fs.c fs.h fs_fs.c fs_fs.h
> pack.c
> >
> > Author: stefan2
> > Date: Sun Mar 16 23:09:45 2014
> > New Revision: 1578176
> >
> > URL: http://svn.apache.org/r1578176
> > Log:
> > Model the FSFS pack lock similarly to our other file based locks in FSFS.
> > This gives us proper mutex functionality should multiple threads try to
> > pack the same repo at the same time.
> >
> > * subversion/libsvn_fs_fs/fs.h
> > (fs_fs_shared_data_t): Add a thread mutex alongside the file lock.
> >
> > * subversion/libsvn_fs_fs/fs.c
> > (fs_serialized_init): Initialize the new mutex.
> >
> > * subversion/libsvn_fs_fs/fs_fs.h
> > (svn_fs_fs__get_lock_on_filesystem): Privatize again.
> > (svn_fs_fs__with_pack_lock): New lock handling function similar
> > to what we do for the other locks.
> >
> > * subversion/libsvn_fs_fs/fs_fs.c
> > (path_pack_lock): New utility function.
> > (svn_fs_fs__get_lock_on_filesystem): Rename back to ...
> > (get_lock_on_filesystem): ... this again.
> > (with_some_lock_file): Update caller.
> > (svn_fs_fs__with_pack_lock): Implement the new lock function.
> >
> > * subversion/libsvn_fs_fs/pack.c
> > (svn_fs_fs__pack): Use the new pack lock handling function.
>
>
> It looks like this patch also removed a subpool around the packing in
> pack.c.
>
> Was that an intended behavior change?
>
Yes, it is. The subpool is already provided by
the with_some_lock_file() function in fs_fs.c.
-- Stefan^2.