You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2013/12/18 16:59:39 UTC
svn commit: r1551993 - in /subversion/trunk/subversion/libsvn_ra_serf:
lock.c ra_serf.h util.c
Author: rhuijben
Date: Wed Dec 18 15:59:39 2013
New Revision: 1551993
URL: http://svn.apache.org/r1551993
Log:
Make ra serf pipeline lock and unlock requests. Currently it does
this on a single connection, so performance can probably
be improved further.
* subversion/libsvn_ra_serf/lock.c
(includes): Add assert.h and svn_sorts.h
(lock_ctx_t: Add unlock members.
(determine_error): Remove function.
(run_locks): New function.
(handle_lock): Always parse headers. Handle error 405.
(svn_ra_serf__lock): Create all requests and then call run_locks.
Set no_fail_on_http_failure_status.
(set_unlock_headers): Use other baton type.
(svn_ra_serf__unlock): Create all requests and then call run_locks.
Set no_fail_on_http_failure_status.
* subversion/libsvn_ra_serf/ra_serf.h
(svn_ra_serf__handler_t): Add boolean.
* subversion/libsvn_ra_serf/util.c
(handle_response): Replace specialized lock handling with a boolean that
allows response handlers to handle all errors themselves.
Modified:
subversion/trunk/subversion/libsvn_ra_serf/lock.c
subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
subversion/trunk/subversion/libsvn_ra_serf/util.c
Modified: subversion/trunk/subversion/libsvn_ra_serf/lock.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/lock.c?rev=1551993&r1=1551992&r2=1551993&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/lock.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/lock.c Wed Dec 18 15:59:39 2013
@@ -25,6 +25,7 @@
#include <apr_uri.h>
#include <serf.h>
+#include <assert.h>
#include "svn_dav.h"
#include "svn_hash.h"
@@ -34,6 +35,7 @@
#include "../libsvn_ra/ra_loader.h"
#include "svn_config.h"
#include "svn_path.h"
+#include "svn_sorts.h"
#include "svn_time.h"
#include "svn_private_config.h"
@@ -63,7 +65,8 @@ typedef struct lock_ctx_t {
const char *path;
- svn_lock_t *lock;
+ const char *token; /* For unlock */
+ svn_lock_t *lock; /* For lock */
svn_boolean_t force;
svn_revnum_t revision;
@@ -195,35 +198,195 @@ set_lock_headers(serf_bucket_t *headers,
return APR_SUCCESS;
}
-
-/* Register an error within the session. If something is already there,
- then it will take precedence. */
+/* Helper function for svn_ra_serf__lock and svn_ra_serf__unlock */
static svn_error_t *
-determine_error(svn_ra_serf__handler_t *handler,
- svn_error_t *err)
+run_locks(svn_ra_serf__session_t *sess,
+ apr_array_header_t *lock_ctxs,
+ svn_boolean_t locking,
+ svn_ra_lock_callback_t lock_func,
+ void *lock_baton,
+ apr_pool_t *scratch_pool)
{
- apr_status_t errcode;
+ apr_pool_t *iterpool;
+ apr_interval_time_t waittime_left = sess->timeout;
- if (err)
- return err;
+ assert(sess->pending_error == SVN_NO_ERROR);
- if (handler->sline.code == 200 || handler->sline.code == 207)
- return SVN_NO_ERROR;
- else if (handler->sline.code == 423)
- errcode = SVN_ERR_FS_PATH_ALREADY_LOCKED;
- else if (handler->sline.code == 403)
- errcode = SVN_ERR_RA_DAV_FORBIDDEN;
- else
- errcode = SVN_ERR_RA_DAV_REQUEST_FAILED;
-
- /* The server did not send us a detailed human-readable error.
- Provide a generic error. */
- return svn_error_createf(errcode, NULL,
- _("Lock request failed: %d %s"),
- handler->sline.code,
- handler->sline.reason ? handler->sline.reason : "");
-}
+ iterpool = svn_pool_create(scratch_pool);
+ while (lock_ctxs->nelts)
+ {
+ apr_status_t status;
+ svn_error_t *err;
+ int i;
+
+ svn_pool_clear(iterpool);
+
+ if (sess->cancel_func)
+ SVN_ERR((*sess->cancel_func)(sess->cancel_baton));
+
+ status = serf_context_run(sess->context,
+ SVN_RA_SERF__CONTEXT_RUN_DURATION,
+ iterpool);
+
+ err = sess->pending_error;
+ sess->pending_error = SVN_NO_ERROR;
+
+ /* If the context duration timeout is up, we'll subtract that
+ duration from the total time alloted for such things. If
+ there's no time left, we fail with a message indicating that
+ the connection timed out. */
+ if (APR_STATUS_IS_TIMEUP(status))
+ {
+ status = 0;
+ if (sess->timeout)
+ {
+ if (waittime_left > SVN_RA_SERF__CONTEXT_RUN_DURATION)
+ {
+ waittime_left -= SVN_RA_SERF__CONTEXT_RUN_DURATION;
+ }
+ else
+ {
+ return
+ svn_error_compose_create(
+ err,
+ svn_error_create(SVN_ERR_RA_DAV_CONN_TIMEOUT, NULL,
+ _("Connection timed out")));
+ }
+ }
+ }
+ else
+ {
+ waittime_left = sess->timeout;
+ }
+
+ SVN_ERR(err);
+ if (status)
+ {
+ /* ### This omits SVN_WARNING, and possibly relies on the fact that
+ ### MAX(SERF_ERROR_*) < SVN_ERR_BAD_CATEGORY_START? */
+ if (status >= SVN_ERR_BAD_CATEGORY_START && status < SVN_ERR_LAST)
+ {
+ /* apr can't translate subversion errors to text */
+ SVN_ERR_W(svn_error_create(status, NULL, NULL),
+ _("Error running context"));
+ }
+
+ return svn_ra_serf__wrap_err(status, _("Error running context"));
+ }
+
+ for (i = 0; i < lock_ctxs->nelts; i++)
+ {
+ lock_ctx_t *ctx = APR_ARRAY_IDX(lock_ctxs, i, lock_ctx_t *);
+
+ if (ctx->handler->done)
+ {
+ svn_error_t *err;
+ svn_error_t *server_err = NULL;
+ svn_error_t *cb_err = NULL;
+
+ if (ctx->handler->server_error)
+ server_err = ctx->handler->server_error->error;
+
+ /* Api users expect specific error code to detect failures,
+ pass the rest to svn_ra_serf__error_on_status */
+ switch (ctx->handler->sline.code)
+ {
+ case 400:
+ err = svn_error_createf(SVN_ERR_FS_NO_SUCH_LOCK, NULL,
+ _("No lock on path '%s' (%d %s)"),
+ ctx->path,
+ ctx->handler->sline.code,
+ ctx->handler->sline.reason);
+ break;
+ case 403:
+ err = svn_error_createf(SVN_ERR_FS_LOCK_OWNER_MISMATCH,
+ NULL,
+ _("Unlock of '%s' failed (%d %s)"),
+ ctx->path,
+ ctx->handler->sline.code,
+ ctx->handler->sline.reason);
+ break;
+ case 405:
+ err = svn_error_createf(SVN_ERR_FS_OUT_OF_DATE,
+ NULL,
+ _("Path '%s' doesn't exist in "
+ "HEAD revision (%d %s)"),
+ ctx->path,
+ ctx->handler->sline.code,
+ ctx->handler->sline.reason);
+ break;
+ case 423:
+ err = svn_error_createf(SVN_ERR_FS_PATH_ALREADY_LOCKED,
+ NULL,
+ _("Path '%s' already locked "
+ "(%d %s)"),
+ ctx->path,
+ ctx->handler->sline.code,
+ ctx->handler->sline.reason);
+ break;
+ case 500:
+ if (server_err)
+ {
+ /* Handle out of date, etc by just passing the server
+ error */
+ err = NULL;
+ }
+ else
+ {
+ /* ### The test suite expects us to return instead
+ of report an error here */
+ return svn_error_trace(
+ svn_error_createf(
+ SVN_ERR_RA_DAV_REQUEST_FAILED,
+ NULL,
+ _("%s request on '%s' failed: %d %s"),
+ ctx->handler->method,
+ ctx->path,
+ ctx->handler->sline.code,
+ ctx->handler->sline.reason));
+ }
+ break;
+ default:
+ err = svn_ra_serf__error_on_status(ctx->handler->sline,
+ ctx->path,
+ ctx->handler->location);
+ }
+
+ if (server_err && err
+ && server_err->apr_err == err->apr_err)
+ err = svn_error_compose_create(server_err, err);
+ else
+ err = svn_error_compose_create(err, server_err);
+
+ if (lock_func)
+ {
+ svn_lock_t *report_lock = NULL;
+
+ if (locking)
+ report_lock = ctx->lock;
+
+
+ cb_err = lock_func(lock_baton, ctx->path, locking,
+ report_lock, err, ctx->pool);
+ }
+ svn_error_clear(err);
+
+ SVN_ERR(cb_err);
+
+ waittime_left = sess->timeout;
+ svn_sort__array_delete(lock_ctxs, i, 1);
+ i--;
+
+ svn_pool_destroy(ctx->pool);
+ continue;
+ }
+ }
+ }
+ svn_pool_destroy(iterpool);
+
+ return SVN_NO_ERROR;
+}
/* Implements svn_ra_serf__response_handler_t */
static svn_error_t *
@@ -234,18 +397,6 @@ handle_lock(serf_request_t *request,
{
lock_ctx_t *ctx = handler_baton;
- /* 403 (Forbidden) when a lock doesn't exist.
- 423 (Locked) when a lock already exists. */
- if (ctx->handler->sline.code == 403
- || ctx->handler->sline.code == 423)
- {
- /* Go look in the body for a server-provided error. This will
- reset flags for the core handler to Do The Right Thing. We
- won't be back to this handler again. */
- return svn_error_trace(svn_ra_serf__expect_empty_body(
- request, response, ctx->handler, pool));
- }
-
if (!ctx->read_headers)
{
serf_bucket_t *headers;
@@ -269,6 +420,20 @@ handle_lock(serf_request_t *request,
ctx->read_headers = TRUE;
}
+ /* 403 (Forbidden) when a lock doesn't exist.
+ 423 (Locked) when a lock already exists.
+ 405 (Method not allowed) when a node does not exist in HEAD */
+ if (ctx->handler->sline.code == 403
+ || ctx->handler->sline.code == 423
+ || ctx->handler->sline.code == 405)
+ {
+ /* Go look in the body for a server-provided error. This will
+ reset flags for the core handler to Do The Right Thing. We
+ won't be back to this handler again. */
+ return svn_error_trace(svn_ra_serf__expect_empty_body(
+ request, response, ctx->handler, pool));
+ }
+
return ctx->inner_handler(request, response, ctx->inner_baton, pool);
}
@@ -321,13 +486,14 @@ svn_ra_serf__lock(svn_ra_session_t *ra_s
svn_ra_serf__session_t *session = ra_session->priv;
apr_hash_index_t *hi;
apr_pool_t *iterpool;
+ apr_array_header_t *lock_requests;
- iterpool = svn_pool_create(scratch_pool);
+ lock_requests = apr_array_make(scratch_pool, apr_hash_count(path_revs),
+ sizeof(lock_ctx_t*));
- /* ### TODO for issue 2263: Send all the locks over the wire at once. This
- ### loop is just a temporary shim.
- ### an alternative, which is backwards-compat with all servers is to
- ### pipeline these requests. ie. stop using run_wait/run_one. */
+ /* ### Perhaps we should open more connections than just one? See update.c */
+
+ iterpool = svn_pool_create(scratch_pool);
for (hi = apr_hash_first(scratch_pool, path_revs);
hi;
@@ -337,34 +503,41 @@ svn_ra_serf__lock(svn_ra_session_t *ra_s
svn_ra_serf__xml_context_t *xmlctx;
const char *req_url;
lock_ctx_t *lock_ctx;
- svn_error_t *err;
- svn_error_t *new_err = NULL;
+ apr_pool_t *lock_pool;
svn_pool_clear(iterpool);
- lock_ctx = apr_pcalloc(iterpool, sizeof(*lock_ctx));
+ lock_pool = svn_pool_create(scratch_pool);
+ lock_ctx = apr_pcalloc(scratch_pool, sizeof(*lock_ctx));
- lock_ctx->pool = iterpool;
+ lock_ctx->pool = lock_pool;
lock_ctx->path = svn__apr_hash_index_key(hi);
lock_ctx->revision = *((svn_revnum_t*)svn__apr_hash_index_val(hi));
- lock_ctx->lock = svn_lock_create(iterpool);
+ lock_ctx->lock = svn_lock_create(lock_pool);
lock_ctx->lock->path = lock_ctx->path;
lock_ctx->lock->comment = comment;
lock_ctx->force = force;
req_url = svn_path_url_add_component2(session->session_url.path,
- lock_ctx->path, iterpool);
+ lock_ctx->path, lock_pool);
xmlctx = svn_ra_serf__xml_context_create(locks_ttable,
NULL, locks_closed, NULL, NULL,
lock_ctx,
- iterpool);
- handler = svn_ra_serf__create_expat_handler(xmlctx, NULL, iterpool);
+ lock_pool);
+ handler = svn_ra_serf__create_expat_handler(xmlctx, NULL, lock_pool);
handler->method = "LOCK";
handler->path = req_url;
handler->body_type = "text/xml";
- handler->conn = session->conns[0];
+
+ /* Same stupid algorithm from get_best_connection() in update.c */
+ handler->conn = session->conns[session->cur_conn];
+ session->cur_conn++;
+
+ if (session->cur_conn >= session->num_conns)
+ session->cur_conn = 0;
+
handler->session = session;
handler->header_delegate = set_lock_headers;
@@ -378,35 +551,29 @@ svn_ra_serf__lock(svn_ra_session_t *ra_s
handler->response_handler = handle_lock;
handler->response_baton = lock_ctx;
- lock_ctx->handler = handler;
+ handler->no_fail_on_http_failure_status = TRUE;
- err = svn_ra_serf__context_run_one(handler, iterpool);
- err = svn_error_trace(determine_error(handler, err));
+ lock_ctx->handler = handler;
- if (lock_func)
- new_err = lock_func(lock_baton, lock_ctx->path, TRUE, lock_ctx->lock,
- err, iterpool);
- svn_error_clear(err);
+ APR_ARRAY_PUSH(lock_requests, lock_ctx_t *) = lock_ctx;
- SVN_ERR(new_err);
+ svn_ra_serf__request_create(handler);
}
+ SVN_ERR(run_locks(session, lock_requests, TRUE, lock_func, lock_baton,
+ iterpool));
+
svn_pool_destroy(iterpool);
return SVN_NO_ERROR;
}
-struct unlock_context_t {
- const char *token;
- svn_boolean_t force;
-};
-
static svn_error_t *
set_unlock_headers(serf_bucket_t *headers,
void *baton,
apr_pool_t *pool)
{
- struct unlock_context_t *ctx = baton;
+ lock_ctx_t *ctx = baton;
serf_bucket_headers_set(headers, "Lock-Token", ctx->token);
if (ctx->force)
@@ -429,6 +596,7 @@ svn_ra_serf__unlock(svn_ra_session_t *ra
svn_ra_serf__session_t *session = ra_session->priv;
apr_hash_index_t *hi;
apr_pool_t *iterpool;
+ apr_array_header_t *lock_requests;
iterpool = svn_pool_create(scratch_pool);
@@ -492,81 +660,62 @@ svn_ra_serf__unlock(svn_ra_session_t *ra
}
}
- /* ### TODO for issue 2263: Send all the locks over the wire at once. This
- ### loop is just a temporary shim.
- ### an alternative, which is backwards-compat with all servers is to
- ### pipeline these requests. ie. stop using run_wait/run_one. */
+ /* ### Perhaps we should open more connections than just one? See update.c */
+
+ lock_requests = apr_array_make(scratch_pool, apr_hash_count(path_tokens),
+ sizeof(lock_ctx_t*));
for (hi = apr_hash_first(scratch_pool, path_tokens);
hi;
hi = apr_hash_next(hi))
{
svn_ra_serf__handler_t *handler;
- const char *req_url, *path, *token;
- svn_lock_t *existing_lock = NULL;
- struct unlock_context_t unlock_ctx;
- svn_error_t *err = NULL;
- svn_error_t *new_err = NULL;
-
+ const char *req_url, *token;
+ lock_ctx_t *lock_ctx;
+ apr_pool_t *lock_pool;
svn_pool_clear(iterpool);
- path = svn__apr_hash_index_key(hi);
+ lock_pool = svn_pool_create(scratch_pool);
+ lock_ctx = apr_pcalloc(lock_pool, sizeof(*lock_ctx));
+
+ lock_ctx->pool = lock_pool;
+
+ lock_ctx->path = svn__apr_hash_index_key(hi);
token = svn__apr_hash_index_val(hi);
- unlock_ctx.force = force;
- unlock_ctx.token = apr_pstrcat(iterpool, "<", token, ">", SVN_VA_NULL);
+ lock_ctx->force = force;
+ lock_ctx->token = apr_pstrcat(lock_pool, "<", token, ">", SVN_VA_NULL);
- req_url = svn_path_url_add_component2(session->session_url.path, path,
- iterpool);
+ req_url = svn_path_url_add_component2(session->session_url.path, lock_ctx->path,
+ lock_pool);
- handler = apr_pcalloc(iterpool, sizeof(*handler));
+ handler = apr_pcalloc(lock_pool, sizeof(*handler));
- handler->handler_pool = iterpool;
+ handler->handler_pool = lock_pool;
handler->method = "UNLOCK";
handler->path = req_url;
handler->conn = session->conns[0];
handler->session = session;
handler->header_delegate = set_unlock_headers;
- handler->header_delegate_baton = &unlock_ctx;
+ handler->header_delegate_baton = lock_ctx;
handler->response_handler = svn_ra_serf__expect_empty_body;
handler->response_baton = handler;
- SVN_ERR(svn_ra_serf__context_run_one(handler, iterpool));
+ handler->no_fail_on_http_failure_status = TRUE;
- switch (handler->sline.code)
- {
- case 403:
- /* Api users expect this specific error code to detect failures */
- err = svn_error_createf(SVN_ERR_FS_LOCK_OWNER_MISMATCH, NULL,
- _("Unlock of '%s' failed: %d %s"),
- path,
- handler->sline.code,
- handler->sline.reason);
- break;
- case 400:
- err = svn_error_createf(SVN_ERR_FS_NO_SUCH_LOCK, NULL,
- _("No lock on path '%s': %d %s"),
- path,
- handler->sline.code,
- handler->sline.reason);
- break;
- default:
- err = svn_ra_serf__error_on_status(handler->sline,
- handler->path,
- handler->location);
- }
+ lock_ctx->handler = handler;
- if (lock_func)
- new_err = lock_func(lock_baton, path, FALSE, existing_lock, err,
- iterpool);
+ APR_ARRAY_PUSH(lock_requests, lock_ctx_t *) = lock_ctx;
- svn_error_clear(err);
- SVN_ERR(new_err);
+ svn_ra_serf__request_create(handler);
}
+ SVN_ERR(run_locks(session, lock_requests, FALSE, lock_func, lock_baton,
+ iterpool));
+
svn_pool_destroy(iterpool);
return SVN_NO_ERROR;
Modified: subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h?rev=1551993&r1=1551992&r2=1551993&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h Wed Dec 18 15:59:39 2013
@@ -433,6 +433,10 @@ typedef struct svn_ra_serf__handler_t {
for request. */
svn_boolean_t no_dav_headers;
+ /* If TRUE doesn't end the context directly on certain HTTP errors like 405,
+ 408, 500 (see util.c handle_response()) */
+ svn_boolean_t no_fail_on_http_failure_status;
+
/* Has the request/response been completed? */
svn_boolean_t done;
Modified: subversion/trunk/subversion/libsvn_ra_serf/util.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/util.c?rev=1551993&r1=1551992&r2=1551993&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/util.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/util.c Wed Dec 18 15:59:39 2013
@@ -2148,21 +2148,15 @@ handle_response(serf_request_t *request,
{
handler->discard_body = TRUE;
- if (!handler->session->pending_error)
+ if (!handler->session->pending_error
+ && !handler->no_fail_on_http_failure_status)
{
- apr_status_t apr_err = SVN_ERR_RA_DAV_REQUEST_FAILED;
-
- /* 405 == Method Not Allowed (Occurs when trying to lock a working
- copy path which no longer exists at HEAD in the repository. */
- if (handler->sline.code == 405
- && strcmp(handler->method, "LOCK") == 0)
- apr_err = SVN_ERR_FS_OUT_OF_DATE;
-
handler->session->pending_error =
- svn_error_createf(apr_err, NULL,
- _("%s request on '%s' failed: %d %s"),
- handler->method, handler->path,
- handler->sline.code, handler->sline.reason);
+ svn_ra_serf__error_on_status(handler->sline,
+ handler->path,
+ handler->location);
+
+ SVN_ERR_ASSERT(handler->session->pending_error != NULL);
}
}
}