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?
Received on 2018-08-22 23:20:20 CEST