On Wed, Jun 19, 2013 at 3:30 PM, Philip Martin
<philip.martin_at_wandisco.com>wrote:
> 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.
>
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.
Received on 2013-06-21 22:01:22 CEST