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

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

From: 'Daniel Shahaf' <danielsh_at_elego.de>
Date: Thu, 28 Mar 2013 15:02:29 +0200

Bert Huijben wrote on Thu, Mar 28, 2013 at 13:39:36 +0100:
>
>
> > -----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.
>

Who cares about the average user? This is server code.

> 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.
>

Right, because for some unexplainable reason you chose to run a virus
scanner on a server box. That's about as questionable to me as running
'verify' during the commit process seems to be to you. :-)

> > >
> <snip>
>
> Bert
>
Received on 2013-03-28 14:03:23 CET

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