Is it a problem to have multiple keys pointing to the same repository?
(I think the answer is no)
The problem Stefan tried to solve is one key pointing to multiple repositories… in this case both source and target of the hotcopy.
The svn_fspath__ -> svn_dirent_ fix is really necessary, but the other points might be nice to fix, but I’m guessing that will be mostly impossible.
The rules on how character folding is handled on Windows are just not reproducible in normal applications. (The case folding table depends on the exact collation used when creating the volume.) Obtaining the true path name from disk (which is the only stable way I can think of) is usually too expensive to use in this caching scenario.
Bert
From: Stefan Fuhrmann [mailto:stefan.fuhrmann_at_wandisco.com]
Sent: woensdag 23 april 2014 13:58
To: Ivan Zhakov
Cc: Subversion Development; Stefan Fuhrman
Subject: Re: svn commit: r1589262 - /subversion/trunk/subversion/libsvn_fs_fs/fs.c
On Wed, Apr 23, 2014 at 1:28 PM, Ivan Zhakov <ivan_at_visualsvn.com <mailto:ivan_at_visualsvn.com> > wrote:
On 22 April 2014 23:36, <stefan2_at_apache.org <mailto: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 <http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs.c?rev=1589262&r1=1589261&r2=1589262&view=diff> &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 14:35:06 CEST