You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by cm...@apache.org on 2012/12/17 22:33:17 UTC

svn commit: r1423162 - in /subversion/trunk/subversion: libsvn_ra_serf/update.c svnrdump/svnrdump.c

Author: cmpilato
Date: Mon Dec 17 21:33:16 2012
New Revision: 1423162

URL: http://svn.apache.org/viewvc?rev=1423162&view=rev
Log:
For issue Issue #4116 ("svnrdump doesn't work with serf"):
Workaround the fact that svnrdump doesn't like ra_serf's editor v1
non-compliance by having svnrdump use the runtime configuration
structure to tell ra_serf to use an approach that *does* work.

NOTE: There remains some brokenness here when the server disallows
bulk updates.  Still debugging that.

* subversion/libsvn_ra_serf/update.c
  (get_best_connection): Rework this function a bit for clarity, but
    also to special case the 'http-max-connections=2' scenario.  In
    that scenario, we will not recycle the first connection (the one on
    which the REPORT was sent) when it becomes available.

* subversion/svnrdump/svnrdump.c
  (init_client_context): Override the runtime configuration to force
    ra_serf to use (effectively) 'http-max-connections=2' and
    'bulk-updates=true'.

Modified:
    subversion/trunk/subversion/libsvn_ra_serf/update.c
    subversion/trunk/subversion/svnrdump/svnrdump.c

