On Tue, Mar 11, 2014 at 1:23 PM, Ivan Zhakov <ivan_at_visualsvn.com> wrote:
> On 8 March 2014 01:35, <stefan2_at_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.
> > + 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);
> > + }
> > + else
> > + {
> > + /* Use the global write lock for older repos. */
> > + err = svn_fs_fs__with_write_lock(fs, pack_body, &pb, pool);
> > + }
> > +
> > + return svn_error_trace(err);
> > }
>
Thanks for the feedback!
-- Stefan^2.
Received on 2014-03-11 18:57:12 CET