Re: FSFS recovery should prune rep-cache even if its use is disabled

CC'ing Philip...

Daniel Shahaf wrote:
> Julian Foad wrote on Wed, 22 Aug 2018 22:20 +0100:
> > Julian Foad wrote:
> > > It looks like r1213716 ("also prune the rep-cache when it's present but
> > > reportedly not being used") was reverted by r1367674, apparently
> > > unintentionally.
> >
> > Well, with some degree of intention, [... but] no mention in
> > https://issues.apache.org/jira/browse/SVN-4214 .

I just noticed that https://issues.apache.org/jira/browse/SVN-4214 says "Recover should not operate on rep-cache.db if rep-caching is disabled" in its description. At first I misread that line as "Recover should not create rep-cache.db ..." like the issue's summary.

Philip, can you recall anything that might help re-evaluate this now?

> I assume, going by that comment, that Philip's reason for changing the
> condition was that svn_fs_fs__del_rep_reference() would create an empty
> rep-cache.db if it was called when (ffd->format >=
> SVN_FS_FS__MIN_REP_SHARING_FORMAT
> && ffd->rep_sharing_allowed == FALSE).

But in the new inner block, __del_rep_reference() isn't called if rep-cache.db doesn't exist, so it can't create it.

> > > If "recovery" while re-sharing is disabled (by the fsfs.conf setting)
> > > leaves future revision entries in the rep-cache, then later re-enabling
> > > the rep-cache could cause serious corruption if those entries are then
> > > used.
> > >
> > > Therefore I think we should repeat r1213716 as a bug fix.
> > >
> > > WDYT?
>
> +1, no question about it. Or rather, I think the question is whether to
> backport it only to 1.10 or also to 1.9...

It can potentially lead to data loss, so we should backport to 1.9 as well.

-- 
- Julian