https://bz.apache.org/bugzilla/show_bug.cgi?id=63626

--- Comment #14 from Michael Osipov <[email protected]> ---
(In reply to Ruediger Pluem from comment #13)
> (In reply to Michael Osipov from comment #12)
> > 
> > Sorry for responding so late. I agree, this should be generalized. Patch has
> > been updated.
> 
> Some comments:
> 
> 1. The patch seems to be against 2.4.x. We need a patch against trunk first.
> Afterwards this code can be backported to the stable branch.

No problem.

> 2. As said in #4 the HTTP_SERVICE_UNAVAILABLE is correct here and must stay
> as it ensures that a correct failover / retry happens in case the existing
> connection was just dead.

Reread and understand your reasoning. I think you have a typo:
> This allows to sent non idempotent requests to a different backend in a load 
> balanced / fail over scenario.

Did you mean idemponent? Otherwise it does not make any sense to retry a
request.

Can we add a comment to the do_100_continue to clearly document this is a ping
and not a real request?

> 3. ap_proxy_backend_broke is a generic procedure that is also called in
> cases where the cause of the failure is not a timeout. Hence the status code
> cannot be just changed. The patch would need to be more complex. As
> ap_proxy_backend_broke is an API function the way forward would be to create
> an ap_proxy_backend_broke_ex which has an additional parameter for the
> status code and hence can be called with HTTP_GATEWAY_TIME_OUT as status
> code where needed and change ap_proxy_backend_broke to call
> ap_proxy_backend_broke_ex with HTTP_BAD_GATEWAY as status code.
> 
> diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c
> index 5482ab8a48..1177173968 100644
> --- a/modules/proxy/proxy_util.c
> +++ b/modules/proxy/proxy_util.c
> @@ -3305,7 +3305,7 @@ PROXY_DECLARE(void) ap_proxy_backend_broke(request_rec
> *r,
>       */
>      if (r->main)
>          r->main->no_cache = 1;
> -    e = ap_bucket_error_create(HTTP_BAD_GATEWAY, NULL, c->pool,
> +    e = ap_bucket_error_create(HTTP_GATEWAY_TIME_OUT, NULL, c->pool,
>                                 c->bucket_alloc);
>      APR_BRIGADE_INSERT_TAIL(brigade, e);
>      e = apr_bucket_eos_create(c->bucket_alloc);

Makes sense, will work on that.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to