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, judging by the code comment having been 
adjusted accordingly, but with no reason given and no mention in 
https://issues.apache.org/jira/browse/SVN-4214 .

- Julian


> https://svn.apache.org/r1213716 (on 2011-12-13)
> > Follow-up to r1213681: also prune the rep-cache when it's present
> > but reportedly not being used.
> [...]
>    /* Prune younger-than-(newfound-youngest) revisions from the rep cache. */
> -  if (ffd->rep_sharing_allowed)
> +  if (ffd->format >= SVN_FS_FS__MIN_REP_SHARING_FORMAT)
>      SVN_ERR(svn_fs_fs__del_rep_reference(fs, max_rev, pool));
> 
> https://svn.apache.org/r1367674 (on 2012-07-31)
> > Fix issue 4214, "svnadmin recover" should not create rep-cache.db.
> > * subversion/libsvn_fs_fs/fs_fs.c (recover_body): Don't create rep-cache.
> [...]
> -  /* Prune younger-than-(newfound-youngest) revisions from the rep cache. */
> -  if (ffd->format >= SVN_FS_FS__MIN_REP_SHARING_FORMAT)
> -    SVN_ERR(svn_fs_fs__del_rep_reference(fs, max_rev, pool));
> +  /* Prune younger-than-(newfound-youngest) revisions from the rep
> +     cache if sharing is enabled taking care not to create the cache
> +     if it does not exist. */
> +  if (ffd->rep_sharing_allowed)
> +    {
> +      svn_boolean_t rep_cache_exists;
> +
> +      SVN_ERR(svn_fs_fs__exists_rep_cache(&rep_cache_exists, fs, pool));
> +      if (rep_cache_exists)
> +        SVN_ERR(svn_fs_fs__del_rep_reference(fs, max_rev, pool));
> +    }
> 
> The function concerned, recover_body(), has since been moved to 
> subversion/libsvn_fs_fs/recovery.c.
> 
> 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?

Reply via email to