On 12 April 2016 at 10:30, Ivan Zhakov <i...@visualsvn.com> wrote: > On 3 March 2015 at 04:06, <rhuij...@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? > Committed in r1739278 and r1739280. And added them to r1663500 nomination in 1.9.x.
-- Ivan Zhakov
