> -----Original Message-----
> From: Daniel Shahaf [mailto:danielsh_at_elego.de]
> Sent: donderdag 28 maart 2013 13:22
> To: Bert Huijben
> Cc: dev_at_subversion.apache.org
> Subject: 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
>
> Bert Huijben wrote on Thu, Mar 28, 2013 at 12:52:51 +0100:
> >
> >
> > > -----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?)
> >
>
> Bugs happen, this is one way to catch them. Compare r1086222, which is
> basically a special-case of this work.
>
> > 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)
> >
>
> OK, we can move this particular config knob to be provided at runtime by
> whoever creates the svn_fs_t.
>
> > 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.
> >
>
> Don't virus-scan repository config files. (Why would you want to do
> that? Do you fear svn would try to execute the config option's value?)
These scanners scan everything; any file access.
Which might just be for looking that the file is uninteresting, but to get
to that point they already opened the file and read the header. (Equivalent
to what libmagic does)
In many companies everything must be scanned and in most companies that use
virusscanners on Windows the average user is not even allowed to configure
skipping specific directories.
And users are certainly not going to configure a specific new in Subversion
1.8 file to be ignored. That doesn't scale. (And .conf is not a standard
windows extension that would be pre-configured)
The skip handling may introduce bugs, so in many virusscanners the skip
handling is done in the scanner process and not in the kernel module that
hands the file to the scanner. So you already have the penalty of at least
two interprocess communication cycles.
> >
<snip>
Bert
Received on 2013-03-28 13:43:53 CET