Modified: subversion/trunk/subversion/libsvn_ra_serf/update.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/update.c?rev=1423162&r1=1423161&r2=1423162&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/update.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/update.c Mon Dec 17 21:33:16 2012
@@ -503,26 +503,42 @@ update_cdata(svn_ra_serf__xml_estate_t *
 static svn_ra_serf__connection_t *
 get_best_connection(report_context_t *ctx)
 {
-  svn_ra_serf__connection_t * conn;
-  int first_conn;
+  svn_ra_serf__connection_t *conn;
+  int first_conn = 1;
 
   /* Skip the first connection if the REPORT response hasn't been completely
-     received yet. */
-  first_conn = ctx->report_received ? 0: 1;
-
+     received yet or if we're being told to limit our connections to
+     2 (because this could be an attempt to ensure that we do all our
+     auxiliary GETs/PROPFINDs on a single connection).
+
+     ### FIXME: This latter requirement (max_connections > 2) is
+     ### really just a hack to work around the fact that some update
+     ### editor implementations (such as svnrdump's dump editor)
+     ### simply can't handle the way ra_serf violates the editor v1
+     ### drive ordering requirements.
+     ### 
+     ### See http://subversion.tigris.org/issues/show_bug.cgi?id=4116.
+  */
+  if (ctx->report_received && (ctx->sess->max_connections > 2))
+    first_conn = 0;
+
+  /* Currently, we just cycle connections.  In the future we could
+     store the number of pending requests on each connection, or
+     perform other heuristics, to achieve better connection usage.
+     (As an optimization, if there's only one available auxiliary
+     connection to use, don't bother doing all the cur_conn math --
+     just return that one connection.)  */
   if (ctx->sess->num_conns - first_conn == 1)
-    return ctx->sess->conns[first_conn];
-
-  /* Currently just cycle connections. In future we could store number of
-   * pending requests on each connection for better connection usage. */
-  conn = ctx->sess->conns[ctx->sess->cur_conn];
-
-  /* Switch our connection. */
-  ctx->sess->cur_conn++;
-
-  if (ctx->sess->cur_conn >= ctx->sess->num_conns)
-      ctx->sess->cur_conn = first_conn;
-
+    {
+      conn = ctx->sess->conns[first_conn];
+    }
+  else
+    {
+      conn = ctx->sess->conns[ctx->sess->cur_conn];
+      ctx->sess->cur_conn++;
+      if (ctx->sess->cur_conn >= ctx->sess->num_conns)
+        ctx->sess->cur_conn = first_conn;
+    }
   return conn;
 }
 

Modified: subversion/trunk/subversion/svnrdump/svnrdump.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnrdump/svnrdump.c?rev=1423162&r1=1423161&r2=1423162&view=diff
==============================================================================
--- subversion/trunk/subversion/svnrdump/svnrdump.c (original)
+++ subversion/trunk/subversion/svnrdump/svnrdump.c Mon Dec 17 21:33:16 2012
@@ -23,6 +23,7 @@
  */
 
 #include <apr_signal.h>
+#include <apr_uri.h>
 
 #include "svn_pools.h"
 #include "svn_cmdline.h"
@@ -328,6 +329,8 @@ replay_revend_v2(svn_revnum_t revision,
  * allocated from POOL.  Use CONFIG_DIR and pass USERNAME, PASSWORD,
  * CONFIG_DIR and NO_AUTH_CACHE to initialize the authorization baton.
  * CONFIG_OPTIONS (if not NULL) is a list of configuration overrides.
+ * REPOS_URL is used to fiddle with server-specific configuration
+ * options.
  */
 static svn_error_t *
 init_client_context(svn_client_ctx_t **ctx_p,
@@ -335,13 +338,14 @@ init_client_context(svn_client_ctx_t **c
                     const char *username,
                     const char *password,
                     const char *config_dir,
+                    const char *repos_url,
                     svn_boolean_t no_auth_cache,
                     svn_boolean_t trust_server_cert,
                     apr_array_header_t *config_options,
                     apr_pool_t *pool)
 {
   svn_client_ctx_t *ctx = NULL;
-  svn_config_t *cfg_config;
+  svn_config_t *cfg_config, *cfg_servers;
 
   SVN_ERR(svn_ra_initialize(pool));
 
@@ -357,6 +361,47 @@ init_client_context(svn_client_ctx_t **c
   cfg_config = apr_hash_get(ctx->config, SVN_CONFIG_CATEGORY_CONFIG,
                             APR_HASH_KEY_STRING);
 
+  /* ### FIXME: This is a hack to work around the fact that our dump
+     ### editor simply can't handle the way ra_serf violates the
+     ### editor v1 drive ordering requirements.
+     ###
+     ### We'll override both the global value and server-specific one
+     ### for the 'http-bulk-updates' and 'http-max-connections'
+     ### options in order to get ra_serf to try a bulk-update if the
+     ### server will allow it, or at least try to limit all its
+     ### auxiliary GETs/PROPFINDs to happening (well-ordered) on a
+     ### single server connection.
+     ### 
+     ### See http://subversion.tigris.org/issues/show_bug.cgi?id=4116.
+  */
+  cfg_servers = apr_hash_get(ctx->config, SVN_CONFIG_CATEGORY_SERVERS,
+                             APR_HASH_KEY_STRING);
+  svn_config_set_bool(cfg_servers, SVN_CONFIG_SECTION_GLOBAL,
+                      SVN_CONFIG_OPTION_HTTP_BULK_UPDATES, TRUE);
+  svn_config_set_int64(cfg_servers, SVN_CONFIG_SECTION_GLOBAL,
+                       SVN_CONFIG_OPTION_HTTP_MAX_CONNECTIONS, 2);
+  if (cfg_servers)
+    {
+      apr_status_t status;
+      apr_uri_t parsed_url;
+
+      status = apr_uri_parse(pool, repos_url, &parsed_url);
+      if (! status)
+        {
+          const char *server_group;
+
+          server_group = svn_config_find_group(cfg_servers, parsed_url.hostname,
+                                               SVN_CONFIG_SECTION_GROUPS, pool);
+          if (server_group)
+            {
+              svn_config_set_bool(cfg_servers, server_group,
+                                  SVN_CONFIG_OPTION_HTTP_BULK_UPDATES, TRUE);
+              svn_config_set_int64(cfg_servers, server_group,
+                                   SVN_CONFIG_OPTION_HTTP_MAX_CONNECTIONS, 2);
+            }
+        }
+    }
+
   /* Set up our cancellation support. */
   ctx->cancel_func = check_cancel;
 
@@ -1064,6 +1109,7 @@ main(int argc, const char **argv)
                                    username,
                                    password,
                                    config_dir,
+                                   opt_baton->url,
                                    no_auth_cache,
                                    trust_server_cert,
                                    config_options,