Second iteration, after some comments in IRC from Greg Hudson and Erik
Huelsmann:
--------------------8-<-------cut-here---------8-<-----------------------
Fix issue #1601: do deltification in core code instead of in
post-commit hook, but (where possible) deltify after responding to the
client, to avoid user-visible delays.
Thanks to Greg Hudson, Sander Striker, Mike Pilato, Erik Huelsmann,
and Ben Collins-Sussman for their help.
* subversion/svnserve/serve.c
(server_baton_t): New boolean field 'daemon'.
(serve): Store new 'daemon' argument in baton.
(commit): Run deltification before or after sending the response,
depending on b->daemon.
* subversion/mod_dav_svn/version.c
(struct cleanup_deltify_baton): New baton type for below.
(cleanup_deltify): New pool cleanup function.
(register_deltification_cleanup): New function to register above.
(dav_svn_checkin, dav_svn_merge): Register deltification cleanup.
* subversion/libsvn_ra_local/ra_plugin.c
(struct deltify_etc_baton): New baton type.
(deltify_etc): New function, wraps original commit callback.
(svn_ra_local__get_commit_editor): Initialize a deltify_etc_baton,
use deltify_etc/baton in place of original callback/baton.
* subversion/libsvn_repos/repos.c
(create_hooks): Remove call to 'svnadmin deltify' from the
post-commit hook template.
Index: subversion/libsvn_ra_local/ra_plugin.c
===================================================================
--- subversion/libsvn_ra_local/ra_plugin.c (revision 7978)
+++ subversion/libsvn_ra_local/ra_plugin.c (working copy)
@@ -307,6 +307,48 @@
}
+struct deltify_etc_baton
+{
+ svn_fs_t *fs; /* the fs to deltify in */
+ apr_pool_t *pool; /* pool for scratch work */
+ svn_commit_callback_t callback; /* the original callback */
+ void *callback_baton; /* the original callback's baton */
+};
+
+/* This implements 'svn_commit_callback_t'. Its invokes the original
+ (wrapped) callback, but also does deltification on the new revision.
+ BATON is 'struct deltify_etc_baton *'. */
+static svn_error_t *
+deltify_etc (svn_revnum_t new_revision,
+ const char *date,
+ const char *author,
+ void *baton)
+{
+ struct deltify_etc_baton *db = baton;
+ svn_error_t *err1, *err2;
+
+ /* Invoke the original callback first, in case someone's waiting to
+ know the revision number so they can go off and annotate an
+ issue or something. */
+ err1 = (*db->callback) (new_revision, date, author, db->callback_baton);
+
+ /* But, deltification shouldn't be stopped just because someone's
+ random callback failed, so proceed unconditionally on to
+ deltification. */
+ err2 = svn_fs_deltify_revision (db->fs, new_revision, db->pool);
+
+ /* It's more interesting if the original callback failed, so let
+ that one dominate. */
+ if (err1)
+ {
+ svn_error_clear (err2);
+ return err1;
+ }
+
+ return err2;
+}
+
+
static svn_error_t *
svn_ra_local__get_commit_editor (void *session_baton,
const svn_delta_editor_t **editor,
@@ -317,12 +359,18 @@
apr_pool_t *pool)
{
svn_ra_local__session_baton_t *sess = session_baton;
-
+ struct deltify_etc_baton *db = apr_palloc (pool, sizeof(*db));
+
+ db->fs = sess->fs;
+ db->pool = pool;
+ db->callback = callback;
+ db->callback_baton = callback_baton;
+
/* Get the repos commit-editor */
SVN_ERR (svn_repos_get_commit_editor (editor, edit_baton, sess->repos,
sess->repos_url, sess->fs_path,
sess->username, log_msg,
- callback, callback_baton, pool));
+ deltify_etc, db, pool));
return SVN_NO_ERROR;
}
Index: subversion/mod_dav_svn/version.c
===================================================================
--- subversion/mod_dav_svn/version.c (revision 7978)
+++ subversion/mod_dav_svn/version.c (working copy)
@@ -19,6 +19,7 @@
#include <httpd.h>
+#include <http_log.h>
#include <mod_dav.h>
#include <apr_tables.h>
#include <apr_uuid.h>
@@ -587,6 +588,90 @@
return dav_svn_working_to_regular_resource(resource);
}
+
+/* Closure object for cleanup_deltify. */
+struct cleanup_deltify_baton
+{
+ /* The repository in which to deltify. We use a path instead of an
+ object, because it's difficult to obtain a repos or fs object
+ with the right lifetime guarantees. */
+ const char *repos_path;
+
+ /* The revision number against which to deltify. */
+ svn_revnum_t revision;
+
+ /* The pool to use for all temporary allocation while working. This
+ may or may not be the same as the pool on which the cleanup is
+ registered, but obviously it must have a lifetime at least as
+ long as that pool. */
+ apr_pool_t *pool;
+};
+
+
+/* APR pool cleanup function to deltify against a just-committed
+ revision. DATA is a 'struct cleanup_deltify_baton *'.
+
+ If any errors occur, log them in the httpd server error log, but
+ return APR_SUCCESS no matter what, as this is a pool cleanup
+ function and deltification is not a matter of correctness
+ anyway. */
+static apr_status_t cleanup_deltify(void *data)
+{
+ struct cleanup_deltify_baton *cdb = data;
+ svn_repos_t *repos;
+ svn_error_t *err;
+
+ /* Yes, opening a repository registers a cleanup on the pool from
+ which we are already running as a cleanup. But these days that's
+ safe. A big +smooch+ to Sander Striker -- I'm not sure it was
+ he who implemented this, but he told me about it anyway :-). */
+ err = svn_repos_open(&repos, cdb->repos_path, cdb->pool);
+ if (err)
+ {
+ ap_log_perror(APLOG_MARK, APLOG_ERR, err->apr_err, cdb->pool,
+ "cleanup_deltify: error opening repository '%s'",
+ cdb->repos_path);
+ svn_error_clear(err);
+ return APR_SUCCESS;
+ }
+
+ err = svn_fs_deltify_revision(svn_repos_fs(repos),
+ cdb->revision, cdb->pool);
+ if (err)
+ {
+ ap_log_perror(APLOG_MARK, APLOG_ERR, err->apr_err, cdb->pool,
+ "cleanup_deltify: error deltifying against revision %"
+ SVN_REVNUM_T_FMT " in repository '%s'",
+ cdb->revision, cdb->repos_path);
+ svn_error_clear(err);
+ }
+
+ return APR_SUCCESS;
+}
+
+
+/* Register the cleanup_deltify function on POOL, which should be the
+ connection pool for the request. This way the time needed for
+ deltification won't delay the response to the client.
+
+ REPOS is the repository in which deltify, and REVISION is the
+ revision against which to deltify. POOL is both the pool on which
+ to register the cleanup function and the pool that will be used for
+ temporary allocations while deltifying. */
+static void register_deltification_cleanup(svn_repos_t *repos,
+ svn_revnum_t revision,
+ apr_pool_t *pool)
+{
+ struct cleanup_deltify_baton *cdb = apr_palloc(pool, sizeof(*cdb));
+
+ cdb->repos_path = svn_repos_path(repos, pool);
+ cdb->revision = revision;
+ cdb->pool = pool;
+
+ apr_pool_cleanup_register(pool, cdb, cleanup_deltify, apr_pool_cleanup_null);
+}
+
+
dav_error *dav_svn_checkin(dav_resource *resource,
int keep_checked_out,
dav_resource **version_resource)
@@ -654,7 +739,10 @@
return dav_svn_convert_err(serr, HTTP_CONFLICT, msg);
}
- /* Commit was successful. */
+ /* Commit was successful, so schedule deltification. */
+ register_deltification_cleanup(resource->info->repos->repos,
+ new_rev,
+ resource->info->r->connection->pool);
/* If caller wants it, return the new VR that was created by
the checkin. */
@@ -901,6 +989,10 @@
return dav_svn_convert_err(serr, HTTP_CONFLICT, msg);
}
+ /* Commit was successful, so schedule deltification. */
+ register_deltification_cleanup(source->info->repos->repos, new_rev,
+ source->info->r->connection->pool);
+
/* Check the dav_resource->info area for information about the
special X-SVN-Options: header that may have come in the http
request. If the header contains "no-merge-response", then pass
Index: subversion/libsvn_repos/repos.c
===================================================================
--- subversion/libsvn_repos/repos.c (revision 7978)
+++ subversion/libsvn_repos/repos.c (working copy)
@@ -637,11 +637,6 @@
"REV=\"$2\""
APR_EOL_STR
APR_EOL_STR
- "# Deltify predecessors of things changed in this revision."
- APR_EOL_STR
- "nice -2 svnadmin deltify \"$REPOS\" -r \"$REV\" &"
- APR_EOL_STR
- APR_EOL_STR
"commit-email.pl \"$REPOS\" \"$REV\" commit-watchers@example.org"
APR_EOL_STR
"log-commit.py --repository \"$REPOS\" --revision \"$REV\""
Index: subversion/svnserve/serve.c
===================================================================
--- subversion/svnserve/serve.c (revision 7978)
+++ subversion/svnserve/serve.c (working copy)
@@ -547,8 +547,20 @@
if (!aborted)
{
SVN_ERR(trivial_auth_request(conn, pool, b));
+
+ /* In tunnel mode, deltify before answering the client, because
+ answering may cause the client to terminate the connection
+ and thus kill the server. But otherwise, deltify after
+ answering the client, to avoid user-visible delay. */
+
+ if (b->tunnel)
+ SVN_ERR(svn_fs_deltify_revision(b->fs, new_rev, pool));
+
SVN_ERR(svn_ra_svn_write_tuple(conn, pool, "r(?c)(?c)",
new_rev, date, author));
+
+ if (! b->tunnel)
+ SVN_ERR(svn_fs_deltify_revision(b->fs, new_rev, pool));
}
return SVN_NO_ERROR;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Dec 12 00:21:47 2003