You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by David Glasser <gl...@davidglasser.net> on 2008/01/31 23:28:24 UTC
Re: svn commit: r29087 - in branches/svnadmin-upgrade/subversion: include libsvn_fs libsvn_fs_base libsvn_fs_fs libsvn_repos svnadmin
On Jan 30, 2008 1:22 PM, <cm...@tigris.org> wrote:
> Author: cmpilato
> Date: Wed Jan 30 13:22:11 2008
> New Revision: 29087
>
> Log:
> On the 'svnadmin-upgrade' branch, take a first pass at implementing
> the 'svnadmin upgrade' subcommand and supporting APIs.
Forgive me if these concerns have been fixed in future revisions.
> Modified: branches/svnadmin-upgrade/subversion/svnadmin/main.c
> URL: http://svn.collab.net/viewvc/svn/branches/svnadmin-upgrade/subversion/svnadmin/main.c?pathrev=29087&r1=29086&r2=29087
> ==============================================================================
> --- branches/svnadmin-upgrade/subversion/svnadmin/main.c (original)
> +++ branches/svnadmin-upgrade/subversion/svnadmin/main.c Wed Jan 30 13:22:11 2008
> @@ -197,6 +197,7 @@
> subcommand_setlog,
> subcommand_setrevprop,
> subcommand_setuuid,
> + subcommand_upgrade,
> subcommand_verify;
>
> enum
> @@ -433,6 +434,19 @@
> "generate a brand new UUID for the repository.\n"),
> {0} },
>
> + {"upgrade", subcommand_upgrade, {0}, N_
> + ("usage: svnadmin upgrade REPOS_PATH\n\n"
> + "Upgrade the repository located at REPOS_PATH to the latest supported\n"
> + "schema version.\n\n"
> + "This functionality is provided as a convenience for repository\n"
> + "administrators who wish to make use of new Subversion functionality\n"
> + "without having to undertake a potentially costly full repository dump\n"
> + "and load operation. As such, the upgrade performs only the minimum\n"
> + "amount of work needed to accomplish this while still maintaining the\n"
> + "integrity of the repository. It does not guarantee the most optimized\n"
> + "repository state as a dump and subsequent load would.\n"),
> + {0} },
> +
> {"verify", subcommand_verify, {0}, N_
> ("usage: svnadmin verify REPOS_PATH\n\n"
> "Verifies the data stored in the repository.\n"),
> @@ -1273,6 +1287,76 @@
> }
>
>
> +/* A callback which is called when the upgrade starts. */
> +static svn_error_t *
> +upgrade_started(void *baton)
> +{
> + apr_pool_t *pool = (apr_pool_t *)baton;
> +
> + SVN_ERR(svn_cmdline_printf(pool,
> + _("Repository lock acquired.\n"
> + "Please wait; upgrading the"
> + " repository may take some time...\n")));
> + SVN_ERR(svn_cmdline_fflush(stdout));
> +
> + /* Enable cancellation signal handlers. */
> + setup_cancellation_signals(signal_handler);
> +
> + return SVN_NO_ERROR;
> +}
I believe this callback is being called in the repos function, not the
FS function, so when it is called it is simply not true for FSFS: the
lock has not been acquired yet. (Yeah, it's a pain that the BDB lock
is acquired in repos code but not the FSFS lock...)
> +
> +
> +/* This implements `svn_opt_subcommand_t'. */
> +static svn_error_t *
> +subcommand_upgrade(apr_getopt_t *os, void *baton, apr_pool_t *pool)
> +{
> + svn_error_t *err;
> + struct svnadmin_opt_state *opt_state = baton;
> +
> + /* Restore default signal handlers. */
> + setup_cancellation_signals(SIG_DFL);
> +
> + err = svn_repos_upgrade(opt_state->repository_path, TRUE,
> + upgrade_started, pool, pool);
> + if (err)
> + {
> + if (APR_STATUS_IS_EAGAIN(err->apr_err))
> + {
> + svn_error_clear(err);
> + err = SVN_NO_ERROR;
> + if (! opt_state->wait)
> + return svn_error_create(SVN_ERR_REPOS_LOCKED, NULL,
> + _("Failed to get exclusive repository "
> + "access; perhaps another process\n"
> + "such as httpd, svnserve or svn "
> + "has it open?"));
> + SVN_ERR(svn_cmdline_printf(pool,
> + _("Waiting on repository lock; perhaps"
> + " another process has it open?\n")));
> + SVN_ERR(svn_cmdline_fflush(stdout));
> + SVN_ERR(svn_repos_upgrade(opt_state->repository_path, FALSE,
> + upgrade_started, pool, pool));
> + }
> + else if (err->apr_err == SVN_ERR_REPOS_UNSUPPORTED_UPGRADE)
This should probably be the FS error?
--dave
> + {
> + return svn_error_quick_wrap
> + (err, _("Upgrade of this repository's underlying versioned "
> + "filesystem is not supported; consider "
> + "dumping and loading the data elsewhere"));
> + }
> + else if (err->apr_err == SVN_ERR_REPOS_UNSUPPORTED_UPGRADE)
> + {
> + return svn_error_quick_wrap
> + (err, _("Upgrade of this repository is not supported; consider "
> + "dumping and loading the data elsewhere"));
> + }
> + }
> + SVN_ERR(err);
> +
> + SVN_ERR(svn_cmdline_printf(pool, _("\nUpgrade completed.\n")));
> + return SVN_NO_ERROR;
> +}
> +
>
>
> /** Main. **/
>
--
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r29087 - in branches/svnadmin-upgrade/subversion: include libsvn_fs libsvn_fs_base libsvn_fs_fs libsvn_repos svnadmin
Posted by David Glasser <gl...@davidglasser.net>.
On Jan 31, 2008 7:00 PM, C. Michael Pilato <cm...@collab.net> wrote:
> (I find it both surprising and utterly annoying that we apparently can't
> serialize access to FSFS repositories, only FSFS filesystems inside
> repositories. What night of drunken revelry inspired that?)
This is a feature. Reads in FSFS require no lock-checking, ever.
Having to put the lock check at the repos layer would make that goal a
lot more difficult.
--dave
--
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r29087 - in branches/svnadmin-upgrade/subversion:
include libsvn_fs libsvn_fs_base libsvn_fs_fs libsvn_repos svnadmin
Posted by "C. Michael Pilato" <cm...@collab.net>.
David Glasser wrote:
>> +/* A callback which is called when the upgrade starts. */
>> +static svn_error_t *
>> +upgrade_started(void *baton)
>> +{
>> + apr_pool_t *pool = (apr_pool_t *)baton;
>> +
>> + SVN_ERR(svn_cmdline_printf(pool,
>> + _("Repository lock acquired.\n"
>> + "Please wait; upgrading the"
>> + " repository may take some time...\n")));
>> + SVN_ERR(svn_cmdline_fflush(stdout));
>> +
>> + /* Enable cancellation signal handlers. */
>> + setup_cancellation_signals(signal_handler);
>> +
>> + return SVN_NO_ERROR;
>> +}
>
> I believe this callback is being called in the repos function, not the
> FS function, so when it is called it is simply not true for FSFS: the
> lock has not been acquired yet. (Yeah, it's a pain that the BDB lock
> is acquired in repos code but not the FSFS lock...)
This code was copied wholesale from the implementation of 'svnadmin
recover', so we'll need to fix that, too.
>> + SVN_ERR(svn_cmdline_fflush(stdout));
>> + SVN_ERR(svn_repos_upgrade(opt_state->repository_path, FALSE,
>> + upgrade_started, pool, pool));
>> + }
>> + else if (err->apr_err == SVN_ERR_REPOS_UNSUPPORTED_UPGRADE)
>
> This should probably be the FS error?
Oops! Yep. I'll make that quick fix. Not sure what to do about the other.
(I find it both surprising and utterly annoying that we apparently can't
serialize access to FSFS repositories, only FSFS filesystems inside
repositories. What night of drunken revelry inspired that?)
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand