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.
--
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com
Received on 2014-04-23 13:29:06 CEST