subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Evgeny Kotkov <evgeny.kot...@visualsvn.com>
Subject Re: svn commit: r1619413 - in /subversion/trunk/subversion: include/ libsvn_fs_fs/ libsvn_ra_local/ svnadmin/ svnlook/ tests/libsvn_fs_fs/
Date Fri, 22 Aug 2014 01:32:30 GMT
>> Author: stefan2
>> Date: Thu Aug 21 14:35:05 2014
>> New Revision: 1619413
>>
>> URL: http://svn.apache.org/r1619413
>> Log:
>> Enforce the "everybody or nobody" restriction on revprop caching, i.e.
>> either all processes (usually servers) or non must use revprop caching.
>>
>> This patch makes sure that the first process to use revprop caching will
>> create the revprop generation file before even reading and caching any
>> revprops - instead of auto-creating it upon the first write.  Then this
>> file is being used as an indicator that the repo has been accessed at
>> least once using revprop caching.  FS instances without that feature
>> enabled will then be banned from writing any revprops.
>
> Is this a right thing to do?  I suspect there are several problems:
>
> - Any *existing* caller of our public API, namely svn_repos_open3(),
>   svn_fs_open2() and their deprecated versions, will not be able to change
>   revision properties for repositories that had this feature enabled at
>   least once (!).  We are not the only ones using that API, so should
>   we tell them everyone to update their software depending on it if they
>   would like it to work?
>
> - The server now dictates the *caching* configuration for 'svn' and 'svnlook'
>   clients.  This looks problematic to me, especially considering some other
>   problems stated in http://svn.haxx.se/dev/archive-2014-08/0235.shtml
>   Shouldn't caching be totally optional by its design?
>
> - How should existing administrators turn this feature off after they had it
>   enabled for a while?  With a change like this they cannot really just go
>   with something like SVNCacheRevProps=off, because doing this would result
>   in any attempt of changing a revision property to error out with "Writing
>   revprops requires revprop caching to be enabled".  Do they have to
>   manually remove all the corresponding revprop generation files we now
>   use as a marker?  What if they have 60000 repositories? And how should
>   they know about this in the first place?
>
> - Finally, why cannot we just stop sharing the revprop generation data
>   (as in http://svn.haxx.se/dev/archive-2014-08/0237.shtml)?

I have no idea whether this changeset is designed to be a complete fix or an
opener of a patch series, however, I tend to think that it makes an existing
problem (which seems quite serious to me) even worse.

Apart from all the problems listed above and the fact it does not somehow
address the part of the problem with existing pre-1.8 and 1.8 executables [1],
we could now pretty much screw a workflow of any user that did use the revprop
caching feature at least once (i.e. she/he has a 'db/revprop-generation' file
in the repository).  A bad thing is that using it could be unintentional, as
we enable revprop caching *silently* in 'svnadmin' whenever the named atomics
are considered efficient [2] — hence, this it true for a major part of 64-bit
Windows binaries and for POSIX binaries built with GCC intrinsics (should also
be a major part).  Of course, this could also happen intentionally, i.e. with
SVNCacheRevProps=on or with --cache-revprops, but the "unintentional" part is
a bit more frustrating.

Now, what I mean is that with this changeset and with a 'db/revprop-generation'
file in a repository, all filesystem users with inefficient named atomics (that
should be 32-bit Windows binaries and POSIX binaries built without intrinsics)
are left in a state when they cannot *ever* change a revision property.  They
would be continuously getting the "Writing revprops requires revprop caching to
be enabled" error without any ability to fix this.  Maybe they could ask an
administrator to remove the 'db/revprop-generation' file from repository(-ies).
But then, with somebody else using an "efficient" build of 'svnadmin' even after
the 'db/revprop-generation' file was deleted, this becomes a never-ending
problem, because the efficient 'svnadmin' would be recreating the file again
and again.

[1] http://svn.haxx.se/dev/archive-2014-08/0264.shtml
[2] http://svn.apache.org/r1405553


Regards,
Evgeny Kotkov

Mime
View raw message