[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Layering violations in the FS cache code? (was "svn commit: r1091573 - /subversion/trunk/build.conf")

From: Stefan Fuhrmann <eqfox_at_web.de>
Date: Sat, 16 Apr 2011 14:40:30 +0200

On 14.04.2011 22:32, Branko Čibej wrote:
> On 14.04.2011 21:03, Daniel Shahaf wrote:
>> On Thu, 14 Apr 2011 14:43 -0400, "C. Michael Pilato"<cmpilato_at_collab.net> wrote:
>>> On 04/13/2011 06:52 PM, Stefan Fuhrmann wrote:
>>>> The code in question has evolved over many months so it is
>>>> very possible that the name of svn_fs_get_cache_config()
>>>> and friends is no longer appropriate. The same goes for the
>>>> place that this has been implemented.
>>>> From a design perspective, I think it is perfectly to o.k. to
>>>> expose resource limits on the server UI level. The fact that FSFS
>>>> is currently the only part of the server that uses these settings
>>>> does not change, IMO, the fact that they are part of the UI.
>>> I don't have an opinion about exposing resource limits via the server
>>> UI. I do have a strong opinion about the code honoring the flow of the
>>> layered library design we have.
I understand that and share that POV. We just stumbled
across some functionality that is somewhat hard to sort.
>>> If you want to tweak server settings
>>> from client code, you need to do so via the RA vtable(), implementing
>>> the logic to do so (or to expressed *not* do so) in every RA provider.
RA vtable would not be the right location because these
settings are global for the server and not session specific.

Again, this is my mistake because I picked RA init code
to tweak the default resource settings. The "clever" part
about that was that it will only influence the *local* process,
i.e. will only affect ra_local data access.
>>> On 04/13/2011 06:52 PM, Stefan Fuhrmann wrote:
>>>> Maybe we should simply move the function in question to libsvn_subr
>>>> and rename them properly.
>>> On 04/13/2011 11:12 PM, Daniel Shahaf wrote:
>>>> I assumed the resolution would be to move that function to libsvn_fs, not
>>>> to libsvn_subr...
>>> I'm honestly not quite sure exactly where the right place is. I don't
>>> really see what moving it to libsvn_fs does for us -- IMO, it's still
>>> wrong for svn_ra_initialize() call into that. (libsvn_ra should only
>>> call into libsvn_subr, libsvn_delta, and its own RA provider vtable.)
>> Would svn_cmdline_init() be a better place to call the init
>> function from?
> Clearly not, since that function is a startup helper for command-line
> programs, not for libraries.
There seem to be two separate questions to answer:
(1) Where to put the functionality?
(2) Where to call it?

I get the impression that there is some consensus that
the answer to (1) is libsvn_subr.

The answer to the second one is more difficult. To default-
override the default settings this must be done early, i.e.
before they will be evaluated / used. And clients like TSVN
must be able to change the settings after the client libs
changed the defaults and before the settings get used.

Since the changed default is only relevant for time-critical
clients, i.e. those being run many times automatically, e.g.
as part of some script. IMHO, it would be fine to simply
move the override code to the SVN.EXE main(). This would
also make it symmetric to svnadmin and svnserve.

-- Stefan^2.
Received on 2011-04-16 14:40:58 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.