On 23 April 2014 15:57, Stefan Fuhrmann <stefan.fuhrmann_at_wandisco.com> wrote:
> 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.
>
I don't have solution, except adding second 'instance UUID' that's
updated on hotcopy and repository create.
But you fix making things worse: before your change locks only
scalability/performance issue exists, but now same repositories
accessed with different path will get different shared data and
transaction list will not be locked correctly.
I disagree with Bert that problem I describe is not possible in normal
application: any server using SVNParentPath and svnserve are affected,
because it uses part of URL as repository name. So users accessing
http://server/repo and http://server/REPO will access repository with
different path and get different FS shared data.
--
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com
Received on 2014-04-23 15:08:42 CEST