You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by Robert O Butts <ro...@apache.org> on 2021/02/26 15:46:17 UTC

[VOTE] New API symbols for Parent Selection

I'd like to propose adding the following symbols to the API, to make it
possible for remap plugins to do next-hop parent selection:

enum TSHostStatus
enum TSHostStatusReason
TSReturnCode TSHostnameIsSelf(const char *hostname);
TSReturnCode TSHostStatusGet(const char *hostname, const size_t
hostname_len, TSHostStatus *status, unsigned int *reason);
void TSHostStatusSet(const char *hostname, const size_t hostname_len,
TSHostStatus status, const unsigned int down_time,  const unsigned int
reason);
struct TSResponseAction;
void TSHttpTxnResponseActionSet(TSHttpTxn txnp, TSResponseAction action);
void TSHttpTxnResponseActionGet(TSHttpTxn txnp, TSResponseAction *action);

I have a PR to do this, as well as a plugin for ConsistentHash parent
selection:

https://github.com/apache/trafficserver/pull/7467

All new symbols except the ResponseAction ones expose existing things in
core which are necessary for parent selection.

The ResponseAction struct is a small C struct with the data necessary to do
parent selection, essentially everything that's a function in Strategies
today.

In a nutshell, everything in core (HttpTransact.cc) that was `if (strategy)
{ } else { /* old parent */ }` becomes `if (response_action.handled) { }
else if (strategy) { } else { /* old parent */ }`.

This supersedes my previous proposal. The previous proposal was a small
core change, but had the downside of not being a real Remap plugin, and not
having access to Continuations, Hooks, etc. The alternative seemed to be
large core State Machine changes.

I believe this is the best of both worlds: a small core change, while still
being a real plugin with access to continuations and hooks.

The advantage is immediately evident: in the plugin, all the
per-transaction hash data is moved into the plugin and stored in the
continuation, out of core (ParentResult). Which means less logic in core,
and ParentResult can be deleted if parent.config and core strategies are
removed. This was impossible with the previous method of dropping the
strategy object into core.

Feedback welcome, please vote, thanks!

Re: [E] [VOTE] New API symbols for Parent Selection

Posted by Alan Carroll <so...@verizonmedia.com.INVALID>.
+1

On Thu, Mar 4, 2021 at 4:22 PM Robert O Butts <ro...@gmail.com>
wrote:

> > > void TSHttpTxnResponseActionSet(TSHttpTxn txnp, TSResponseAction
> action); -
> > I still don't like passing the action by value.
>
> Yes, it's a pointer now. Sorry, I changed the code, forgot to update the PR
> description:
>
> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_apache_trafficserver_blob_f22e1c_src_traffic-5Fserver_InkAPI.cc-23L10039&d=DwIBaQ&c=sWW_bEwW_mLyN3Kx2v57Q8e-CRbmiT9yOhqES_g_wVY&r=5nE_8e-Jc1t5vF6GVeub9BCN4FzSc_6kU7_mjSiUrDs&m=zFZbls36NrhIT2vj_0hfqinSa9lAhT-6H4trx4FBBoQ&s=fB-M3Z_KFA6aSyaHeYdEIPIMyITM1nAhfCfnCwf7pO4&e=
>
> void TSHttpTxnResponseActionSet(TSHttpTxn txnp, TSResponseAction *action);
>
>
> On Thu, Mar 4, 2021 at 3:18 PM Alan Carroll
> <so...@verizonmedia.com.invalid> wrote:
>
> > >
> > > I think you noted that TSHostnameIsSelf(const char *hostname) is
> painful
> > > to make take a non-null terminated string. Is that still the case?
> >
> >
> > void TSHttpTxnResponseActionSet(TSHttpTxn txnp, TSResponseAction
> action); -
> > I still don't like passing the action by value.
> >
>

Re: [E] [VOTE] New API symbols for Parent Selection

