You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <ph...@wandisco.com> on 2012/07/31 19:32:21 UTC

FSFS verifies rep-cache when disabled

"svnadmin verify" verifies a rep-cache.db file even when rep-caching is
disabled.  This appears to be intentional but I don't understand the
reasoning.

svn_fs_fs__verify calls svn_fs_fs__exists_rep_cache to see if the cache
exists and then calls svn_fs_fs__walk_rep_reference which has the
comment:

  /* Don't check ffd->rep_sharing_allowed. */
  SVN_ERR_ASSERT(ffd->format >= SVN_FS_FS__MIN_REP_SHARING_FORMAT);

Why should verify attempt to verify a cache that is disabled?

-- 
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download

Re: FSFS verifies rep-cache when disabled

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Bert Huijben wrote on Tue, Jul 31, 2012 at 22:11:31 +0200:
> 
> 
> > -----Original Message-----
> > From: MARTIN PHILIP [mailto:codematters@ntlworld.com] On Behalf Of
> > Philip Martin
> > Sent: dinsdag 31 juli 2012 21:30
> > To: Julian Foad
> > Cc: dev@subversion.apache.org
> > Subject: Re: FSFS verifies rep-cache when disabled
> > 
> > Julian Foad <ju...@btopenworld.com> writes:
> > 
> > > Philip Martin wrote:
> > >
> > >>& quot;svnadmin verify" verifies a rep-cache.db file even when
> > >> rep-caching is disabled.  This appears to be intentional but I don't
> > >> understand the reasoning.
> > >>
> > >> svn_fs_fs__verify calls svn_fs_fs__exists_rep_cache to see if the
> > >> cache exists and then calls svn_fs_fs__walk_rep_reference which has
> > >> the comment:
> > >>
> > >>   /* Don't check ffd->rep_sharing_allowed. */
> > >> SVN_ERR_ASSERT(ffd->format >=
> > SVN_FS_FS__MIN_REP_SHARING_FORMAT);
> > >>
> > >> Why should verify attempt to verify a cache that is disabled?
> > >
> > >
> > > I have a vague recollection that we argued at the time, that if and
> > > when the admin twiddles the knob to enable it, then it will be used
> > > without any further checking (or emptying) of it at that time, so it
> > > is part of the repository state that needs to be correct for the
> > > repository (including its knobs) to be considered 'verified good'.
> > > And that still makes sense to me.
> 
> If it isn't enabled at all the performance cost of verifying is almost nil (but we shouldn't create the DB if it doesn't exist).

We don't, largely because you pointed that out in your commit review when
the above code was written.

Daniel
(I'm behind on my dev@ traffic, haven't read the other concurrent
threads I'm CC'd on yet)

RE: FSFS verifies rep-cache when disabled

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: MARTIN PHILIP [mailto:codematters@ntlworld.com] On Behalf Of
> Philip Martin
> Sent: dinsdag 31 juli 2012 21:30
> To: Julian Foad
> Cc: dev@subversion.apache.org
> Subject: Re: FSFS verifies rep-cache when disabled
> 
> Julian Foad <ju...@btopenworld.com> writes:
> 
> > Philip Martin wrote:
> >
> >>& quot;svnadmin verify" verifies a rep-cache.db file even when
> >> rep-caching is disabled.  This appears to be intentional but I don't
> >> understand the reasoning.
> >>
> >> svn_fs_fs__verify calls svn_fs_fs__exists_rep_cache to see if the
> >> cache exists and then calls svn_fs_fs__walk_rep_reference which has
> >> the comment:
> >>
> >>   /* Don't check ffd->rep_sharing_allowed. */
> >> SVN_ERR_ASSERT(ffd->format >=
> SVN_FS_FS__MIN_REP_SHARING_FORMAT);
> >>
> >> Why should verify attempt to verify a cache that is disabled?
> >
> >
> > I have a vague recollection that we argued at the time, that if and
> > when the admin twiddles the knob to enable it, then it will be used
> > without any further checking (or emptying) of it at that time, so it
> > is part of the repository state that needs to be correct for the
> > repository (including its knobs) to be considered 'verified good'.
> > And that still makes sense to me.

If it isn't enabled at all the performance cost of verifying is almost nil (but we shouldn't create the DB if it doesn't exist).
If there is data inside then the admin tiddled the knob and I think we should verify it (whether or not it is enabled for new revisions).

> I still find it a bit odd: I'd expect enable-rep-sharing in fsfs.conf to
> control all processing of rep-caching.  What about recover and upgrade?
> Should they also ignore that flag?  If so then my r1367674 change to
> recover is wrong.

Not sure about those.

	Bert
> 
> --
> Certified & Supported Apache Subversion Downloads:
> http://www.wandisco.com/subversion/download


Re: FSFS verifies rep-cache when disabled

Posted by Philip Martin <ph...@wandisco.com>.
Julian Foad <ju...@btopenworld.com> writes:

> Philip Martin wrote:
>
>>& quot;svnadmin verify" verifies a rep-cache.db file even when
>> rep-caching is disabled.  This appears to be intentional but I don't
>> understand the reasoning.
>> 
>> svn_fs_fs__verify calls svn_fs_fs__exists_rep_cache to see if the
>> cache exists and then calls svn_fs_fs__walk_rep_reference which has
>> the comment:
>> 
>>   /* Don't check ffd->rep_sharing_allowed. */  
>> SVN_ERR_ASSERT(ffd->format >= SVN_FS_FS__MIN_REP_SHARING_FORMAT);
>> 
>> Why should verify attempt to verify a cache that is disabled?
>
>
> I have a vague recollection that we argued at the time, that if and
> when the admin twiddles the knob to enable it, then it will be used
> without any further checking (or emptying) of it at that time, so it
> is part of the repository state that needs to be correct for the
> repository (including its knobs) to be considered 'verified good'. 
> And that still makes sense to me.

I still find it a bit odd: I'd expect enable-rep-sharing in fsfs.conf to
control all processing of rep-caching.  What about recover and upgrade?
Should they also ignore that flag?  If so then my r1367674 change to
recover is wrong.

-- 
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download

Re: FSFS verifies rep-cache when disabled

Posted by Julian Foad <ju...@btopenworld.com>.
Philip Martin wrote:

>& quot;svnadmin verify" verifies a rep-cache.db file even when rep-caching 
> is
> disabled.  This appears to be intentional but I don't understand the
> reasoning.
> 
> svn_fs_fs__verify calls svn_fs_fs__exists_rep_cache to see if the cache
> exists and then calls svn_fs_fs__walk_rep_reference which has the
> comment:
> 
>   /* Don't check ffd->rep_sharing_allowed. */
>   SVN_ERR_ASSERT(ffd->format >= SVN_FS_FS__MIN_REP_SHARING_FORMAT);
> 
> Why should verify attempt to verify a cache that is disabled?


I have a vague recollection that we argued at the time, that if and when the admin twiddles the knob to enable it, then it will be used without any further checking (or emptying) of it at that time, so it is part of the repository state that needs to be correct for the repository (including its knobs) to be considered 'verified good'.  And that still makes sense to me.


- Julian