On Wed, Jun 19, 2013 at 3:30 PM, Philip Martin <philip.mar...@wandisco.com>wrote:
> stef...@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. > I consider them "leftovers" like abandoned transactions. But I agree that we should do something about them. Perhaps pack should cleanup old revprop files? > Good idea. Done in r1495566. Not foolproof but covers 99% percent of the already fringy "I cancel upgrade' case. -- Stefan^2.
