Re: svn commit: r1494298 - /subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c

stefan2_at_apache.org writes:

> Author: stefan2
> Date: Tue Jun 18 20:59:07 2013
> New Revision: 1494298

> static svn_error_t *
> @@ -1501,6 +1507,29 @@ upgrade_pack_revprops(svn_fs_t *fs,
> svn_pool_clear(iterpool);
> }

Is there any reason for upgrade_pack_revprops and
upgrade_cleanup_pack_revprops to clear the pool at the end of the loop
rather than the standard pattern at the start of the loop?

> /* Bump the format file. */
> - return write_format(format_path, SVN_FS_FS__FORMAT_NUMBER, max_files_per_dir,
> - TRUE, pool);
> + SVN_ERR(write_format(format_path, SVN_FS_FS__FORMAT_NUMBER,
> + max_files_per_dir, TRUE, pool));
> +
> + /* Now, it is safe to remove the redundant revprop files. */
> + if (needs_revprop_shard_cleanup)
> + SVN_ERR(upgrade_pack_revprops(fs, pool));

That's corrected to upgrade_cleanup_pack_revprops in a later revision.

This whole change is to handle a rerun after an interrupted upgrade but
suppose the rerun is itself interupted between write_format and
upgrade_cleanup_pack_revprops. Is the user reduced to manually removing
the revprop files? Neither upgrade nor pack appear to do it.

Perhaps pack should cleanup old revprop files?

-- 
Philip Martin | Subversion Committer
WANdisco | Non-Stop Data
www.wandisco.com