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

Julian Foad wrote on Thu, Aug 23, 2018 at 10:21:17 +0100:
> +++ subversion/libsvn_fs_fs/recovery.c (working copy)
> @@ -468,15 +468,15 @@ recover_body(void *baton, apr_pool_t *po
> /* 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)
> + cache, no matter whether sharing is currently enabled, taking care
> + not to create the cache if it does not exist. */
> + if (ffd->format >= SVN_FS_FS__MIN_REP_SHARING_FORMAT)

Looks good to me: that should fix both #4077 and #4214.

I would only suggest expanding the comment to explain why it's important that
the condition is what it is; for example:

  /* Prune younger-than-(newfound-youngest) revisions from the rep
     cache, taking care not to create the cache if it does not exist.

     We do this whenever rep-cache.db exists, whether it's currently enabled
     or not, to prevent a data loss that could result from having revisions
     created after this 'recover' operation referring to rep-cache.db rows
     that were created before the recover and that point to revisions younger-
     than-(newfound-youngest).
   */

(Possibly with some rewording to make it easier to read in three years when
we've all forgotten the context.)

Cheers,

Daniel

> {
> 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));
Received on 2018-08-23 13:34:39 CEST