subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ivan Zhakov <i...@visualsvn.com>
Subject Re: svn commit: r1663500 - in /subversion/trunk/subversion: libsvn_ra_serf/commit.c libsvn_ra_serf/merge.c libsvn_ra_serf/ra_serf.h tests/cmdline/lock_tests.py
Date Tue, 12 Apr 2016 07:30:20 GMT
On 3 March 2015 at 04:06,  <rhuijben@apache.org> wrote:
> Author: rhuijben
> Date: Tue Mar  3 01:06:16 2015
> New Revision: 1663500
>
> URL: http://svn.apache.org/r1663500
> Log:
> In ra-serf (for issue #4557) reinstate a bit of code that retries a delete
> with an altered request if the original request fails, because the server
> determined that it is an invalid request, because it has too many bytes
> in the headers.
>
> Before r1553501 we always retried DELETE requests that required lock
> tokens, as the initial request didn't have an If header at all.
>
Hi Bert,

See my review inline.

[...]

> Modified: subversion/trunk/subversion/libsvn_ra_serf/commit.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/commit.c?rev=1663500&r1=1663499&r2=1663500&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_ra_serf/commit.c (original)
> +++ subversion/trunk/subversion/libsvn_ra_serf/commit.c Tue Mar  3 01:06:16 2015
[...]

>  static svn_error_t *
>  delete_entry(const char *path,
>               svn_revnum_t revision,
> @@ -1412,6 +1443,7 @@ delete_entry(const char *path,
>    delete_context_t *delete_ctx;
>    svn_ra_serf__handler_t *handler;
>    const char *delete_target;
> +  svn_error_t *err;
>
>    if (USING_HTTPV2_COMMIT_SUPPORT(dir->commit_ctx))
>      {
> @@ -1446,7 +1478,23 @@ delete_entry(const char *path,
>    handler->method = "DELETE";
>    handler->path = delete_target;
>
> -  SVN_ERR(svn_ra_serf__context_run_one(handler, pool));
> +  err = svn_ra_serf__context_run_one(handler, pool);
> +  if (err && err->apr_err == SVN_ERR_RA_DAV_REQUEST_FAILED
> +      && handler->sline.code == 400)
May be it's better set handler->no_fail_on_http_failure_status to TRUE
and check for handler->sline.code? This will remove dependency on
specific error code and simplify code a bit.

> +    {
> +      svn_error_clear(err);
> +
> +      /* Try again with non-standard body to overcome Apache Httpd
> +         header limit */
> +      delete_ctx->non_recursive_if = TRUE;
> +      handler->body_type = "text/xml";
> +      handler->body_delegate = create_delete_body;
> +      handler->body_delegate_baton = delete_ctx;
> +
> +      SVN_ERR(svn_ra_serf__context_run_one(handler, pool));
I'm not sure that we can reuse svn_ra_serf__handler_t instance for
multiple requests.

I propose the following patch. Do you have any objections?

-- 
Ivan Zhakov

Mime
View raw message