Re: [PATCH] verify at each commit

On 27.03.2013 07:54, Daniel Shahaf wrote:
> Branko Čibej wrote on Wed, Mar 27, 2013 at 05:14:51 +0100:
>> On 26.03.2013 23:11, Daniel Shahaf wrote:
>>> [[[
>>> Run the per-revision verify code on a transaction just before it becomes
>>> a revision. The intent is to catch corruption bugs as early as possible.
>>>
>>> * subversion/libsvn_fs/fs-loader.c
>>> (svn_fs_commit_txn): As above.
>>> ]]]
>>>
>>> [[[
>>> Index: subversion/libsvn_fs/fs-loader.c
>>> ===================================================================
>>> --- subversion/libsvn_fs/fs-loader.c (revision 1461335)
>>> +++ subversion/libsvn_fs/fs-loader.c (working copy)
>>> @@ -761,6 +761,11 @@ svn_fs_commit_txn(const char **conflict_p, svn_rev
>>> fs_path = svn_fs_path(fs, pool);
>>> #endif
>>>
>>> +#ifdef SVN_DEBUG
>>> + /* Verify. */
>>> + SVN_ERR(svn_fs_verify_rev(fs, txn_root, pool));
>>> +#endif
>>> +
>>> err = txn->vtable->commit(conflict_p, new_rev, txn, pool);
>>>
>>> #ifdef SVN_DEBUG
>>> ]]]
>>>
>>> Maybe this should be optional behaviour in release mode, too?
>> I was thinking the same; as it is, it doesn't really help, since most of
>> the time only Subversion developers will be running our test suite with
>> SVN_DEBUG enabled.
> Right. The patch as is is useful primarily for developers. A
> user-oriented feature would be the same minus the #ifdef plus a knob
> (off by default), possibly combined with a recommendation for 'verify -r'
> in the post-commit hook.
>
> What config file do I put the knob in? I think my options are
> db/fs-type, db/fs.conf, db/fsfs.conf. (with the middle one being the
> cleanest design, but also the most code to implement and introduces a
> new uesr-facing config file)

A new db/config kind of feels like the right place.

-- Brane

-- 
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com