Bert Huijben wrote on Thu, Mar 28, 2013 at 13:39:36 +0100: > > > > -----Original Message----- > > From: Daniel Shahaf [mailto:danie...@elego.de] > > Sent: donderdag 28 maart 2013 13:22 > > To: Bert Huijben > > Cc: dev@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: danie...@apache.org [mailto:danie...@apache.org] > > > > Sent: donderdag 28 maart 2013 12:37 > > > > To: comm...@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 >
