Mailing List Archive

sub probe_resp - VIP RFC
Hi,

https://github.com/varnishcache/varnish-cache/wiki/Varnish-Improvement-Proposals :
"The VIP procedure starts with a discussions on varnish-dev"

I had skipped this bit for previous suggestions, but this time I want to get it
right. I'd like to propose the following:


# Synopsis

Add support for calling vcl subs on the response of backend probes.

# Why?

Add a way to manipulate backends not just on the basis of a binary health check
result, but optionally also on the basis of other information returned by the
backend with the probe response.

One example would be to dynamically change the weight of backends based on a
load metric returned with the probe response, which will also require
vmod_directors to support changing the weight dynamically.

# How?

* Add VCC support to register VCL subs with probes

* Add a default vcl_probe_resp sub to the builtin.vcl which implements the
current behavior. Probes without an explicit response sub definition will
use vcl_probe_resp

* in probe_resp context, make the following objects available

- analogous to beresp

- prresp.backend
- prresp.http.*
- prresp.proto
- prresp.status
- prresp.reason

- later?
- prresp.body

By design, all access should be read-only, but we might want to
have all but .backend writable for practical reasons (writes
having no effect other than being visible in the sub probe_resp)

- probe.* attributes of the probe (read-only)

- probe.name
- probe.expected_response
- probe.timeout
- probe.interval
- probe.initial
- probe.window
- probe.threshold

* a probe_resp sub may return with the following

healthy
sick

* The default vcl_probe_resp:

sub vcl_probe_resp {
if (prresp.proto ~ "^HTTP/\d+\.\d+$" &&
prresp.status == probe.expected_response) {
return (healthy);
}

return (sick);
}

* Toy example for the use case mentioned above (needs more changes)

sub probe_weight {
if (prresp.http.X-Load ~ "^\d+\.\d+$") {
my_rr.set_weight(prresp.backend,
1 / std.real(req.http.X-Load, 1.0));
}
}


_______________________________________________
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev
Re: sub probe_resp - VIP RFC [ In reply to ]
> # Why?

No opinion on the why, I get the point and I see the benefits but no
opinion. It also leaves out the non-VBE backends, although as of today
I m only aware of Martin's fsbackend implementation of an out-of-tree
backend.

> * Add a default vcl_probe_resp sub to the builtin.vcl which implements the
> current behavior. Probes without an explicit response sub definition will
> use vcl_probe_resp

I'd rather have vcl_probe_response, akin to existing v_b_r.

> * in probe_resp context, make the following objects available
>
> - analogous to beresp
>
> - prresp.backend
> - prresp.http.*
> - prresp.proto
> - prresp.status
> - prresp.reason

Why not beresp? It's a response from the backend, I don't think
reusing the name would be confusing.

> * a probe_resp sub may return with the following
>
> healthy
> sick

Or we could reuse "ok" and the universal "fail" like in vcl_init.

> * The default vcl_probe_resp:
>
> sub vcl_probe_resp {
> if (prresp.proto ~ "^HTTP/\d+\.\d+$" &&

^HTTP/1\.[01]$

Dridi

_______________________________________________
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev
Re: sub probe_resp - VIP RFC [ In reply to ]
Dridi,

thank you for the quick feedback. I'm fine with your suggestions, except:

>> * The default vcl_probe_resp:
>>
>> sub vcl_probe_resp {
>> if (prresp.proto ~ "^HTTP/\d+\.\d+$" &&
>
> ^HTTP/1\.[01]$

This was intended to be equivalent to the current code:

i = sscanf(vt->resp_buf, "HTTP/%*f %u ", &resp);

_______________________________________________
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev
Re: sub probe_resp - VIP RFC [ In reply to ]
On Tue, Apr 11, 2017 at 4:31 PM, Nils Goroll <slink@schokola.de> wrote:
> Dridi,
>
> thank you for the quick feedback. I'm fine with your suggestions, except:
>
>>> * The default vcl_probe_resp:
>>>
>>> sub vcl_probe_resp {
>>> if (prresp.proto ~ "^HTTP/\d+\.\d+$" &&
>>
>> ^HTTP/1\.[01]$
>
> This was intended to be equivalent to the current code:
>
> i = sscanf(vt->resp_buf, "HTTP/%*f %u ", &resp);

Considering that we only support 1.0 and 1.1 today, I believe we would
get a more accurate result with this regex instead of a clunky scanf
format.

If we go for vcl_probe_response we might as well improve the parsing
of the response, why not?

Cheers,
Dridi

_______________________________________________
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev
Re: sub probe_resp - VIP RFC [ In reply to ]
On 11/04/17 16:47, Dridi Boukelmoune wrote:
> If we go for vcl_probe_response we might as well improve the parsing
> of the response, why not?

Agreed, this is an easy additional step once we got the change in place.

_______________________________________________
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev