On Wed, Mar 12, 2014 at 9:49 AM, Ivan Zhakov <i...@visualsvn.com> wrote:
> On 11 March 2014 21:56, Stefan Fuhrmann <stefan.fuhrm...@wandisco.com> > wrote: > > On Tue, Mar 11, 2014 at 1:23 PM, Ivan Zhakov <i...@visualsvn.com> wrote: > >> > >> On 8 March 2014 01:35, <stef...@apache.org> wrote: > >> > Author: stefan2 > >> > Date: Fri Mar 7 21:35:54 2014 > >> > New Revision: 1575418 > >> > > >> > URL: http://svn.apache.org/r1575418 > >> > Log: > >> > Enable FSFS format 7 repositories to be packed without completely > >> > blocking commits. We simply introduce a separate lock file for > >> > 'svnadmin pack' and take out the global write lock for packing > >> > revprops and switching the shard to "packed" state. > >> > > >> > Most of the run time is spent building the revision pack file > >> > and does not require any synchronization with writers. > >> > > >> [...] > >> > >> > @@ -1987,10 +2005,33 @@ svn_fs_fs__pack(svn_fs_t *fs, > >> > apr_pool_t *pool) > >> > { > >> > struct pack_baton pb = { 0 }; > >> > + fs_fs_data_t *ffd = fs->fsap_data; > >> > + svn_error_t *err; > >> > + > >> > pb.fs = fs; > >> > pb.notify_func = notify_func; > >> > pb.notify_baton = notify_baton; > >> > pb.cancel_func = cancel_func; > >> > pb.cancel_baton = cancel_baton; > >> > - return svn_fs_fs__with_write_lock(fs, pack_body, &pb, pool); > >> > + > >> > + if (ffd->format >= SVN_FS_FS__MIN_PACK_LOCK_FORMAT) > >> > + { > >> > + /* Newer repositories provide a pack operation specific lock. > >> > + Acquire it to prevent concurrent packs. */ > >> > + apr_pool_t *subpool = svn_pool_create(pool); > >> What is the reason for using subpool here? Could you please document > >> it if any, otherwise it looks very confusing. > > > > > > Done in r1576427. > > > Thanks! It's clearer now. But I have suggestions for this code: > 1. It seems mutex is missed for POSIX platform where file lock is > per-process, not per-thread. Like we > fs_fs_shared_data_t->fs_write_lock > You are right. Although it is unlikely for an application to do attempt multi-threaded packs ... ;) > 2. I think the code will be clearer with explicit > svn_fs_fs__with_pack_lock() function. In this case mutexes and subpool > will be abstracted from pack logic, which is good imho. > Both fixed in r1578176. -- Stefan^2.
