RE: svn commit: r1462054 - in /subversion/branches/verify-at-commit/subversion: include/svn_fs.h libsvn_fs/fs-loader.c libsvn_fs/fs-loader.h

> -----Original Message-----
> From: danielsh_at_apache.org [mailto:danielsh_at_apache.org]
> Sent: donderdag 28 maart 2013 12:37
> To: commits_at_subversion.apache.org
> Subject: svn commit: r1462054 - in /subversion/branches/verify-at-
> commit/subversion: include/svn_fs.h libsvn_fs/fs-loader.c libsvn_fs/fs-
> loader.h
>
> Author: danielsh
> Date: Thu Mar 28 11:36:50 2013
> New Revision: 1462054
>
> URL: http://svn.apache.org/r1462054
> Log:
> On the verify-at-commit branch, add a backend-independent
> implementation:
> a db/fs.conf file.
>
> * subversion/include/svn_fs.h
> (SVN_FS_CONFIG_SECTION_MISC,
> (SVN_FS_CONFIG_OPTION_VERIFY_AT_COMMIT):
> New macros.
>
> * subversion/libsvn_fs/fs-loader.h
> (svn_fs_t.verify_at_commit): New struct member.
>
> * subversion/libsvn_fs/fs-loader.c
> (svn_config.h): Include.
> (CONFIG_FILENAME): New macro.
> (write_config): New helper, based on the eponymous helper in FSFS (rather
> than on write_fs_type()).
> (svn_fs_create): Call write_config().
> (fs_new): Initialise new struct member.
> (svn_fs_open): Read config.
> (svn_fs_commit_txn): Use the config.

Summarizing what was said on #svn-dev

I think we should make this a fsfs only feature that uses the existing fsfs.conf file.

The option doesn't appear to make much sense for the now mostly deprecated for new development BDB filesystem and the new work of stefan2 might give new ideas. Besides if a filesystem needs a verification step to be stable that should be part of the design. Subversion shouldn't start assuming that filesystems are likely to break down. (What would that tell us about the stability of previous versions of Subversion?)

The reading of one file for each access to the repository is a more than measurable slowdown when profiling operations. (Reading fsfs.conf over and over again is one of the most expensive things apache worker processes do when I profiled them. I think stefan2 optimized some of this away)

Opening a file is expensive on Windows and probably on any other system that always uses locking for file accesss and/or on-demand virus scanners and also on network drives.

I don't think introducing yet another config file makes much sense. If users want to turn on verifications at every commit they probably want it for all their repositories (in which case the config option belongs in the apache or svnserve config) or they are looking at specific fsfs issues, in which case the option can be in fsfs.conf which is read anyway.

I wouldn't want to introduce yet another layer of configuration for this verification helper.

But then again I can see reasons that some users want the explicit verification. fsfs.conf and/or a post commit hook are good enough solutions without any performance/design implications.

        Bert
Received on 2013-03-28 12:53:34 CET