On Wed, Apr 23, 2014 at 1:28 PM, Ivan Zhakov <ivan_at_visualsvn.com> wrote:
> On 22 April 2014 23:36, <stefan2_at_apache.org> wrote:
> > Author: stefan2
> > Date: Tue Apr 22 19:36:12 2014
> > New Revision: 1589262
> >
> > URL: http://svn.apache.org/r1589262
> > Log:
> > Fix a scalability issue with FSFS repositories: The lock mutexes are
> > part of the process-wide per-repo shared structs. Using only the
> > UUID as identification for the repo is not enough in case a repo gets
> > duplicated or the admins don't follow good practice. The result may
> > be serializing all commits globally across all repos.
> >
> > Now, we simply add the normalized repo path to the hash key but keep
> > the UUID as well. The latter allows us to detect and handle replaced
> > repositories gracefully in some cases.
> >
> > * subversion/libsvn_fs_fs/fs.c
> > (fs_serialized_init): Add FS path to the hash lookup key.
> >
> > Modified:
> > subversion/trunk/subversion/libsvn_fs_fs/fs.c
> >
> > Modified: subversion/trunk/subversion/libsvn_fs_fs/fs.c
> > URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs.c?rev=1589262&r1=1589261&r2=1589262&view=diff
> >
> ==============================================================================
> > --- subversion/trunk/subversion/libsvn_fs_fs/fs.c (original)
> > +++ subversion/trunk/subversion/libsvn_fs_fs/fs.c Tue Apr 22 19:36:12
> 2014
> > @@ -72,19 +72,12 @@ fs_serialized_init(svn_fs_t *fs, apr_poo
> > each separate repository opened during the lifetime of the
> > svn_fs_initialize pool. It's unlikely that anyone will notice
> > the modest expenditure; the alternative is to allocate each
> structure
> > - in a subpool, add a reference-count, and add a serialized
> deconstructor
> > - to the FS vtable. That's more machinery than it's worth.
> > -
> > - Using the uuid to obtain the lock creates a corner case if a
> > - caller uses svn_fs_set_uuid on the repository in a process where
> > - other threads might be using the same repository through another
> > - FS object. The only real-world consumer of svn_fs_set_uuid is
> > - "svnadmin load", so this is a low-priority problem, and we don't
> > - know of a better way of associating such data with the
> > - repository. */
> > + in a subpool, add a reference-count, and add a serialized
> destructor
> > + to the FS vtable. That's more machinery than it's worth. */
> >
> > SVN_ERR_ASSERT(fs->uuid);
> > key = apr_pstrcat(pool, SVN_FSFS_SHARED_USERDATA_PREFIX, fs->uuid,
> > + svn_fs__canonicalize_abspath(fs->path, pool),
> > SVN_VA_NULL);
> Hi Stefan,
>
> I agree that problem is important and should be fixed somehow, but
> committed fix is wrong IMHO:
> 1. svn_fs__canonicalize_abspath is for FS path canonicalization, not
> for disk path
> 2. fs->path is not absolute path, so you may get different keys for
> same repository accessed but different path relative path
> 3. On Windows path names is case insensitive and repository can be
> accessed using 'C:\REPO' and 'C:\repo' path names. Converting path to
> upper case because Windows performs different normalization like
> removing trailing dot and etc. Reimplementing this logic in Subversion
> will be very hard.
>
Alright, do you have a suggestion for how to handle
that problem? Without r1589262, we can't lock the
source *and* target of a hotcopy - which is how I ran
into this issue.
-- Stefan^2.
Received on 2014-04-23 13:58:33 CEST