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

Reply via email to