On 12 April 2016 at 10:30, Ivan Zhakov <ivan_at_visualsvn.com> wrote:
> On 3 March 2015 at 04:06, <rhuijben_at_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
Received on 2016-04-15 14:07:47 CEST