Posted by Robert O Butts <ro...@gmail.com>.
> > void TSHttpTxnResponseActionSet(TSHttpTxn txnp, TSResponseAction
action); -
> I still don't like passing the action by value.

Yes, it's a pointer now. Sorry, I changed the code, forgot to update the PR
description:
https://github.com/apache/trafficserver/blob/f22e1c/src/traffic_server/InkAPI.cc#L10039

void TSHttpTxnResponseActionSet(TSHttpTxn txnp, TSResponseAction *action);


On Thu, Mar 4, 2021 at 3:18 PM Alan Carroll
<so...@verizonmedia.com.invalid> wrote:

> >
> > I think you noted that TSHostnameIsSelf(const char *hostname) is painful
> > to make take a non-null terminated string. Is that still the case?
>
>
> void TSHttpTxnResponseActionSet(TSHttpTxn txnp, TSResponseAction action); -
> I still don't like passing the action by value.
>

Re: [E] [VOTE] New API symbols for Parent Selection

Posted by Alan Carroll <so...@verizonmedia.com.INVALID>.
>
> I think you noted that TSHostnameIsSelf(const char *hostname) is painful
> to make take a non-null terminated string. Is that still the case?


void TSHttpTxnResponseActionSet(TSHttpTxn txnp, TSResponseAction action); -
I still don't like passing the action by value.

Re: [VOTE] New API symbols for Parent Selection

Posted by John Rushford <jr...@apache.org>.
+1 add the new API's

John Rushford
jrushford@apache.org

On Tue, Mar 9, 2021 at 10:08 AM Robert O Butts <ro...@apache.org> wrote:

>
>
> ---------- Forwarded message ---------
> From: Robert O Butts <ro...@apache.org>
> Date: Fri, Feb 26, 2021 at 8:46 AM
> Subject: [VOTE] New API symbols for Parent Selection
> To: <de...@trafficserver.apache.org>
>
>
> I'd like to propose adding the following symbols to the API, to make it
> possible for remap plugins to do next-hop parent selection:
>
> enum TSHostStatus
> enum TSHostStatusReason
> TSReturnCode TSHostnameIsSelf(const char *hostname);
> TSReturnCode TSHostStatusGet(const char *hostname, const size_t
> hostname_len, TSHostStatus *status, unsigned int *reason);
> void TSHostStatusSet(const char *hostname, const size_t hostname_len,
> TSHostStatus status, const unsigned int down_time,  const unsigned int
> reason);
> struct TSResponseAction;
> void TSHttpTxnResponseActionSet(TSHttpTxn txnp, TSResponseAction action);
> void TSHttpTxnResponseActionGet(TSHttpTxn txnp, TSResponseAction *action);
>
> I have a PR to do this, as well as a plugin for ConsistentHash parent
> selection:
>
> https://github.com/apache/trafficserver/pull/7467
>
> All new symbols except the ResponseAction ones expose existing things in
> core which are necessary for parent selection.
>
> The ResponseAction struct is a small C struct with the data necessary to
> do parent selection, essentially everything that's a function in Strategies
> today.
>
> In a nutshell, everything in core (HttpTransact.cc) that was `if
> (strategy) { } else { /* old parent */ }` becomes `if
> (response_action.handled) { } else if (strategy) { } else { /* old parent
> */ }`.
>
> This supersedes my previous proposal. The previous proposal was a small
> core change, but had the downside of not being a real Remap plugin, and not
> having access to Continuations, Hooks, etc. The alternative seemed to be
> large core State Machine changes.
>
> I believe this is the best of both worlds: a small core change, while
> still being a real plugin with access to continuations and hooks.
>
> The advantage is immediately evident: in the plugin, all the
> per-transaction hash data is moved into the plugin and stored in the
> continuation, out of core (ParentResult). Which means less logic in core,
> and ParentResult can be deleted if parent.config and core strategies are
> removed. This was impossible with the previous method of dropping the
> strategy object into core.
>
> Feedback welcome, please vote, thanks!
>
>